prestodb / presto

The official home of the Presto distributed SQL query engine for big data
http://prestodb.io
Apache License 2.0
16.06k stars 5.38k forks source link

Easier scalar func support #23642

Open auden-woolfson opened 2 months ago

auden-woolfson commented 2 months ago

Description

Fix for issue #23605

Currently in order to add new operator/function mappings a new function needs to be defined for each possible mapping. By introducing some new annotations for functions and handling the case we can re use function definitions with different possible type signatures.

== RELEASE NOTES ==
General Changes
* Add SupportedSignatures and SqlSignature annotations. :pr:`23642`
* Update OperatorTranslators and tests to use new annotations. :pr:`23642`
aaneja commented 2 months ago

Can you add tests ? Take a look at TestRowExpressionTranslator - this uses translations defined in TestRowExpressionTranslator.TestFunctions

ScrapCodes commented 1 week ago

Hi Auden,

1) how does the annotation on a scalar function with two parameters and both parameter having different Sql types, look like?

2) Can we provide alternatives with different argument types?

For example:

   @SupportedSignatures(
   {@SqlSignature(argumentType = {StandardTypes.BIGINT, StandardTypes.Double}, returnType = StandardTypes.BIGINT),
   @SqlSignature(argumentType = {StandardTypes.BIGINT, StandardTypes.BIGINT}, returnType = StandardTypes.BIGINT)})
    public static ClickHouseExpression add(ClickHouseExpression left, ClickHouseExpression right)
    {
        return new ClickHouseExpression(infixOperation("+", left, right), forwardBindVariables(left, right));
    }

Do you think it makes sense?

steveburnett commented 1 week ago

Thanks for the release note entry! Just a formatting nit.

== RELEASE NOTES ==
General Changes
* Add SupportedSignatures and SqlSignature annotations. :pr:`23642`
* Update OperatorTranslators and tests to use new annotations. :pr:`23642`
auden-woolfson commented 1 week ago

Hi Auden,

1. how does the annotation on a scalar function with two parameters and both parameter having different Sql types, look like?

2. Can we provide alternatives with different argument types?

For example:

   @SupportedSignatures(
   {@SqlSignature(argumentType = {StandardTypes.BIGINT, StandardTypes.Double}, returnType = StandardTypes.BIGINT),
   @SqlSignature(argumentType = {StandardTypes.BIGINT, StandardTypes.BIGINT}, returnType = StandardTypes.BIGINT)})
    public static ClickHouseExpression add(ClickHouseExpression left, ClickHouseExpression right)
    {
        return new ClickHouseExpression(infixOperation("+", left, right), forwardBindVariables(left, right));
    }

Do you think it makes sense?

Yes this makes sense. I considered adding this functionality as well but as far as I can tell there are no SQL functions that we currently support that take multiple parameters of different types. If this is something that you could see happening in the future I can update this PR with that functionality, but my original thinking was that we would handle this case if and when it arises.

ScrapCodes commented 5 days ago

StringFunctions.java is one example

auden-woolfson commented 5 days ago

StringFunctions.java is one example

Thanks. I just added support for multiple argument types in a signature if you could please review my new commits.