[ 
https://issues.apache.org/jira/browse/BEAM-8241?focusedWorklogId=313339&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-313339
 ]

ASF GitHub Bot logged work on BEAM-8241:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 16/Sep/19 22:46
            Start Date: 16/Sep/19 22:46
    Worklog Time Spent: 10m 
      Work Description: amaliujia commented on pull request #9586: [BEAM-8241] 
SQL code gen is more restrictive than Calcite
URL: https://github.com/apache/beam/pull/9586#discussion_r324914716
 
 

 ##########
 File path: 
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/ScalarFunctionImpl.java
 ##########
 @@ -139,7 +139,7 @@ public CallImplementor getImplementor() {
 
     private static List<Expression> translate(List<Type> types, 
List<Expression> expressions) {
       Preconditions.checkArgument(
-          types.size() == expressions.size(), "types.size() != 
expressions.size()");
+          types.size() >= expressions.size(), "types.size() < 
expressions.size()");
 
 Review comment:
   The following loop was written based on the assumption that types = 
expression so each expression has a corresponding type. The check was based on 
the same assumption.
   
   But now the assumption is proven to be wrong.
   
   For example, I can see a valid input is:
   types: [String[].class]
   expression: [param1, param2, ...]
   
   so the for loop will need to be modified to make this input work(every param 
match with String type). The loop happens to be right with `types.size() >= 
expressions.size()`.
   
   I don't want to make this PR a one that needs long time to fix something. So 
if we want to check-in `types.size() >= expressions.size()`, at least can 
@11moon11 add comments to document what is going on, why need this change and 
leave a TODO in the for loop with a JIRA to support user defined function with 
var-arg?
   
   
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 313339)
    Time Spent: 1h 40m  (was: 1.5h)

> BEAM SQL code gen is more restrictive than Calcite
> --------------------------------------------------
>
>                 Key: BEAM-8241
>                 URL: https://issues.apache.org/jira/browse/BEAM-8241
>             Project: Beam
>          Issue Type: Bug
>          Components: dsl-sql
>    Affects Versions: 2.15.0
>            Reporter: Kirill Kozlov
>            Priority: Major
>          Time Spent: 1h 40m
>  Remaining Estimate: 0h
>
> In user defined functions Calcite allows variants with fewer arguments than 
> Beam defined.
> Calcite, when generating code for functions with less arguments than defined 
> will generate a proper function call.
> Some scalar functions, like concat, take advantage of this feature. Thus, 
> requiring more permissive checks in translate.



--
This message was sent by Atlassian Jira
(v8.3.2#803003)

Reply via email to