ASF GitHub Bot logged work on BEAM-4622:

                Author: ASF GitHub Bot
            Created on: 11/Jul/18 13:50
            Start Date: 11/Jul/18 13:50
    Worklog Time Spent: 10m 
      Work Description: aromanenko-dev commented on a change in pull request 
#5912: [BEAM-4622] Makes required to call Beam SQL expressions validation
URL: https://github.com/apache/beam/pull/5912#discussion_r201696488

 File path: 
 @@ -47,6 +48,13 @@ public BeamSqlReinterpretExpression(List<BeamSqlExpression> 
operands, SqlTypeNam
   public boolean accept() {
+    // Interval types will be already converted into BIGINT after evaluation.
+    SqlTypeFamily opTypeFamily = opType(0).getFamily();
+    if (opTypeFamily.equals(SqlTypeFamily.INTERVAL_DAY_TIME)
 Review comment:
   @vectorijk Actually, it's already implicitly tested by 
`BeamSqlDslSqlStdOperatorsTest.testTimestampDiff()` and it failed after I did a 
refactoring of `BeamSqlFnExecutor` and before adding this check. Do you think 
it will make sense to add specific test for this case?

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:

Issue Time Tracking

    Worklog Id:     (was: 121903)
    Time Spent: 40m  (was: 0.5h)

> Many Beam SQL expressions never have their validation called
> ------------------------------------------------------------
>                 Key: BEAM-4622
>                 URL: https://issues.apache.org/jira/browse/BEAM-4622
>             Project: Beam
>          Issue Type: Bug
>          Components: dsl-sql
>            Reporter: Kenneth Knowles
>            Assignee: Alexey Romanenko
>            Priority: Major
>              Labels: easyfix, newbie, starter
>          Time Spent: 40m
>  Remaining Estimate: 0h
> In {{BeamSqlFnExecutor}} there is a pattern where first the returned 
> expression is assigned to a variable {{ret}} and then after a giant switch 
> statement the validation is invoked. But there are many code paths that just 
> call {{return}} and skip validation. This should be refactored so it is 
> impossible to short-circuit on accident like this.

This message was sent by Atlassian JIRA

Reply via email to