Skip to content

Commit 02c93e1

Browse files
alex-spiesalex-spies
authored
[8.13] ESQL: Fix fully pruned aggregates (#106673) (#107144)
Fix a bug where PruneColumns would sometimes completely replace an Aggregate, producing the wrong number of rows as a result. (cherry picked from commit edc9e67 and updated the test skips)
1 parent 4d62a32 commit 02c93e1

File tree

4 files changed

+183
-3
lines changed

4 files changed

+183
-3
lines changed

‎docs/changelog/106673.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 106673
2+
summary: "ESQL: Fix fully pruned aggregates"
3+
area: ES|QL
4+
type: bug
5+
issues:
6+
- 106427

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

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1226,3 +1226,92 @@ FROM employees
12261226
vals:l
12271227
183
12281228
;
1229+
1230+
emptyProjectInStatWithEval#[skip:-8.13.0,reason:fixed in 8.13.1]
1231+
FROM employees
1232+
| STATS c = COUNT(salary)
1233+
| EVAL x = 3.14
1234+
| DROP c
1235+
;
1236+
1237+
x:d
1238+
3.14
1239+
;
1240+
1241+
emptyProjectInStatWithCountGroupAndEval#[skip:-8.13.0,reason:fixed in 8.13.1]
1242+
FROM employees
1243+
| STATS c = COUNT(salary) BY gender
1244+
| EVAL x = 3.14
1245+
| DROP c, gender
1246+
;
1247+
1248+
x:d
1249+
3.14
1250+
3.14
1251+
3.14
1252+
;
1253+
1254+
1255+
emptyProjectInStatWithMinGroupAndEval#[skip:-8.13.0,reason:fixed in 8.13.1]
1256+
FROM employees
1257+
| STATS m = MIN(salary) BY gender
1258+
| EVAL x = 3.14
1259+
| DROP m, gender
1260+
;
1261+
1262+
x:d
1263+
3.14
1264+
3.14
1265+
3.14
1266+
;
1267+
1268+
emptyProjectInStatOnlyGroupAndEval#[skip:-8.13.0,reason:fixed in 8.13.1]
1269+
FROM employees
1270+
| STATS BY gender
1271+
| EVAL x = 3.14
1272+
| DROP gender
1273+
;
1274+
1275+
x:d
1276+
3.14
1277+
3.14
1278+
3.14
1279+
;
1280+
1281+
emptyProjectInStatWithTwoGroupsAndEval#[skip:-8.13.0,reason:fixed in 8.13.1]
1282+
FROM employees
1283+
| STATS c = COUNT(salary) BY gender, still_hired
1284+
| EVAL x = 3.14
1285+
| DROP c, gender, still_hired
1286+
;
1287+
1288+
x:d
1289+
3.14
1290+
3.14
1291+
3.14
1292+
3.14
1293+
3.14
1294+
3.14
1295+
;
1296+
1297+
emptyProjectInStatDueToAnotherStat#[skip:-8.13.0,reason:fixed in 8.13.1]
1298+
FROM employees
1299+
| STATS s = SUM(salary), m = MIN(salary)
1300+
| EVAL x = 3.14
1301+
| STATS rows = COUNT(*)
1302+
;
1303+
1304+
rows:l
1305+
1
1306+
;
1307+
1308+
emptyProjectInStatDueToAnotherStatWithGroups#[skip:-8.13.0,reason:fixed in 8.13.1]
1309+
FROM employees
1310+
| STATS m = MEDIAN(salary) BY gender, still_hired
1311+
| EVAL x = 3.14
1312+
| STATS rows = COUNT(*)
1313+
;
1314+
1315+
rows:l
1316+
6
1317+
;

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizer.java‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.elasticsearch.xpack.ql.expression.Attribute;
3434
import org.elasticsearch.xpack.ql.expression.AttributeMap;
3535
import org.elasticsearch.xpack.ql.expression.AttributeSet;
36+
import org.elasticsearch.xpack.ql.expression.EmptyAttribute;
3637
import org.elasticsearch.xpack.ql.expression.Expression;
3738
import org.elasticsearch.xpack.ql.expression.ExpressionSet;
3839
import org.elasticsearch.xpack.ql.expression.Expressions;
@@ -997,11 +998,23 @@ public LogicalPlan apply(LogicalPlan plan) {
997998
recheck = false;
998999
if (p instanceof Aggregate aggregate) {
9991000
var remaining = seenProjection.get() ? removeUnused(aggregate.aggregates(), used) : null;
1000-
// no aggregates, no need
1001+
10011002
if (remaining != null) {
10021003
if (remaining.isEmpty()) {
1003-
recheck = true;
1004-
p = aggregate.child();
1004+
// We still need to have a plan that produces 1 row per group.
1005+
if (aggregate.groupings().isEmpty()) {
1006+
p = new LocalRelation(
1007+
aggregate.source(),
1008+
List.of(new EmptyAttribute(aggregate.source())),
1009+
LocalSupplier.of(
1010+
new Block[] { BlockUtils.constantBlock(PlannerUtils.NON_BREAKING_BLOCK_FACTORY, null, 1) }
1011+
)
1012+
);
1013+
} else {
1014+
// Aggs cannot produce pages with 0 columns, so retain one grouping.
1015+
remaining = List.of(Expressions.attribute(aggregate.groupings().get(0)));
1016+
p = new Aggregate(aggregate.source(), aggregate.child(), aggregate.groupings(), remaining);
1017+
}
10051018
} else {
10061019
p = new Aggregate(aggregate.source(), aggregate.child(), aggregate.groupings(), remaining);
10071020
}

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,78 @@ public void testEmptyProjectionInStat() {
221221
assertThat(relation.supplier().get(), emptyArray());
222222
}
223223

224+
/**
225+
* Expects
226+
*
227+
* EsqlProject[[x{r}#6]]
228+
* \_Eval[[1[INTEGER] AS x]]
229+
* \_Limit[1000[INTEGER]]
230+
* \_LocalRelation[[{e}#18],[ConstantNullBlock[positions=1]]]
231+
*/
232+
public void testEmptyProjectInStatWithEval() {
233+
var plan = plan("""
234+
from test
235+
| where languages > 1
236+
| stats c = count(salary)
237+
| eval x = 1, c2 = c*2
238+
| drop c, c2
239+
""");
240+
241+
var project = as(plan, Project.class);
242+
var eval = as(project.child(), Eval.class);
243+
var limit = as(eval.child(), Limit.class);
244+
var singleRowRelation = as(limit.child(), LocalRelation.class);
245+
var singleRow = singleRowRelation.supplier().get();
246+
assertThat(singleRow.length, equalTo(1));
247+
assertThat(singleRow[0].getPositionCount(), equalTo(1));
248+
249+
var exprs = eval.fields();
250+
assertThat(exprs.size(), equalTo(1));
251+
var alias = as(exprs.get(0), Alias.class);
252+
assertThat(alias.name(), equalTo("x"));
253+
assertThat(alias.child().fold(), equalTo(1));
254+
}
255+
256+
/**
257+
* Expects
258+
*
259+
* EsqlProject[[x{r}#8]]
260+
* \_Eval[[1[INTEGER] AS x]]
261+
* \_Limit[1000[INTEGER]]
262+
* \_Aggregate[[emp_no{f}#15],[emp_no{f}#15]]
263+
* \_Filter[languages{f}#18 > 1[INTEGER]]
264+
* \_EsRelation[test][_meta_field{f}#21, emp_no{f}#15, first_name{f}#16, ..]
265+
*/
266+
public void testEmptyProjectInStatWithGroupAndEval() {
267+
var plan = plan("""
268+
from test
269+
| where languages > 1
270+
| stats c = count(salary) by emp_no
271+
| eval x = 1, c2 = c*2
272+
| drop c, emp_no, c2
273+
""");
274+
275+
var project = as(plan, Project.class);
276+
var eval = as(project.child(), Eval.class);
277+
var limit = as(eval.child(), Limit.class);
278+
var agg = as(limit.child(), Aggregate.class);
279+
var filter = as(agg.child(), Filter.class);
280+
var relation = as(filter.child(), EsRelation.class);
281+
282+
assertThat(Expressions.names(agg.groupings()), contains("emp_no"));
283+
assertThat(Expressions.names(agg.aggregates()), contains("emp_no"));
284+
285+
var exprs = eval.fields();
286+
assertThat(exprs.size(), equalTo(1));
287+
var alias = as(exprs.get(0), Alias.class);
288+
assertThat(alias.name(), equalTo("x"));
289+
assertThat(alias.child().fold(), equalTo(1));
290+
291+
var filterCondition = as(filter.condition(), GreaterThan.class);
292+
assertThat(Expressions.name(filterCondition.left()), equalTo("languages"));
293+
assertThat(filterCondition.right().fold(), equalTo(1));
294+
}
295+
224296
public void testCombineProjections() {
225297
var plan = plan("""
226298
from test

0 commit comments

Comments
 (0)