Skip to content

Commit 12bcc4f

Browse files
ivanceaivancea
authored
[9.0] ESQL: Fail in AggregateFunction when LogicPlan is not an Aggregate (elastic#125402)
Manual 9.0 backport of elastic#124446, with support for Metrics aggs and removed Dedup support
1 parent b403600 commit 12bcc4f

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
@@ -1005,6 +1005,17 @@ public void testNotFoundFieldInNestedFunction() {
10051005
line 1:23: Unknown column [avg]""", error("from test | stats c = avg by missing + 1, not_found"));
10061006
}
10071007

1008+
public void testMultipleAggsOutsideStats() {
1009+
assertEquals(
1010+
"""
1011+
1:71: aggregate function [avg(salary)] not allowed outside STATS command
1012+
line 1:96: aggregate function [median(emp_no)] not allowed outside STATS command
1013+
line 1:22: aggregate function [sum(salary)] not allowed outside STATS command
1014+
line 1:39: aggregate function [avg(languages)] not allowed outside STATS command""",
1015+
error("from test | eval s = sum(salary), l = avg(languages) | where salary > avg(salary) and emp_no > median(emp_no)")
1016+
);
1017+
}
1018+
10081019
public void testSpatialSort() {
10091020
String prefix = "ROW wkt = [\"POINT(42.9711 -14.7553)\", \"POINT(75.8093 22.7277)\"] | MV_EXPAND wkt ";
10101021
assertEquals("1:130: cannot sort on geo_point", error(prefix + "| EVAL shape = TO_GEOPOINT(wkt) | limit 5 | sort shape"));
@@ -2032,10 +2043,53 @@ public void testCategorizeWithFilteredAggregations() {
20322043
}
20332044

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

20412095
public void testLookupJoinDataTypeMismatch() {

0 commit comments

Comments
 (0)