ESQL: Improve agg verification (#99827)

When verifying aggregation expressions like
from employees | stats percentile(salary_change, 25*2)
, both arguments are treated the same way during verification. This is
incorrect, as salary_change is the actual aggregation's field, while
25*2 is merely it's (first and only) parameter. This is overly
restrictive.

Apply the current verification only to the aggregation's actual field,
as the parameter is already verified during type resolution (it needs to
be a constant expression).

docs/changelog/99827.yaml

@@ -0,0 +1,5 @@
+pr: 99827
+summary: "ESQL: Fix NPE when aggregating literals"
+area: ES|QL
+type: bug
+issues: []

x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats_count_distinct.csv-spec

@@ -117,7 +117,7 @@ m:long  | languages:i
 15   | 4
 20   | 5
 10   | null
-; 
+;
 
 countDistinctOfIpGroupByKeyword
 from hosts | stats h0 = count_distinct(ip0), h1 = count_distinct(ip1)  by host | sort host;
@@ -128,3 +128,27 @@ h0:long | h1:long | host:keyword
 5       | 6       | epsilon
 1       | 2       | gamma
 ;
+
+countDistinctWithPrecisionExpression
+from employees | stats m = count_distinct(height, 9875+1) by languages | sort languages;
+
+m:long  | languages:i
+13   | 1
+16   | 2
+14   | 3
+15   | 4
+20   | 5
+10   | null
+;
+
+countDistinctWithComplexPrecisionExpression
+from employees | stats m = count_distinct(height, 9876*3+(-9876*2)) by languages | sort languages;
+
+m:long  | languages:i
+13   | 1
+16   | 2
+14   | 3
+15   | 4
+20   | 5
+10   | null
+;

x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats_percentile.csv-spec

@@ -142,3 +142,17 @@ MEDIAN(salary):double | MEDIAN_ABSOLUTE_DEVIATION(salary):double
 47003                 | 10096.5
 // end::median-absolute-deviation-result[]
 ;
+
+medianViaExpression
+from employees | stats p50 = percentile(salary_change, 25*2);
+
+p50:double
+0.75
+;
+
+medianViaComplexExpression
+from employees | stats p50 = percentile(salary_change, -(50-1)+99);
+
+p50:double
+0.75
+;

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java

@@ -125,24 +125,24 @@ else if (p.resolved()) {
                 agg.aggregates().forEach(e -> {
                     var exp = e instanceof Alias ? ((Alias) e).child() : e;
                     if (exp instanceof AggregateFunction aggFunc) {
-                        aggFunc.arguments().forEach(a -> {
-                            // TODO: allow an expression?
-                            if ((a instanceof FieldAttribute
-                                || a instanceof MetadataAttribute
-                                || a instanceof ReferenceAttribute
-                                || a instanceof Literal) == false) {
-                                failures.add(
-                                    fail(
-                                        e,
-                                        "aggregate function's parameters must be an attribute or literal; found ["
-                                            + a.sourceText()
-                                            + "] of type ["
-                                            + a.nodeName()
-                                            + "]"
-                                    )
-                                );
-                            }
-                        });
+                        Expression field = aggFunc.field();
+
+                        // TODO: allow an expression?
+                        if ((field instanceof FieldAttribute
+                            || field instanceof MetadataAttribute
+                            || field instanceof ReferenceAttribute
+                            || field instanceof Literal) == false) {
+                            failures.add(
+                                fail(
+                                    e,
+                                    "aggregate function's field must be an attribute or literal; found ["
+                                        + field.sourceText()
+                                        + "] of type ["
+                                        + field.nodeName()
+                                        + "]"
+                                )
+                            );
+                        }
                     } else if (agg.groupings().contains(exp) == false) { // TODO: allow an expression?
                         failures.add(
                             fail(

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/CountDistinct.java

@@ -29,6 +29,7 @@
 
 import static org.elasticsearch.xpack.ql.expression.TypeResolutions.ParamOrdinal.DEFAULT;
 import static org.elasticsearch.xpack.ql.expression.TypeResolutions.ParamOrdinal.SECOND;
+import static org.elasticsearch.xpack.ql.expression.TypeResolutions.isFoldable;
 import static org.elasticsearch.xpack.ql.expression.TypeResolutions.isInteger;
 
 public class CountDistinct extends AggregateFunction implements OptionalArgument, ToAggregator {
@@ -66,7 +67,7 @@ protected TypeResolution resolveType() {
             return resolution;
         }
 
-        return isInteger(precision, sourceText(), SECOND);
+        return isInteger(precision, sourceText(), SECOND).and(isFoldable(precision, sourceText(), SECOND));
     }
 
     @Override

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Percentile.java

@@ -56,11 +56,7 @@ protected TypeResolution resolveType() {
             return resolution;
         }
 
-        resolution = isNumeric(percentile, sourceText(), SECOND);
-        if (resolution.unresolved()) {
-            return resolution;
-        }
-        return isFoldable(percentile, sourceText(), SECOND);
+        return isNumeric(percentile, sourceText(), SECOND).and(isFoldable(percentile, sourceText(), SECOND));
     }
 
     @Override

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java

@@ -68,21 +68,29 @@ public void testAggsExpressionsInStatsAggs() {
             error("from test | stats length(first_name), count(1) by first_name")
         );
         assertEquals(
-            "1:19: aggregate function's parameters must be an attribute or literal; found [emp_no / 2] of type [Div]",
+            "1:19: aggregate function's field must be an attribute or literal; found [emp_no / 2] of type [Div]",
             error("from test | stats x = avg(emp_no / 2) by emp_no")
         );
         assertEquals(
             "1:25: argument of [avg(first_name)] must be [numeric], found value [first_name] type [keyword]",
             error("from test | stats count(avg(first_name)) by first_name")
         );
         assertEquals(
-            "1:19: aggregate function's parameters must be an attribute or literal; found [length(first_name)] of type [Length]",
+            "1:19: aggregate function's field must be an attribute or literal; found [length(first_name)] of type [Length]",
             error("from test | stats count(length(first_name)) by first_name")
         );
         assertEquals(
             "1:23: expected an aggregate function or group but got [emp_no + avg(emp_no)] of type [Add]",
             error("from test | stats x = emp_no + avg(emp_no) by emp_no")
         );
+        assertEquals(
+            "1:23: second argument of [percentile(languages, languages)] must be a constant, received [languages]",
+            error("from test | stats x = percentile(languages, languages) by emp_no")
+        );
+        assertEquals(
+            "1:23: second argument of [count_distinct(languages, languages)] must be a constant, received [languages]",
+            error("from test | stats x = count_distinct(languages, languages) by emp_no")
+        );
     }
 
     public void testDoubleRenamingField() {

x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/Expression.java

@@ -50,6 +50,10 @@ public boolean resolved() {
             return failed == false;
         }
 
+        public TypeResolution and(TypeResolution other) {
+            return failed ? this : other;
+        }
+
         public String message() {
             return message;
         }