From 4932be3eb7c968eddeec01788152070975e60a67 Mon Sep 17 00:00:00 2001 From: Fang Xing <155562079+fang-xing-esql@users.noreply.github.com> Date: Wed, 6 Nov 2024 15:23:53 -0500 Subject: [PATCH] [ES|QL] Verify aggregation filter's type is boolean to avoid class_cast_exception (#116274) * validate agg filter's type is boolean (cherry picked from commit 0e044d70db20d67e0bc43d7671b4cfcffbcc4bae) --- docs/changelog/116274.yaml | 5 +++++ .../elasticsearch/xpack/esql/analysis/Verifier.java | 5 +++++ .../xpack/esql/analysis/VerifierTests.java | 13 +++++++++++++ 3 files changed, 23 insertions(+) create mode 100644 docs/changelog/116274.yaml diff --git a/docs/changelog/116274.yaml b/docs/changelog/116274.yaml new file mode 100644 index 0000000000000..9d506c7725afd --- /dev/null +++ b/docs/changelog/116274.yaml @@ -0,0 +1,5 @@ +pr: 116274 +summary: "[ES|QL] Verify aggregation filter's type is boolean to avoid `class_cast_exception`" +area: ES|QL +type: bug +issues: [] diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java index 01af8adbfca67..0fa2a69438358 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java @@ -69,6 +69,7 @@ import static org.elasticsearch.xpack.esql.common.Failure.fail; import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.FIRST; import static org.elasticsearch.xpack.esql.core.type.DataType.BOOLEAN; +import static org.elasticsearch.xpack.esql.core.type.DataType.NULL; /** * This class is part of the planner. Responsible for failing impossible queries with a human-readable error message. In particular, this @@ -315,6 +316,10 @@ private static void checkInvalidNamedExpressionUsage( Expression filter = fe.filter(); failures.add(fail(filter, "WHERE clause allowed only for aggregate functions, none found in [{}]", fe.sourceText())); } + Expression f = fe.filter(); // check the filter has to be a boolean term, similar as checkFilterConditionType + if (f.dataType() != NULL && f.dataType() != BOOLEAN) { + failures.add(fail(f, "Condition expression needs to be boolean, found [{}]", f.dataType())); + } // but that the filter doesn't use grouping or aggregate functions fe.filter().forEachDown(c -> { if (c instanceof AggregateFunction af) { diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java index f3de74ff97b4c..759a0b914f1d7 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java @@ -394,6 +394,19 @@ public void testAggFilterOnBucketingOrAggFunctions() { assertEquals("1:60: Unknown column [m]", error("from test | stats m = max(languages), min(languages) WHERE m + 2 > 1 by emp_no")); } + public void testAggWithNonBooleanFilter() { + for (String filter : List.of("\"true\"", "1", "1 + 0", "concat(\"a\", \"b\")")) { + String type = (filter.equals("1") || filter.equals("1 + 0")) ? "INTEGER" : "KEYWORD"; + assertEquals("1:19: Condition expression needs to be boolean, found [" + type + "]", error("from test | where " + filter)); + for (String by : List.of("", " by languages", " by bucket(salary, 10)")) { + assertEquals( + "1:34: Condition expression needs to be boolean, found [" + type + "]", + error("from test | stats count(*) where " + filter + by) + ); + } + } + } + public void testGroupingInsideAggsAsAgg() { assertEquals( "1:18: can only use grouping function [bucket(emp_no, 5.)] part of the BY clause",