phoenix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Eric Lomore (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (PHOENIX-3355) Register Phoenix built-in functions as Calcite functions
Date Mon, 24 Oct 2016 21:55:58 GMT

    [ https://issues.apache.org/jira/browse/PHOENIX-3355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15603343#comment-15603343
] 

Eric Lomore commented on PHOENIX-3355:
--------------------------------------

[~maryannxue] thanks for the feedback!

For 2. I think I might have miscommunicated the problem a bit. While calcite does support
default values that isn't the issue I'm facing.The BUILT_IN_FUNCTION_MAP in phoenix elects
to store every possible signature of each function. When registering builtins, as you noted,
Calcite wants only the original declaration, the one saying 'FuncTest(VARCHAR var1, VARCHAR
var2 (default = null), DATE date (default = TODAY()))'. But in the function map we have 3
simplified versions of that function, with 3, 2, and 1 args. Getting the signature with 3
arguments is expensive on processing time because it would have to do a lengthy iteration
to be sure the base signature is found. This example method helps demonstrate what I mean:

{code}
    public static List<BuiltInFunctionInfo> getBaseFunction(String normalizedName) {
        initBuiltInFunctionMap();
        List<BuiltInFunctionInfo> infos = Lists.newArrayList();
        while(!baseFunction) { // numArgs would be incremented here
            BuiltInFunctionInfo
                    info =
                    BUILT_IN_FUNCTION_MAP.get(new BuiltInFunctionKey(normalizedName, numArgs));
            if (info.isBaseFunction()) {
                return info;
            }
        }
        throw new RuntimeException("Couldn't find the base signature for builtin function");
    }
{code}

This points me to the tradeoff I was discussing,
- We could do this workaround, but I'm very hesitant
- Could create another function map, only with the largest declaration.
- Could go with the approach in the patch I submitted originally. There's some degree of code
duplication but it skips over these issues.

For 3. I'm not sure which calcite mechanism you and [~jamestaylor] are referring to. I have
mulled over the ways we could adjust PhoenixScalarFunction.getReturnType(RelDataTypeFactory)
but PhoenixScalarFunction never sees the input parameters so I think some changes would have
to be done upstream of this. Will keep exploring, but let me know if you have any thoughts!

> 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.wip
>
>
> We should register all Phoenix built-in functions that don't exist in Calcite.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message