Skip to content

Commit f5377c3

Browse files
resolve groupings before resolving references to groupings in the aggregations
1 parent dbedc51 commit f5377c3

File tree

2 files changed

+55
-14
lines changed

2 files changed

+55
-14
lines changed

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -173,13 +173,13 @@ public class Analyzer extends ParameterizedRuleExecutor<LogicalPlan, AnalyzerCon
173173
),
174174
new Batch<>(
175175
"Resolution",
176+
new ResolveRefs(),
176177
/*
177178
* ImplicitCasting must be before ResolveRefs. Because a reference is created for a Bucket in Aggregate's aggregates,
178179
* resolving this reference before implicit casting may cause this reference to have customMessage=true, it prevents further
179180
* attempts to resolve this reference.
180181
*/
181182
new ImplicitCasting(),
182-
new ResolveRefs(),
183183
new ResolveUnionTypes() // Must be after ResolveRefs, so union types can be found
184184
),
185185
new Batch<>("Finish Analysis", Limiter.ONCE, new AddImplicitLimit(), new AddImplicitForkLimit(), new UnionTypesCleanup())
@@ -574,7 +574,7 @@ private Aggregate resolveAggregate(Aggregate aggregate, List<Attribute> children
574574
}
575575
}
576576

577-
if (Resolvables.resolved(groupings) == false || (Resolvables.resolved(aggregates) == false)) {
577+
if (Resolvables.resolved(groupings) == false || Resolvables.resolved(aggregates) == false) {
578578
ArrayList<Attribute> resolved = new ArrayList<>();
579579
for (Expression e : groupings) {
580580
Attribute attr = Expressions.attribute(e);
@@ -585,17 +585,26 @@ private Aggregate resolveAggregate(Aggregate aggregate, List<Attribute> children
585585
List<Attribute> resolvedList = NamedExpressions.mergeOutputAttributes(resolved, childrenOutput);
586586

587587
List<NamedExpression> newAggregates = new ArrayList<>();
588-
for (NamedExpression ag : aggregate.aggregates()) {
589-
var agg = (NamedExpression) ag.transformUp(UnresolvedAttribute.class, ua -> {
590-
Expression ne = ua;
591-
Attribute maybeResolved = maybeResolveAttribute(ua, resolvedList);
592-
if (maybeResolved != null) {
593-
changed.set(true);
594-
ne = maybeResolved;
595-
}
596-
return ne;
597-
});
598-
newAggregates.add(agg);
588+
// If the groupings are not resolved, skip the resolution of the references to groupings in the aggregates, resolve the
589+
// aggregations besides groupings, so that the fields and attributes referenced by the aggregations can be resolved, and
590+
// verifier doesn't report field/reference/column not found errors for them.
591+
int size = Resolvables.resolved(groupings) ? aggregates.size() : aggregates.size() - groupings.size();
592+
for (int i = 0; i < aggregates.size(); i++) {
593+
NamedExpression ag = aggregates.get(i);
594+
if (i < size) {
595+
var agg = (NamedExpression) ag.transformUp(UnresolvedAttribute.class, ua -> {
596+
Expression ne = ua;
597+
Attribute maybeResolved = maybeResolveAttribute(ua, resolvedList);
598+
if (maybeResolved != null) {
599+
changed.set(true);
600+
ne = maybeResolved;
601+
}
602+
return ne;
603+
});
604+
newAggregates.add(agg);
605+
} else {
606+
newAggregates.add(ag); // Groupings are not resolved
607+
}
599608
}
600609

601610
// TODO: remove this when Stats interface is removed

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

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,8 @@
110110
import static org.elasticsearch.xpack.esql.analysis.AnalyzerTestUtils.randomValueOtherThanTest;
111111
import static org.elasticsearch.xpack.esql.analysis.AnalyzerTestUtils.tsdbIndexResolution;
112112
import static org.elasticsearch.xpack.esql.core.tree.Source.EMPTY;
113+
import static org.elasticsearch.xpack.esql.core.type.DataType.DATETIME;
114+
import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.dateTimeToString;
113115
import static org.hamcrest.Matchers.contains;
114116
import static org.hamcrest.Matchers.containsString;
115117
import static org.hamcrest.Matchers.empty;
@@ -368,7 +370,7 @@ public void testNoProjection() {
368370
DataType.INTEGER,
369371
DataType.KEYWORD,
370372
DataType.TEXT,
371-
DataType.DATETIME,
373+
DATETIME,
372374
DataType.TEXT,
373375
DataType.KEYWORD,
374376
DataType.INTEGER,
@@ -3776,6 +3778,36 @@ public void testResolveCompletionOutputField() {
37763778
assertThat(getAttributeByName(esRelation.output(), "description"), not(equalTo(completion.targetField())));
37773779
}
37783780

3781+
public void testResolveAggregateWithGroupings() {
3782+
var plan = analyze("""
3783+
FROM test
3784+
| EVAL date = "2025-01-01"::datetime
3785+
| STATS c = count(emp_no) BY d = (date == "2025-01-01")
3786+
""", "mapping-default.json");
3787+
3788+
var limit = as(plan, Limit.class);
3789+
var agg = as(limit.child(), Aggregate.class);
3790+
var aggregates = agg.aggregates();
3791+
assertThat(aggregates, hasSize(2));
3792+
Alias a = as(aggregates.get(0), Alias.class);
3793+
assertEquals("c", a.name());
3794+
Count c = as(a.child(), Count.class);
3795+
FieldAttribute fa = as(c.field(), FieldAttribute.class);
3796+
assertEquals("emp_no", fa.name());
3797+
ReferenceAttribute ra = as(aggregates.get(1), ReferenceAttribute.class); // reference in aggregates is resolved
3798+
assertEquals("d", ra.name());
3799+
List<Expression> groupings = agg.groupings();
3800+
assertEquals(1, groupings.size());
3801+
a = as(groupings.get(0), Alias.class); // reference in grouping is resolved
3802+
assertEquals("d", ra.name());
3803+
Equals equals = as(a.child(), Equals.class);
3804+
ra = as(equals.left(), ReferenceAttribute.class);
3805+
assertEquals("date", ra.name());
3806+
Literal literal = as(equals.right(), Literal.class);
3807+
assertEquals("2025-01-01T00:00:00.000Z", dateTimeToString(Long.parseLong(literal.value().toString())));
3808+
assertEquals(DATETIME, literal.dataType());
3809+
}
3810+
37793811
@Override
37803812
protected IndexAnalyzers createDefaultIndexAnalyzers() {
37813813
return super.createDefaultIndexAnalyzers();

0 commit comments

Comments
 (0)