[GitHub] [druid] clintropolis commented on pull request #11088: add round test

[GitBox <gi...@apache.org>]

clintropolis commented on pull request #11088:
URL: https://github.com/apache/druid/pull/11088#issuecomment-817094633


   Ah, your test has found a bug :+1:
   
   The native `round` function does not correctly handle null values:
   ```
   [ERROR] testRoundFuc(org.apache.druid.sql.calcite.CalciteQueryTest)  Time elapsed: 0.047 s  <<< ERROR!
   org.apache.druid.java.util.common.IAE: The first argument to the function[round] should be integer or double type but got the type: STRING
   	at org.apache.druid.math.expr.Function$Round.apply(Function.java:1318)
   	at org.apache.druid.math.expr.FunctionExpr.eval(FunctionalExpr.java:176)
   	at org.apache.druid.segment.virtual.ExpressionColumnValueSelector.getObject(ExpressionColumnValueSelector.java:76)
   	at org.apache.druid.segment.virtual.ExpressionColumnValueSelector.getObject(ExpressionColumnValueSelector.java:34)
   	at org.apache.druid.segment.virtual.ExpressionSelectors$1.getObject(ExpressionSelectors.java:107)
   	at org.apache.druid.query.scan.ScanQueryEngine$1$1.getColumnValue(ScanQueryEngine.java:241)
   ```
   
   The error message is sort of funny, but what is happening here is that in non-vectorized expression processing, null values end up "typed" as STRING.
   
   this is the check: https://github.com/apache/druid/blob/master/core/src/main/java/org/apache/druid/math/expr/Function.java#L1314
   
   we need to modify this code to be something like 
   
   ```
         if (NullHandling.sqlCompatible() && value1.isNumericNull()) {
           return ExprEval.of(null);
         }
         if (value1.type() != ExprType.LONG && value1.type() != ExprType.DOUBLE) {
           throw new IAE(
               "The first argument to the function[%s] should be integer or double type but got the type: %s",
               name(),
               value1.type()
           );
         }
   ...
   ```
   
   


-- 
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org