SQL: Improve handling of invalid args for PERCENTILE/PERCENTILE_RANK (#37803)

matriv · matriv · commit 1dde748a5095 · 2019-01-24T15:16:01.000+02:00
Improve the Exception and the error message returned when 2nd argument
of PERCENTILE and PERCENTILE_RANK is not a constant.
diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Percentile.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Percentile.java
@@ -5,7 +5,6 @@
  */
 package org.elasticsearch.xpack.sql.expression.function.aggregate;
 
-import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
 import org.elasticsearch.xpack.sql.expression.Expression;
 import org.elasticsearch.xpack.sql.expression.Expressions;
 import org.elasticsearch.xpack.sql.expression.Expressions.ParamOrdinal;
@@ -17,6 +16,7 @@
 import java.util.List;
 
 import static java.util.Collections.singletonList;
+import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
 
 public class Percentile extends NumericAggregate implements EnclosedAgg {
 
@@ -43,8 +43,8 @@ public Percentile replaceChildren(List<Expression> newChildren) {
     @Override
     protected TypeResolution resolveType() {
         if (!percent.foldable()) {
-            throw new SqlIllegalArgumentException("2nd argument of PERCENTILE must be constant, received [{}]",
-                Expressions.name(percent));
+            return new TypeResolution(format(null, "2nd argument of PERCENTILE must be a constant, received [{}]",
+                Expressions.name(percent)));
         }
 
         TypeResolution resolution = super.resolveType();
diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/PercentileRank.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/PercentileRank.java
@@ -5,7 +5,6 @@
  */
 package org.elasticsearch.xpack.sql.expression.function.aggregate;
 
-import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
 import org.elasticsearch.xpack.sql.expression.Expression;
 import org.elasticsearch.xpack.sql.expression.Expressions;
 import org.elasticsearch.xpack.sql.expression.Expressions.ParamOrdinal;
@@ -17,6 +16,7 @@
 import java.util.List;
 
 import static java.util.Collections.singletonList;
+import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
 
 public class PercentileRank extends AggregateFunction implements EnclosedAgg {
 
@@ -43,8 +43,8 @@ public Expression replaceChildren(List<Expression> newChildren) {
     @Override
     protected TypeResolution resolveType() {
         if (!value.foldable()) {
-            throw new SqlIllegalArgumentException("2nd argument of PERCENTILE_RANK must be constant, received [{}]",
-                Expressions.name(value));
+            return new TypeResolution(format(null, "2nd argument of PERCENTILE_RANK must be a constant, received [{}]",
+                Expressions.name(value)));
         }
 
         TypeResolution resolution = super.resolveType();
diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java
@@ -6,7 +6,6 @@
 package org.elasticsearch.xpack.sql.analysis.analyzer;
 
 import org.elasticsearch.test.ESTestCase;
-import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
 import org.elasticsearch.xpack.sql.TestUtils;
 import org.elasticsearch.xpack.sql.analysis.AnalysisException;
 import org.elasticsearch.xpack.sql.analysis.index.EsIndex;
@@ -539,19 +538,13 @@ public void testAggsInHistogram() {
     }
 
     public void testErrorMessageForPercentileWithSecondArgBasedOnAField() {
-        Analyzer analyzer = new Analyzer(TestUtils.TEST_CFG, new FunctionRegistry(), indexResolution, new Verifier(new Metrics()));
-        SqlIllegalArgumentException e = expectThrows(SqlIllegalArgumentException.class, () -> analyzer.analyze(parser.createStatement(
-            "SELECT PERCENTILE(int, ABS(int)) FROM test"), true));
-        assertEquals("2nd argument of PERCENTILE must be constant, received [ABS(int)]",
-            e.getMessage());
+        assertEquals("1:8: 2nd argument of PERCENTILE must be a constant, received [ABS(int)]",
+            error("SELECT PERCENTILE(int, ABS(int)) FROM test"));
     }
 
     public void testErrorMessageForPercentileRankWithSecondArgBasedOnAField() {
-        Analyzer analyzer = new Analyzer(TestUtils.TEST_CFG, new FunctionRegistry(), indexResolution, new Verifier(new Metrics()));
-        SqlIllegalArgumentException e = expectThrows(SqlIllegalArgumentException.class, () -> analyzer.analyze(parser.createStatement(
-            "SELECT PERCENTILE_RANK(int, ABS(int)) FROM test"), true));
-        assertEquals("2nd argument of PERCENTILE_RANK must be constant, received [ABS(int)]",
-            e.getMessage());
+        assertEquals("1:8: 2nd argument of PERCENTILE_RANK must be a constant, received [ABS(int)]",
+            error("SELECT PERCENTILE_RANK(int, ABS(int)) FROM test"));
     }
 }
 
