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

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

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



##########
File path: core/src/main/java/org/apache/druid/math/expr/Function.java
##########
@@ -737,7 +745,12 @@ private ExprEval eval(ExprEval param, int scale)
       if (param.type() == ExprType.LONG) {
         return ExprEval.of(BigDecimal.valueOf(param.asLong()).setScale(scale, RoundingMode.HALF_UP).longValue());
       } else if (param.type() == ExprType.DOUBLE) {
-        return ExprEval.of(BigDecimal.valueOf(param.asDouble()).setScale(scale, RoundingMode.HALF_UP).doubleValue());
+        double val = param.asDouble();
+        if (Double.isNaN(val) || val == Double.POSITIVE_INFINITY || val == Double.NEGATIVE_INFINITY) {
+          // This is the behavior of Math.round()
+          return ExprEval.of(0L);

Review comment:
       I wasn't sure of the perf implications of calling `Math.round` It looks like it calls out to some native function, does some bit magic and then finally just casts the object to a long - which is how we get `0L`
   
   So I thought it would be better to just return 0 directly




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