ASF GitHub Bot commented on PHOENIX-3355:

Github user maryannxue commented on the issue:

    No differentiation between the regex functions (String based and Byte based)
    -- Let's just add support for this one?
    Invert and MD5 Function don't work (phoenix doesn't supply the correct 
arguments in annotation)
    -- Can we make it right, or is it more complicated than that? If the 
latter, we can open another issue.
    Type matching doesn't work on input arguments (date/timestamp)
    -- It's probably a Calcite issue. Let's try repo it as a Calcite test case. 
Let me know if you need help working with Calcite unit tests.
    Timezone parameter doesn't work properly in ToDateFunction (basic cases 
work, but custom timezones do not act as expected)
    I probably know this one. Not sure if this is the issue though: the 
date/time/timestamp objects are local time (w/ timezone offset) in 
Calcite/Avatica, while they are UTC time on the Phoenix side.
    Array constructors are not supported (another issue entirely)
    -- Yes, another issue. We could open a JIRA listing those test failures 
blocking on this issue as a way to keep track of them.
    As to the code refinement, I think there are 4 (maybe more?) big chunks of 
code: createBuiltinFunctions(), evaluateReturnType(), overloadArguments() and 
registerBuiltinFunctions(). We should probably put all these in one place. I 
prefer in PhoenixSchema, otherwise CalciteUtils could also do. What makes sense 
to where the code should live, aside from what class of object it instantiates, 
is also who is the caller or who decides the logic or routines. So it seems to 
me that PhoenixScalarFunction here is only the "callee" class which is used by 
PhoenixSchema and CalciteUtils.
    There are a couple of things here that eventually forms the complete list 
of functions that we register in Calcite:
    1. We create multiple signatures corresponding to each overload of the same 
    2. We merge all versions of the same signature but with different default 
parameter counts into one function.
    3. We deal with aliases.
    Part of the reason why we are doing this is that we are trying to 
reorganize the existing function dictionary into something that would match the 
Calcite conventions. The implementation could end up being very different if 
the existing Phoenix function structure were something else. So I guess it'd be 
easier to maintain if we put these methods all in one place and maybe we should 
add more code comment describing what each of these methods do.

> Register Phoenix built-in functions as Calcite functions
> --------------------------------------------------------
>                 Key: PHOENIX-3355
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-3355
>             Project: Phoenix
>          Issue Type: Bug
>            Reporter: James Taylor
>            Assignee: Eric Lomore
>              Labels: calcite
>         Attachments: PHOENIX-3355.function_constructor.patch, 
> PHOENIX-3355.wip, PHOENIX-3355.wip2
> We should register all Phoenix built-in functions that don't exist in Calcite.

This message was sent by Atlassian JIRA

Reply via email to