Skip to content

Commit 8ea4172

Browse files
authored
ESQL: Have CombineProjections propagate references upwards (#127264) (#128682)
This will have CombineProjections allow references from the "under" plan be kept when merging stacked Projections. This is to prevent field attributes that are dropped by ReplaceMissingFieldWithNull be used in the resulting plan. (cherry picked from commit 9626fe5)
1 parent d8e97b7 commit 8ea4172

File tree

5 files changed

+55
-14
lines changed

5 files changed

+55
-14
lines changed

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Alias.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,8 @@ protected NodeInfo<Alias> info() {
9393
}
9494

9595
public Alias replaceChild(Expression child) {
96-
return new Alias(source(), name(), child, id(), synthetic());
96+
// these "nop" replacements can occur on attribute map resolutions having the default return same as the lookup key
97+
return child == this.child ? this : new Alias(source(), name(), child, id(), synthetic());
9798
}
9899

99100
@Override

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import org.elasticsearch.xpack.esql.core.expression.Expression;
1616
import org.elasticsearch.xpack.esql.core.expression.Expressions;
1717
import org.elasticsearch.xpack.esql.core.expression.NamedExpression;
18+
import org.elasticsearch.xpack.esql.core.expression.ReferenceAttribute;
1819
import org.elasticsearch.xpack.esql.expression.function.grouping.GroupingFunction;
1920
import org.elasticsearch.xpack.esql.plan.logical.Aggregate;
2021
import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
@@ -128,6 +129,8 @@ private static List<NamedExpression> combineProjections(List<? extends NamedExpr
128129
if (ne instanceof Alias as) {
129130
Expression child = as.child();
130131
namedExpressionsBuilder.put(ne.toAttribute(), as.replaceChild(aliasesBuilder.build().resolve(child, child)));
132+
} else if (ne instanceof ReferenceAttribute ra) {
133+
namedExpressionsBuilder.put(ra, ra);
131134
}
132135
}
133136
List<NamedExpression> replaced = new ArrayList<>();

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ private LogicalPlan replaceWithNullOrConstant(
9595
for (int i = 0, size = relationOutput.size(); i < size; i++) {
9696
Attribute attr = relationOutput.get(i);
9797
NamedExpression projection;
98-
if (attr instanceof FieldAttribute f && (shouldBeRetained.test(f) == false)) {
98+
if (attr instanceof FieldAttribute f && shouldBeRetained.test(f) == false) {
9999
DataType dt = f.dataType();
100100
Alias nullAlias = nullLiterals.get(dt);
101101
// save the first field as null (per datatype)

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

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
package org.elasticsearch.xpack.esql.optimizer;
99

10+
import org.apache.lucene.util.BytesRef;
1011
import org.elasticsearch.common.io.stream.StreamOutput;
1112
import org.elasticsearch.common.util.Maps;
1213
import org.elasticsearch.index.IndexMode;
@@ -22,6 +23,7 @@
2223
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
2324
import org.elasticsearch.xpack.esql.core.expression.FoldContext;
2425
import org.elasticsearch.xpack.esql.core.expression.Literal;
26+
import org.elasticsearch.xpack.esql.core.expression.ReferenceAttribute;
2527
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
2628
import org.elasticsearch.xpack.esql.core.tree.Source;
2729
import org.elasticsearch.xpack.esql.core.type.DataType;
@@ -141,10 +143,10 @@ public void testMissingFieldInFilterString() {
141143

142144
/**
143145
* Expects
144-
* Project[[last_name{f}#6]]
146+
* Project[[last_name{r}#7]]
145147
* \_Eval[[null[KEYWORD] AS last_name]]
146-
* \_Limit[10000[INTEGER]]
147-
* \_EsRelation[test][_meta_field{f}#8, emp_no{f}#2, first_name{f}#3, gen..]
148+
* \_Limit[1000[INTEGER],false]
149+
* \_EsRelation[test][_meta_field{f}#9, emp_no{f}#3, first_name{f}#4, gen..]
148150
*/
149151
public void testMissingFieldInProject() {
150152
var plan = plan("""
@@ -158,7 +160,7 @@ public void testMissingFieldInProject() {
158160
var project = as(localPlan, Project.class);
159161
var projections = project.projections();
160162
assertThat(Expressions.names(projections), contains("last_name"));
161-
as(projections.get(0), FieldAttribute.class);
163+
as(projections.get(0), ReferenceAttribute.class);
162164
var eval = as(project.child(), Eval.class);
163165
assertThat(Expressions.names(eval.fields()), contains("last_name"));
164166
var alias = as(eval.fields().get(0), Alias.class);
@@ -171,6 +173,39 @@ public void testMissingFieldInProject() {
171173
assertThat(Expressions.names(source.output()), not(contains("last_name")));
172174
}
173175

176+
/*
177+
* Expects a similar plan to testMissingFieldInProject() above, except for the Alias's child value
178+
* Project[[last_name{r}#4]]
179+
* \_Eval[[[66 6f 6f][KEYWORD] AS last_name]]
180+
* \_Limit[1000[INTEGER],false]
181+
* \_EsRelation[test][_meta_field{f}#11, emp_no{f}#5, first_name{f}#6, ge..]
182+
*/
183+
public void testReassignedMissingFieldInProject() {
184+
var plan = plan("""
185+
from test
186+
| keep last_name
187+
| eval last_name = "foo"
188+
""");
189+
190+
var testStats = statsForMissingField("last_name");
191+
var localPlan = localPlan(plan, testStats);
192+
193+
var project = as(localPlan, Project.class);
194+
var projections = project.projections();
195+
assertThat(Expressions.names(projections), contains("last_name"));
196+
as(projections.get(0), ReferenceAttribute.class);
197+
var eval = as(project.child(), Eval.class);
198+
assertThat(Expressions.names(eval.fields()), contains("last_name"));
199+
var alias = as(eval.fields().get(0), Alias.class);
200+
var literal = as(alias.child(), Literal.class);
201+
assertThat(literal.value(), is(new BytesRef("foo")));
202+
assertThat(literal.dataType(), is(DataType.KEYWORD));
203+
204+
var limit = as(eval.child(), Limit.class);
205+
var source = as(limit.child(), EsRelation.class);
206+
assertThat(Expressions.names(source.output()), not(contains("last_name")));
207+
}
208+
174209
/**
175210
* Expects
176211
* EsqlProject[[first_name{f}#4]]

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

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
4242
import org.elasticsearch.xpack.esql.core.expression.Literal;
4343
import org.elasticsearch.xpack.esql.core.expression.NamedExpression;
44+
import org.elasticsearch.xpack.esql.core.expression.ReferenceAttribute;
4445
import org.elasticsearch.xpack.esql.core.tree.Source;
4546
import org.elasticsearch.xpack.esql.core.type.DataType;
4647
import org.elasticsearch.xpack.esql.core.type.EsField;
@@ -1272,15 +1273,16 @@ private EsQueryExec doTestOutOfRangeFilterPushdown(String query, Analyzer analyz
12721273
return luceneQuery;
12731274
}
12741275

1275-
/**
1276+
/*
12761277
* Expects
12771278
* LimitExec[1000[INTEGER]]
1278-
* \_ExchangeExec[[],false]
1279-
* \_ProjectExec[[_meta_field{f}#8, emp_no{r}#2, first_name{r}#3, gender{f}#4, job{f}#9, job.raw{f}#10, languages{f}#5, first_n
1280-
* ame{r}#3 AS last_name, long_noidx{f}#11, emp_no{r}#2 AS salary]]
1281-
* \_FieldExtractExec[_meta_field{f}#8, gender{f}#4, job{f}#9, job.raw{f}..]
1279+
* \_ExchangeExec[[_meta_field{f}#8, emp_no{f}#2, first_name{f}#3, gender{f}#4, hire_date{f}#9, job{f}#10, job.raw{f}#11, langua
1280+
* ges{f}#5, last_name{f}#6, long_noidx{f}#12, salary{f}#7],false]
1281+
* \_ProjectExec[[_meta_field{f}#8, emp_no{r}#2, first_name{r}#3, gender{f}#4, hire_date{f}#9, job{f}#10, job.raw{f}#11, langua
1282+
* ges{f}#5, first_name{r}#3 AS last_name, long_noidx{f}#12, emp_no{r}#2 AS salary]]
1283+
* \_FieldExtractExec[_meta_field{f}#8, gender{f}#4, hire_date{f}#9, job{..]<[],[]>
12821284
* \_EvalExec[[null[INTEGER] AS emp_no, null[KEYWORD] AS first_name]]
1283-
* \_EsQueryExec[test], query[][_doc{f}#12], limit[1000], sort[] estimatedRowSize[270]
1285+
* \_EsQueryExec[test], indexMode[standard], query[][_doc{f}#13], limit[1000], sort[] estimatedRowSize[278]
12841286
*/
12851287
public void testMissingFieldsDoNotGetExtracted() {
12861288
var stats = EsqlTestUtils.statsForMissingField("first_name", "last_name", "emp_no", "salary");
@@ -1307,9 +1309,9 @@ public void testMissingFieldsDoNotGetExtracted() {
13071309
)
13081310
);
13091311
// emp_no
1310-
assertThat(projections.get(1), instanceOf(FieldAttribute.class));
1312+
assertThat(projections.get(1), instanceOf(ReferenceAttribute.class));
13111313
// first_name
1312-
assertThat(projections.get(2), instanceOf(FieldAttribute.class));
1314+
assertThat(projections.get(2), instanceOf(ReferenceAttribute.class));
13131315

13141316
// last_name --> first_name
13151317
var nullAlias = Alias.unwrap(projections.get(8));

0 commit comments

Comments
 (0)