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

Julian Hyde commented on CALCITE-5575:
--------------------------------------

[~dzamo], I've said it before but I'll say it again. Drill cannot reasonably 
complain about 'regressions' if there are no tests in Calcite to detect those 
regressions. It would be very helpful if Drill upstreamed some of its tests 
Calcite (or added tests that check its assumptions, or ran its test suite 
against Calcite main fairly regularly). Same goes for any project (or 
commercial product) that has tight dependencies on Calcite.

> Incorrect priority of builtinFunctionCallMethods in Parser.jj
> -------------------------------------------------------------
>
>                 Key: CALCITE-5575
>                 URL: https://issues.apache.org/jira/browse/CALCITE-5575
>             Project: Calcite
>          Issue Type: Bug
>          Components: core
>    Affects Versions: 1.34.0
>            Reporter: James Turton
>            Priority: Major
>
> CALCITE-5469 and others have introduced new date and time functions. In 
> Parser.jj (line 6152) these new functions are handled _above_ the 
> builtinFunctionCallMethods Freemarker loop that allows applications to add 
> their own function defintions.
> {code:java}
>         node = DateTimeConstructorCall() { return node; }
>     |
>         node = DateDiffFunctionCall() { return node; }
>     |
>         node = DateTruncFunctionCall() { return node; }
>     |
>         node = TimestampAddFunctionCall() { return node; }
>     |
>         node = DatetimeDiffFunctionCall() { return node; }
>     |
>         node = TimestampDiffFunctionCall() { return node; }
>     |
>         node = TimestampDiff3FunctionCall() { return node; }
>     |
>         node = TimestampTruncFunctionCall() { return node; }
>     |
>         node = TimeDiffFunctionCall() { return node; }
>     |
>         node = TimeTruncFunctionCall() { return node; }
>     |
> <#list 
> (parser.builtinFunctionCallMethods!default.parser.builtinFunctionCallMethods) 
> as method>
>         node = ${method} { return node; }
>     |
> </#list>{code}
> This means (I think) that applications are effectively unable to override the 
> new date and time function definitions with their own definitions. In 
> particular, Drill cannot override the new three-parameter DATE_DIFF function 
> in order to provide the parser method needed by its own two-parameter 
> DATE_DIFF function resulting in a regression in the form of a broken 
> DATE_DIFF function.
> If the above is correct then can the builtinFunctionCallMethods Freemarker 
> loop move above these date and time functions? Could it even move above the 
> special syntax functions higher up, for that matter?
> Incidentally I also noted in Parser.jj that the new date and time functions 
> mentioned above are also handled seperately in JdbcFunctionCall which 
> implements JDBC function call syntax. This makes me unsure whether 
> application overrides provided in builtinFunctionCallMethods will take 
> priority over these Calcite definitions when they are invoked with JDBC 
> syntax.
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to