You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/07/05 11:48:33 UTC
[GitHub] [druid] abhishekagarwal87 opened a new pull request #11409: Better error message for unsupported double values
abhishekagarwal87 opened a new pull request #11409:
URL: https://github.com/apache/druid/pull/11409
A constant expression may evaluate to Double.NEGATIVE_INFINITY/Double.POSITIVE_INFINITY/Double.NAN e.g. `log10(0)`. When using such an expression in native queries, the user will get the corresponding value without any error. In SQL, however, the user will run into `NumberFormatException` because we convert the double to big-decimal while constructing a literal numeric expression. This probably should be fixed in calcite - see https://issues.apache.org/jira/browse/CALCITE-2067. This PR adds a verbose error message so that users can take corrective action without scratching their heads.
<hr>
##### Key changed/added classes in this PR
* `DruidRexExecutor`
<hr>
<!-- Check the items by putting "x" in the brackets for the done things. Not all of these items apply to every PR. Remove the items which are not done or not relevant to the PR. None of the items from the checklist below are strictly necessary, but it would be very helpful if you at least self-review the PR. -->
This PR has:
- [ ] been self-reviewed.
- [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
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
[GitHub] [druid] rohangarg commented on a change in pull request #11409: Better error message for unsupported double values
Posted by GitBox <gi...@apache.org>.
rohangarg commented on a change in pull request #11409:
URL: https://github.com/apache/druid/pull/11409#discussion_r663920216
##########
File path: sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java
##########
@@ -126,6 +126,23 @@ public void reduce(
// if exprResult evaluates to Nan or infinity, this will throw a NumberFormatException.
// If you find yourself in such a position, consider casting the literal to a BIGINT so that
// the query can execute.
+ double exprResultDouble = exprResult.asDouble();
+ if (Double.isNaN(exprResultDouble))
+ {
+ String expression = druidExpression.getExpression();
+ throw new IAE("'%s' evaluates to 'NaN' that is not supported. You can either cast the expression as bigint ('cast(%s as bigint)') or char ('cast(%s as char)') or change the expression itself",
+ expression,
+ expression,
+ expression);
+ }
+ if (Double.isInfinite(exprResultDouble))
+ {
+ String expression = druidExpression.getExpression();
+ throw new IAE("'%s' evaluates to '+/-Infinity' that is not supported. You can either cast the expression as bigint ('cast(%s as bigint)') or char ('cast(%s as char)') or change the expression itself",
Review comment:
I think that `toString` for `exprResultDouble` does evaluate as `+/-INFINITY` or `NaN` - so maybe that can be directly used when `Double.isNaN(exprResultDouble) || Double.isInfinite(exprResultDouble)`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
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
[GitHub] [druid] abhishekagarwal87 commented on a change in pull request #11409: Better error message for unsupported double values
Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on a change in pull request #11409:
URL: https://github.com/apache/druid/pull/11409#discussion_r663869282
##########
File path: sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java
##########
@@ -126,6 +126,23 @@ public void reduce(
// if exprResult evaluates to Nan or infinity, this will throw a NumberFormatException.
// If you find yourself in such a position, consider casting the literal to a BIGINT so that
// the query can execute.
+ double exprResultDouble = exprResult.asDouble();
+ if (Double.isNaN(exprResultDouble))
+ {
+ String expression = druidExpression.getExpression();
+ throw new IAE("'%s' evaluates to 'NaN' that is not supported. You can either cast the expression as bigint ('cast(%s as bigint)') or char ('cast(%s as char)') or change the expression itself",
Review comment:
wondering if using a `NumberFormatException` instead of `IAE` makes more sense here.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
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
[GitHub] [druid] rohangarg commented on a change in pull request #11409: Better error message for unsupported double values
Posted by GitBox <gi...@apache.org>.
rohangarg commented on a change in pull request #11409:
URL: https://github.com/apache/druid/pull/11409#discussion_r663920216
##########
File path: sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java
##########
@@ -126,6 +126,23 @@ public void reduce(
// if exprResult evaluates to Nan or infinity, this will throw a NumberFormatException.
// If you find yourself in such a position, consider casting the literal to a BIGINT so that
// the query can execute.
+ double exprResultDouble = exprResult.asDouble();
+ if (Double.isNaN(exprResultDouble))
+ {
+ String expression = druidExpression.getExpression();
+ throw new IAE("'%s' evaluates to 'NaN' that is not supported. You can either cast the expression as bigint ('cast(%s as bigint)') or char ('cast(%s as char)') or change the expression itself",
+ expression,
+ expression,
+ expression);
+ }
+ if (Double.isInfinite(exprResultDouble))
+ {
+ String expression = druidExpression.getExpression();
+ throw new IAE("'%s' evaluates to '+/-Infinity' that is not supported. You can either cast the expression as bigint ('cast(%s as bigint)') or char ('cast(%s as char)') or change the expression itself",
Review comment:
I think that `toString` for `exprResultDouble` does evaluate as `+/-INFINITY` or `NaN` - so maybe that can be directly used when `Double.isNaN(exprResultDouble) || Double.isInfinite(exprResultDouble)`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
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
[GitHub] [druid] clintropolis commented on a change in pull request #11409: Better error message for unsupported double values
Posted by GitBox <gi...@apache.org>.
clintropolis commented on a change in pull request #11409:
URL: https://github.com/apache/druid/pull/11409#discussion_r665037572
##########
File path: sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java
##########
@@ -126,6 +126,15 @@ public void reduce(
// if exprResult evaluates to Nan or infinity, this will throw a NumberFormatException.
// If you find yourself in such a position, consider casting the literal to a BIGINT so that
// the query can execute.
+ double exprResultDouble = exprResult.asDouble();
+ if (Double.isNaN(exprResultDouble) || Double.isInfinite(exprResultDouble)) {
+ String expression = druidExpression.getExpression();
+ throw new IAE("'%s' evaluates to '%s' that is not supported. You can either cast the expression as bigint ('cast(%s as bigint)') or char ('cast(%s as char)') or change the expression itself",
Review comment:
i wonder if it is worth clarify that the not supported part is specific to SQL
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
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
[GitHub] [druid] abhishekagarwal87 commented on a change in pull request #11409: Better error message for unsupported double values
Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on a change in pull request #11409:
URL: https://github.com/apache/druid/pull/11409#discussion_r665069339
##########
File path: sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java
##########
@@ -126,6 +126,15 @@ public void reduce(
// if exprResult evaluates to Nan or infinity, this will throw a NumberFormatException.
// If you find yourself in such a position, consider casting the literal to a BIGINT so that
// the query can execute.
+ double exprResultDouble = exprResult.asDouble();
+ if (Double.isNaN(exprResultDouble) || Double.isInfinite(exprResultDouble)) {
+ String expression = druidExpression.getExpression();
+ throw new IAE("'%s' evaluates to '%s' that is not supported. You can either cast the expression as bigint ('cast(%s as bigint)') or char ('cast(%s as char)') or change the expression itself",
Review comment:
done
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
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
[GitHub] [druid] abhishekagarwal87 merged pull request #11409: Better error message for unsupported double values
Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 merged pull request #11409:
URL: https://github.com/apache/druid/pull/11409
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
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
[GitHub] [druid] abhishekagarwal87 commented on a change in pull request #11409: Better error message for unsupported double values
Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on a change in pull request #11409:
URL: https://github.com/apache/druid/pull/11409#discussion_r663869282
##########
File path: sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java
##########
@@ -126,6 +126,23 @@ public void reduce(
// if exprResult evaluates to Nan or infinity, this will throw a NumberFormatException.
// If you find yourself in such a position, consider casting the literal to a BIGINT so that
// the query can execute.
+ double exprResultDouble = exprResult.asDouble();
+ if (Double.isNaN(exprResultDouble))
+ {
+ String expression = druidExpression.getExpression();
+ throw new IAE("'%s' evaluates to 'NaN' that is not supported. You can either cast the expression as bigint ('cast(%s as bigint)') or char ('cast(%s as char)') or change the expression itself",
Review comment:
wondering if using a `NumberFormatException` instead of `IAE` makes more sense here.
##########
File path: sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java
##########
@@ -126,6 +126,23 @@ public void reduce(
// if exprResult evaluates to Nan or infinity, this will throw a NumberFormatException.
// If you find yourself in such a position, consider casting the literal to a BIGINT so that
// the query can execute.
+ double exprResultDouble = exprResult.asDouble();
+ if (Double.isNaN(exprResultDouble))
+ {
+ String expression = druidExpression.getExpression();
+ throw new IAE("'%s' evaluates to 'NaN' that is not supported. You can either cast the expression as bigint ('cast(%s as bigint)') or char ('cast(%s as char)') or change the expression itself",
+ expression,
+ expression,
+ expression);
+ }
+ if (Double.isInfinite(exprResultDouble))
+ {
+ String expression = druidExpression.getExpression();
+ throw new IAE("'%s' evaluates to '+/-Infinity' that is not supported. You can either cast the expression as bigint ('cast(%s as bigint)') or char ('cast(%s as char)') or change the expression itself",
Review comment:
👍 good tip.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
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
[GitHub] [druid] abhishekagarwal87 commented on a change in pull request #11409: Better error message for unsupported double values
Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on a change in pull request #11409:
URL: https://github.com/apache/druid/pull/11409#discussion_r663924343
##########
File path: sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java
##########
@@ -126,6 +126,23 @@ public void reduce(
// if exprResult evaluates to Nan or infinity, this will throw a NumberFormatException.
// If you find yourself in such a position, consider casting the literal to a BIGINT so that
// the query can execute.
+ double exprResultDouble = exprResult.asDouble();
+ if (Double.isNaN(exprResultDouble))
+ {
+ String expression = druidExpression.getExpression();
+ throw new IAE("'%s' evaluates to 'NaN' that is not supported. You can either cast the expression as bigint ('cast(%s as bigint)') or char ('cast(%s as char)') or change the expression itself",
+ expression,
+ expression,
+ expression);
+ }
+ if (Double.isInfinite(exprResultDouble))
+ {
+ String expression = druidExpression.getExpression();
+ throw new IAE("'%s' evaluates to '+/-Infinity' that is not supported. You can either cast the expression as bigint ('cast(%s as bigint)') or char ('cast(%s as char)') or change the expression itself",
Review comment:
👍 good tip.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
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