From 615f726e7fbdbc8510f19d7714471d70beae2bc2 Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Mon, 5 Jul 2021 17:05:18 +0530 Subject: [PATCH 1/3] Better error message for unsupported double values --- .../sql/calcite/planner/DruidRexExecutor.java | 17 ++++++++++++++ .../druid/sql/calcite/CalciteQueryTest.java | 22 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java index c181cb8c885a..f06075867e61 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java +++ b/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", + expression, + expression, + expression); + } bigDecimal = BigDecimal.valueOf(exprResult.asDouble()); } literal = rexBuilder.makeLiteral(bigDecimal, constExp.getType(), true); diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java index 2c4c87e3fd90..6145426ce894 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java @@ -283,6 +283,28 @@ public void testSelectConstantExpressionFromTable() throws Exception ); } + @Test + public void testSelectConstantExpressionEquivalentToNaN() throws Exception + { + expectedException.expectMessage("'(log10(0) - log10(0))' evaluates to 'NaN' that is not supported. You can either cast the expression as bigint ('cast((log10(0) - log10(0)) as bigint)') or char ('cast((log10(0) - log10(0)) as char)') or change the expression itself"); + testQuery( + "SELECT log10(0) - log10(0), dim1 FROM foo LIMIT 1", + ImmutableList.of(), + ImmutableList.of() + ); + } + + @Test + public void testSelectConstantExpressionEquivalentToInfinity() throws Exception + { + expectedException.expectMessage("'log10(0)' evaluates to '+/-Infinity' that is not supported. You can either cast the expression as bigint ('cast(log10(0) as bigint)') or char ('cast(log10(0) as char)') or change the expression itself"); + testQuery( + "SELECT log10(0), dim1 FROM foo LIMIT 1", + ImmutableList.of(), + ImmutableList.of() + ); + } + @Test public void testGroupByWithPostAggregatorReferencingTimeFloorColumnOnTimeseries() throws Exception { From 59e49de32c1e797a32189ca8427aa7372744120f Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Mon, 5 Jul 2021 18:47:57 +0530 Subject: [PATCH 2/3] Review comments --- .../sql/calcite/planner/DruidRexExecutor.java | 14 +++----------- .../apache/druid/sql/calcite/CalciteQueryTest.java | 2 +- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java index f06075867e61..ae0cffdfcffd 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java @@ -127,19 +127,11 @@ public void reduce( // 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)) - { + if (Double.isNaN(exprResultDouble) || Double.isInfinite(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", + 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", expression, + Double.toString(exprResultDouble), expression, expression); } diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java index 6145426ce894..4fd23b17e269 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java @@ -297,7 +297,7 @@ public void testSelectConstantExpressionEquivalentToNaN() throws Exception @Test public void testSelectConstantExpressionEquivalentToInfinity() throws Exception { - expectedException.expectMessage("'log10(0)' evaluates to '+/-Infinity' that is not supported. You can either cast the expression as bigint ('cast(log10(0) as bigint)') or char ('cast(log10(0) as char)') or change the expression itself"); + expectedException.expectMessage("'log10(0)' evaluates to '-Infinity' that is not supported. You can either cast the expression as bigint ('cast(log10(0) as bigint)') or char ('cast(log10(0) as char)') or change the expression itself"); testQuery( "SELECT log10(0), dim1 FROM foo LIMIT 1", ImmutableList.of(), From 27c724e135a07e5aa1920be69248b7878d29aad0 Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Wed, 7 Jul 2021 11:37:36 +0530 Subject: [PATCH 3/3] Update error message --- .../apache/druid/sql/calcite/planner/DruidRexExecutor.java | 2 +- .../java/org/apache/druid/sql/calcite/CalciteQueryTest.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java index ae0cffdfcffd..0f1988a9b241 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java @@ -129,7 +129,7 @@ public void reduce( 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", + throw new IAE("'%s' evaluates to '%s' that is not supported in SQL. You can either cast the expression as bigint ('cast(%s as bigint)') or char ('cast(%s as char)') or change the expression itself", expression, Double.toString(exprResultDouble), expression, diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java index 4fd23b17e269..7796018b6a9f 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java @@ -286,7 +286,7 @@ public void testSelectConstantExpressionFromTable() throws Exception @Test public void testSelectConstantExpressionEquivalentToNaN() throws Exception { - expectedException.expectMessage("'(log10(0) - log10(0))' evaluates to 'NaN' that is not supported. You can either cast the expression as bigint ('cast((log10(0) - log10(0)) as bigint)') or char ('cast((log10(0) - log10(0)) as char)') or change the expression itself"); + expectedException.expectMessage("'(log10(0) - log10(0))' evaluates to 'NaN' that is not supported in SQL. You can either cast the expression as bigint ('cast((log10(0) - log10(0)) as bigint)') or char ('cast((log10(0) - log10(0)) as char)') or change the expression itself"); testQuery( "SELECT log10(0) - log10(0), dim1 FROM foo LIMIT 1", ImmutableList.of(), @@ -297,7 +297,7 @@ public void testSelectConstantExpressionEquivalentToNaN() throws Exception @Test public void testSelectConstantExpressionEquivalentToInfinity() throws Exception { - expectedException.expectMessage("'log10(0)' evaluates to '-Infinity' that is not supported. You can either cast the expression as bigint ('cast(log10(0) as bigint)') or char ('cast(log10(0) as char)') or change the expression itself"); + expectedException.expectMessage("'log10(0)' evaluates to '-Infinity' that is not supported in SQL. You can either cast the expression as bigint ('cast(log10(0) as bigint)') or char ('cast(log10(0) as char)') or change the expression itself"); testQuery( "SELECT log10(0), dim1 FROM foo LIMIT 1", ImmutableList.of(),