[GitHub] [druid] suneet-s commented on a change in pull request #10014: ROUND and having comparators correctly handle doubles

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

suneet-s commented on a change in pull request #10014:
URL: https://github.com/apache/druid/pull/10014#discussion_r437956928



##########
File path: sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java
##########
@@ -122,6 +122,7 @@ public void reduce(
             if (exprResult.type() == ExprType.LONG) {
               bigDecimal = BigDecimal.valueOf(exprResult.asLong());
             } else {
+              // if exprResult evaluates to Nan or infinity, this will throw a NumberFormatException

Review comment:
       I wasn't sure how to deal with non numbers here, so I decided to leave the behavior as is and let it fail. It's unclear to me if Calcite will allow us to get here. In my local testing, I've seen NumberFormatExceptions thrown in Calcite when I tried to write sql expressions that would compute to Nan or infinity (like `0D / 0`).
   
   My thoughts were this is an edge case so it's ok to leave this behavior as is.




----------------------------------------------------------------
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