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

…37803) Improve the Exception and the error message returned when 2nd argument of PERCENTILE and PERCENTILE_RANK is not a constant.
elastic · Jan 24, 2019 · 529f3ec · 529f3ec
1 parent a9861f4
commit 529f3ec
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 1 deletion.
diff --git a/...l/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Percentile.java b/...l/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Percentile.java
@@ -16,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 {
 
@@ -41,6 +42,11 @@ public Percentile replaceChildren(List<Expression> newChildren) {
 
     @Override
     protected TypeResolution resolveType() {
+        if (!percent.foldable()) {
+            return new TypeResolution(format(null, "2nd argument of PERCENTILE must be a constant, received [{}]",
+                Expressions.name(percent)));
+        }
+
         TypeResolution resolution = super.resolveType();
 
         if (TypeResolution.TYPE_RESOLVED.equals(resolution)) {

diff --git a/...c/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/PercentileRank.java b/...c/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/PercentileRank.java
@@ -16,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 {
 
@@ -41,6 +42,11 @@ public Expression replaceChildren(List<Expression> newChildren) {
 
     @Override
     protected TypeResolution resolveType() {
+        if (!value.foldable()) {
+            return new TypeResolution(format(null, "2nd argument of PERCENTILE_RANK must be a constant, received [{}]",
+                Expressions.name(value)));
+        }
+
         TypeResolution resolution = super.resolveType();
         if (resolution.unresolved()) {
             return resolution;

diff --git a/...c/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java b/...c/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java
@@ -529,4 +529,14 @@ public void testAggsInHistogram() {
         assertEquals("1:47: Cannot use an aggregate [MAX] for grouping",
                 error("SELECT MAX(date) FROM test GROUP BY HISTOGRAM(MAX(int), 1)"));
     }
-}
+
+    public void testErrorMessageForPercentileWithSecondArgBasedOnAField() {
+        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() {
+        assertEquals("1:8: 2nd argument of PERCENTILE_RANK must be a constant, received [ABS(int)]",
+            error("SELECT PERCENTILE_RANK(int, ABS(int)) FROM test"));
+    }
+}