Skip to content

Commit 7444595

Browse files
ivanceaivancea
authored
[8.x] ESQL: Fail in AggregateFunction when LogicPlan is not an Aggregate (elastic#125400)
Manual 8.x backport of elastic#124446, with support for Metrics aggs and removed Dedup support
1 parent 271b04f commit 7444595

File tree

4 files changed

+73
-23
lines changed

4 files changed

+73
-23
lines changed

‎docs/changelog/124446.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 124446
2+
summary: "ESQL: Fail in `AggregateFunction` when `LogicPlan` is not an `Aggregate`"
3+
area: ES|QL
4+
type: bug
5+
issues:
6+
- 124311

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919
import org.elasticsearch.xpack.esql.core.tree.Source;
2020
import org.elasticsearch.xpack.esql.core.util.CollectionUtils;
2121
import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput;
22+
import org.elasticsearch.xpack.esql.plan.logical.Aggregate;
2223
import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
23-
import org.elasticsearch.xpack.esql.plan.logical.OrderBy;
2424

2525
import java.io.IOException;
2626
import java.util.List;
@@ -139,14 +139,14 @@ public boolean equals(Object obj) {
139139
@Override
140140
public BiConsumer<LogicalPlan, Failures> postAnalysisPlanVerification() {
141141
return (p, failures) -> {
142-
if (p instanceof OrderBy order) {
143-
order.order().forEach(o -> {
144-
o.forEachDown(Function.class, f -> {
145-
if (f instanceof AggregateFunction) {
146-
failures.add(fail(f, "Aggregate functions are not allowed in SORT [{}]", f.functionName()));
147-
}
148-
});
149-
});
142+
if ((p instanceof Aggregate) == false) {
143+
p.expressions().forEach(x -> x.forEachDown(AggregateFunction.class, af -> {
144+
if (af instanceof Rate) {
145+
failures.add(fail(af, "aggregate function [{}] not allowed outside METRICS command", af.sourceText()));
146+
} else {
147+
failures.add(fail(af, "aggregate function [{}] not allowed outside STATS command", af.sourceText()));
148+
}
149+
}));
150150
}
151151
};
152152
}

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Eval.java‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@
2424
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
2525
import org.elasticsearch.xpack.esql.core.tree.Source;
2626
import org.elasticsearch.xpack.esql.core.type.DataType;
27-
import org.elasticsearch.xpack.esql.expression.function.aggregate.AggregateFunction;
28-
import org.elasticsearch.xpack.esql.expression.function.aggregate.Rate;
2927
import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput;
3028
import org.elasticsearch.xpack.esql.plan.GeneratingPlan;
3129

@@ -179,14 +177,6 @@ public void postAnalysisVerification(Failures failures) {
179177
)
180178
);
181179
}
182-
// check no aggregate functions are used
183-
field.forEachDown(AggregateFunction.class, af -> {
184-
if (af instanceof Rate) {
185-
failures.add(fail(af, "aggregate function [{}] not allowed outside METRICS command", af.sourceText()));
186-
} else {
187-
failures.add(fail(af, "aggregate function [{}] not allowed outside STATS command", af.sourceText()));
188-
}
189-
});
190180
});
191181
}
192182
}

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

Lines changed: 58 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -978,6 +978,17 @@ public void testNotFoundFieldInNestedFunction() {
978978
line 1:23: Unknown column [avg]""", error("from test | stats c = avg by missing + 1, not_found"));
979979
}
980980

981+
public void testMultipleAggsOutsideStats() {
982+
assertEquals(
983+
"""
984+
1:71: aggregate function [avg(salary)] not allowed outside STATS command
985+
line 1:96: aggregate function [median(emp_no)] not allowed outside STATS command
986+
line 1:22: aggregate function [sum(salary)] not allowed outside STATS command
987+
line 1:39: aggregate function [avg(languages)] not allowed outside STATS command""",
988+
error("from test | eval s = sum(salary), l = avg(languages) | where salary > avg(salary) and emp_no > median(emp_no)")
989+
);
990+
}
991+
981992
public void testSpatialSort() {
982993
String prefix = "ROW wkt = [\"POINT(42.9711 -14.7553)\", \"POINT(75.8093 22.7277)\"] | MV_EXPAND wkt ";
983994
assertEquals("1:130: cannot sort on geo_point", error(prefix + "| EVAL shape = TO_GEOPOINT(wkt) | limit 5 | sort shape"));
@@ -2024,10 +2035,53 @@ public void testCategorizeWithFilteredAggregations() {
20242035
}
20252036

20262037
public void testSortByAggregate() {
2027-
assertEquals("1:18: Aggregate functions are not allowed in SORT [COUNT]", error("ROW a = 1 | SORT count(*)"));
2028-
assertEquals("1:28: Aggregate functions are not allowed in SORT [COUNT]", error("ROW a = 1 | SORT to_string(count(*))"));
2029-
assertEquals("1:22: Aggregate functions are not allowed in SORT [MAX]", error("ROW a = 1 | SORT 1 + max(a)"));
2030-
assertEquals("1:18: Aggregate functions are not allowed in SORT [COUNT]", error("FROM test | SORT count(*)"));
2038+
assertEquals("1:18: aggregate function [count(*)] not allowed outside STATS command", error("ROW a = 1 | SORT count(*)"));
2039+
assertEquals(
2040+
"1:28: aggregate function [count(*)] not allowed outside STATS command",
2041+
error("ROW a = 1 | SORT to_string(count(*))")
2042+
);
2043+
assertEquals("1:22: aggregate function [max(a)] not allowed outside STATS command", error("ROW a = 1 | SORT 1 + max(a)"));
2044+
assertEquals("1:18: aggregate function [count(*)] not allowed outside STATS command", error("FROM test | SORT count(*)"));
2045+
}
2046+
2047+
public void testFilterByAggregate() {
2048+
assertEquals("1:19: aggregate function [count(*)] not allowed outside STATS command", error("ROW a = 1 | WHERE count(*) > 0"));
2049+
assertEquals(
2050+
"1:29: aggregate function [count(*)] not allowed outside STATS command",
2051+
error("ROW a = 1 | WHERE to_string(count(*)) IS NOT NULL")
2052+
);
2053+
assertEquals("1:23: aggregate function [max(a)] not allowed outside STATS command", error("ROW a = 1 | WHERE 1 + max(a) > 0"));
2054+
assertEquals(
2055+
"1:24: aggregate function [min(languages)] not allowed outside STATS command",
2056+
error("FROM employees | WHERE min(languages) > 2")
2057+
);
2058+
}
2059+
2060+
public void testDissectByAggregate() {
2061+
assertEquals(
2062+
"1:21: aggregate function [min(first_name)] not allowed outside STATS command",
2063+
error("from test | dissect min(first_name) \"%{foo}\"")
2064+
);
2065+
assertEquals(
2066+
"1:21: aggregate function [avg(salary)] not allowed outside STATS command",
2067+
error("from test | dissect avg(salary) \"%{foo}\"")
2068+
);
2069+
}
2070+
2071+
public void testGrokByAggregate() {
2072+
assertEquals(
2073+
"1:18: aggregate function [max(last_name)] not allowed outside STATS command",
2074+
error("from test | grok max(last_name) \"%{WORD:foo}\"")
2075+
);
2076+
assertEquals(
2077+
"1:18: aggregate function [sum(salary)] not allowed outside STATS command",
2078+
error("from test | grok sum(salary) \"%{WORD:foo}\"")
2079+
);
2080+
}
2081+
2082+
public void testAggregateInRow() {
2083+
assertEquals("1:13: aggregate function [count(*)] not allowed outside STATS command", error("ROW a = 1 + count(*)"));
2084+
assertEquals("1:9: aggregate function [avg(2)] not allowed outside STATS command", error("ROW a = avg(2)"));
20312085
}
20322086

20332087
public void testLookupJoinDataTypeMismatch() {

0 commit comments

Comments
 (0)