From 0d006905497e3ca824707307863421fc92c6aabb Mon Sep 17 00:00:00 2001
From: Marios Trivyzas <matriv@gmail.com>
Date: Thu, 24 Jan 2019 15:03:49 +0200
Subject: [PATCH] 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.
---
 .../expression/function/aggregate/Percentile.java    |  6 ++++++
 .../function/aggregate/PercentileRank.java           |  6 ++++++
 .../analyzer/VerifierErrorMessagesTests.java         | 12 +++++++++++-
 3 files changed, 23 insertions(+), 1 deletion(-)

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
index 6e644fb4f751c..c0118e96833f2 100644
--- 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
@@ -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/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
index f01dad8800ccf..41a3d51e50e6e 100644
--- 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
@@ -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/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
index 711ba32bdffa1..3768ef105b2de 100644
--- 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
@@ -366,4 +366,14 @@ public void testInvalidTypeForFunction_WithFourArgs() {
         assertEquals("1:8: [INSERT] fourth argument must be [string], found value [3] type [integer]",
             error("SELECT INSERT('text', 1, 2, 3)"));
     }
-}
\ No newline at end of file
+
+    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"));
+    }
+}