calcite
calcite copied to clipboard
apache
[CALCITE-2772] Support varargs UDF for scalar function
Support varargs for user-defined functions as the case followed:
public class ConcatWs { public String eval(String sep, String... strs) {...} }
JIRA: https://issues.apache.org/jira/browse/CALCITE-2772
@danny0405
One example I found for varargs use case is when I tried to built ZetaSQL on top of Calcite. ZetaSQL has a concat function definition that accepts varargs [1]. So varargs will make concat(string...) implementable. I will agree concat(string...) can be written as a nested of binary concat, but it makes query more readable if need to concat more than 3 strings.
[1] : https://github.com/google/zetasql/blob/master/docs/string_functions.md#concat
@danny0405
One example I found for varargs use case is when I tried to built ZetaSQL on top of Calcite. ZetaSQL has a
concatfunction definition that accepts varargs [1]. So varargs will makeconcat(string...)implementable. I will agreeconcat(string...)can be written as a nested of binaryconcat, but it makes query more readable if need to concat more than 3 strings.[1] : https://github.com/google/zetasql/blob/master/docs/string_functions.md#concat
Thanks for the example for CONCAT, in Calcite, we already have such function support as sql dialect [1], we declare it's operand types as OperandTypes.repeat, for repeated types arguments, we can support as this way, but if the arguments are different types but also variadic, maybe support freely variadic arguments is the right way to go.
[1] https://github.com/apache/calcite/blob/2dc97e6723e1b5bf762540f87ffffb5cd1a848a1/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java#L255
Maybe I was looking into a wrong place. At least Calcite does not have a vararg version of concat in [1]. This PR should be able to enable it as last time I tried.
[1] : https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java#L447
I have fixed most issues based on above comments. Are there any issues that need to be improved? @danny0405
Hi, @gr4ve, could you please rebase this work?
We have encountered use case that varargs UDF can play important role. As we use Calcite to connect different datasources, we need to define many "fake" (i.e., just registered but without implemention) functions and push them down and then generates SQL to run in underlying datasources. On one hand, the function iteself may be vararag. On the other hand, varargs UDF can help us to define just one for many functions that have same name.
It seems that @gr4ve adressed the comments of @danny0405 . @DonnyZone , @XuQianJin-Stars do you believe this PR is ready for merge? If yes then I can do the rebase and solve any remaining minor glithces (if there are).
Thanks @zabetak, I have not ever tried the fix in our environment. But the PR is OK to me. I do not have any further comments.
Hi @gr4ve, to solve the conflicts, you need to rebase this pr on latest calcite/master branch.
@gr4ve, do you still want to work on this? If no, I would like to take over if you don't mind.
when will this pull request merge? this feature is very helpful. thanks @gr4ve @hsyuan
Agreed. This is something we'd really like because we're having trouble working around it. Any chance this could be handed off to someone with the time to do it?
