maytasm commented on a change in pull request #10014:
URL: https://github.com/apache/druid/pull/10014#discussion_r439132322
##########
File path: docs/misc/math-expr.md
##########
@@ -141,7 +141,7 @@ See javadoc of java.lang.Math for detailed explanation for
each function.
|pow|pow(x, y) would return the value of the x raised to the power of y|
|remainder|remainder(x, y) would return the remainder operation on two
arguments as prescribed by the IEEE 754 standard|
|rint|rint(x) would return value that is closest in value to x and is equal to
a mathematical integer|
-|round|round(x, y) would return the value of the x rounded to the y decimal
places. While x can be an integer or floating-point number, y must be an
integer. The type of the return value is specified by that of x. y defaults to
0 if omitted. When y is negative, x is rounded on the left side of the y
decimal points.|
+|round|round(x, y) would return the value of the x rounded to the y decimal
places. While x can be an integer or floating-point number, y must be an
integer. The type of the return value is specified by that of x. y defaults to
0 if omitted. When y is negative, x is rounded on the left side of the y
decimal points. If x is either `NaN` or infinity, this will return 0. |
Review comment:
explicitly mention positive and negative infinity
##########
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:
Should we use Math.round(val) here instead of hard-code to 0?
##########
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:
Wondering what is your thought on making this reduce to value of 0 (same
as the other stuff)
##########
File path: docs/querying/sql.md
##########
@@ -287,7 +287,7 @@ to FLOAT. At runtime, Druid will widen 32-bit floats to
64-bit for most expressi
|`SQRT(expr)`|Square root.|
|`TRUNCATE(expr[, digits])`|Truncate expr to a specific number of decimal
digits. If digits is negative, then this truncates that many places to the left
of the decimal point. Digits defaults to zero if not specified.|
|`TRUNC(expr[, digits])`|Synonym for `TRUNCATE`.|
-|`ROUND(expr[, digits])`|`ROUND(x, y)` would return the value of the x rounded
to the y decimal places. While x can be an integer or floating-point number, y
must be an integer. The type of the return value is specified by that of x. y
defaults to 0 if omitted. When y is negative, x is rounded on the left side of
the y decimal points.|
+|`ROUND(expr[, digits])`|`ROUND(x, y)` would return the value of the x rounded
to the y decimal places. While x can be an integer or floating-point number, y
must be an integer. The type of the return value is specified by that of x. y
defaults to 0 if omitted. When y is negative, x is rounded on the left side of
the y decimal points. If `expr` evaluates to either `NaN` or infinity, this
will return 0. |
Review comment:
explicitly mention positive and negative infinity
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
