Skip to content

Commit 00654c8

Browse files
authored
ESQL: Retain aggregate when grouping (#126598)
1 parent a15b9dd commit 00654c8

File tree

6 files changed

+120
-3
lines changed

6 files changed

+120
-3
lines changed

docs/changelog/126598.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 126598
2+
summary: "ESQL: Retain aggregate when grouping"
3+
area: ES|QL
4+
type: bug
5+
issues:
6+
- 126026

x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/GenerativeRestTest.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ public abstract class GenerativeRestTest extends ESRestTestCase {
5252
"Plan \\[ProjectExec\\[\\[<no-fields>.* optimized incorrectly due to missing references", // https://github.com/elastic/elasticsearch/issues/125866
5353
"token recognition error at: '``", // https://github.com/elastic/elasticsearch/issues/125870
5454
// https://github.com/elastic/elasticsearch/issues/127167
55-
"Unknown column \\[.*\\]", // https://github.com/elastic/elasticsearch/issues/126026
5655
"optimized incorrectly due to missing references", // https://github.com/elastic/elasticsearch/issues/116781
5756
"No matches found for pattern", // https://github.com/elastic/elasticsearch/issues/126418
5857
"The incoming YAML document exceeds the limit:" // still to investigate, but it seems to be specific to the test framework

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

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -673,3 +673,82 @@ ROW foo = [10, 11, 12]
673673
max:integer
674674
13
675675
;
676+
677+
evalAfterAvgGroupingUsingSameName
678+
required_capability: retain_aggregate_when_grouping
679+
from employees
680+
| stats max = max(salary), avg = avg(salary) by g = gender
681+
| keep max, g
682+
| eval max = 12, g = 3
683+
;
684+
685+
max:integer | g:integer
686+
12 | 3
687+
12 | 3
688+
12 | 3
689+
;
690+
691+
evalAfterGroupingUsingSameName
692+
required_capability: retain_aggregate_when_grouping
693+
row foo = [10,11,9], bar = [1,2,3]
694+
| mv_expand foo
695+
| mv_expand bar
696+
| stats this = max(foo) by bar
697+
| keep this
698+
| eval this = 12
699+
;
700+
701+
this:integer
702+
12
703+
12
704+
12
705+
;
706+
707+
evalAfterGroupingUsingSameName2
708+
required_capability: retain_aggregate_when_grouping
709+
from employees
710+
| stats count = count(emp_no) by gender, is_rehired
711+
| keep count
712+
| rename count as x
713+
| keep x
714+
| eval x = 12
715+
;
716+
717+
x:integer
718+
12
719+
12
720+
12
721+
12
722+
12
723+
12
724+
12
725+
12
726+
12
727+
;
728+
729+
evalAfterGroupingUsingSameName3
730+
required_capability: retain_aggregate_when_grouping
731+
row foo = 10
732+
| stats field_1 = max(foo), this = count(*), field_2 = max(foo) by foo
733+
| rename this AS field_2
734+
| keep field_1, field_2
735+
| eval field_2 = 1, field_1 = 1
736+
;
737+
738+
field_2:integer | field_1:integer
739+
1 | 1
740+
;
741+
742+
evalAfterGroupingUsingSameName4
743+
required_capability: retain_aggregate_when_grouping
744+
from employees
745+
| stats sum = sum(salary), max = max(salary) by g = gender
746+
| keep max, g
747+
| eval g = 12, max = 2
748+
;
749+
750+
g:integer | max:integer
751+
12 | 2
752+
12 | 2
753+
12 | 2
754+
;

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,12 @@ public enum Cap {
418418
*/
419419
REMOVE_EMPTY_ATTRIBUTE_IN_MERGING_OUTPUT,
420420

421+
/**
422+
* Support for retain aggregate when grouping.
423+
* See <a href="https://github.com/elastic/elasticsearch/issues/126026"> ES|QL: columns not projected away despite KEEP #126026 </a>
424+
*/
425+
RETAIN_AGGREGATE_WHEN_GROUPING,
426+
421427
/**
422428
* Fix for union-types when some indexes are missing the required field. Done in #111932.
423429
*/

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PruneColumns.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
import org.elasticsearch.compute.data.Block;
1111
import org.elasticsearch.compute.data.BlockUtils;
1212
import org.elasticsearch.index.IndexMode;
13+
import org.elasticsearch.xpack.esql.core.expression.Alias;
14+
import org.elasticsearch.xpack.esql.core.expression.Attribute;
1315
import org.elasticsearch.xpack.esql.core.expression.AttributeSet;
1416
import org.elasticsearch.xpack.esql.core.expression.Expressions;
1517
import org.elasticsearch.xpack.esql.core.expression.NamedExpression;
@@ -83,7 +85,11 @@ public LogicalPlan apply(LogicalPlan plan) {
8385
);
8486
} else {
8587
// Aggs cannot produce pages with 0 columns, so retain one grouping.
86-
remaining = List.of(Expressions.attribute(aggregate.groupings().get(0)));
88+
Attribute attribute = Expressions.attribute(aggregate.groupings().getFirst());
89+
NamedExpression firstAggregate = aggregate.aggregates().getFirst();
90+
remaining = List.of(
91+
new Alias(firstAggregate.source(), firstAggregate.name(), attribute, firstAggregate.id())
92+
);
8793
p = aggregate.with(aggregate.groupings(), remaining);
8894
}
8995
} else {

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

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@ public void testEmptyProjectInStatWithGroupAndEval() {
423423
var relation = as(filter.child(), EsRelation.class);
424424

425425
assertThat(Expressions.names(agg.groupings()), contains("emp_no"));
426-
assertThat(Expressions.names(agg.aggregates()), contains("emp_no"));
426+
assertThat(Expressions.names(agg.aggregates()), contains("c"));
427427

428428
var exprs = eval.fields();
429429
assertThat(exprs.size(), equalTo(1));
@@ -2802,6 +2802,27 @@ public void testEvalAfterStats() {
28022802
assertThat(Expressions.names(eval.output()), contains("x"));
28032803
}
28042804

2805+
/**
2806+
* Expects
2807+
* Eval[[2[INTEGER] AS x]]
2808+
* \_Limit[1000[INTEGER],false]
2809+
* \_Aggregate[[foo{r}#3],[foo{r}#3 AS x]]
2810+
* \_LocalRelation[[foo{r}#3],[IntVectorBlock[vector=ConstantIntVector[positions=1, value=1]]]]
2811+
*/
2812+
public void testEvalAfterGroupBy() {
2813+
var plan = optimizedPlan("""
2814+
ROW foo = 1
2815+
| STATS x = max(foo) by foo
2816+
| KEEP x
2817+
| EVAL x = 2
2818+
""");
2819+
var eval = as(plan, Eval.class);
2820+
var limit = as(eval.child(), Limit.class);
2821+
var aggregate = as(limit.child(), Aggregate.class);
2822+
var localRelation = as(aggregate.child(), LocalRelation.class);
2823+
assertThat(Expressions.names(eval.output()), contains("x"));
2824+
}
2825+
28052826
public void testCombineLimitWithOrderByThroughFilterAndEval() {
28062827
LogicalPlan plan = optimizedPlan("""
28072828
from test

0 commit comments

Comments
 (0)