Skip to content

Commit 873d214

Browse files
committed
ESQL: Have CombineProjections propagate references upwards (#127264)
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 f26c944 commit 873d214

File tree

5 files changed

+57
-16
lines changed

5 files changed

+57
-16
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.Categorize;
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/ReplaceMissingFieldWithNull.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ public LogicalPlan apply(LogicalPlan plan, LocalLogicalOptimizerContext localLog
5858
// Do not use the attribute name, this can deviate from the field name for union types; use fieldName() instead.
5959
// Also retain fields from lookup indices because we do not have stats for these.
6060
Predicate<FieldAttribute> shouldBeRetained = f -> f.field() instanceof PotentiallyUnmappedKeywordEsField
61-
|| (localLogicalOptimizerContext.searchStats().exists(f.fieldName()) || lookupFieldsBuilder.contains(f));
61+
|| localLogicalOptimizerContext.searchStats().exists(f.fieldName())
62+
|| lookupFieldsBuilder.contains(f);
6263

6364
return plan.transformUp(p -> missingToNull(p, shouldBeRetained));
6465
}
@@ -77,7 +78,7 @@ private LogicalPlan missingToNull(LogicalPlan plan, Predicate<FieldAttribute> sh
7778
for (int i = 0, size = relationOutput.size(); i < size; i++) {
7879
Attribute attr = relationOutput.get(i);
7980
NamedExpression projection;
80-
if (attr instanceof FieldAttribute f && (shouldBeRetained.test(f) == false)) {
81+
if (attr instanceof FieldAttribute f && shouldBeRetained.test(f) == false) {
8182
DataType dt = f.dataType();
8283
Alias nullAlias = nullLiterals.get(dt);
8384
// 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 & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@
3838
import org.elasticsearch.xpack.esql.analysis.Verifier;
3939
import org.elasticsearch.xpack.esql.core.expression.Alias;
4040
import org.elasticsearch.xpack.esql.core.expression.Expressions;
41-
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
4241
import org.elasticsearch.xpack.esql.core.expression.Literal;
42+
import org.elasticsearch.xpack.esql.core.expression.ReferenceAttribute;
4343
import org.elasticsearch.xpack.esql.core.tree.Source;
4444
import org.elasticsearch.xpack.esql.core.type.DataType;
4545
import org.elasticsearch.xpack.esql.core.type.EsField;
@@ -1305,15 +1305,16 @@ private EsQueryExec doTestOutOfRangeFilterPushdown(String query, Analyzer analyz
13051305
return luceneQuery;
13061306
}
13071307

1308-
/**
1308+
/*
13091309
* Expects
13101310
* LimitExec[1000[INTEGER]]
1311-
* \_ExchangeExec[[],false]
1312-
* \_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
1313-
* ame{r}#3 AS last_name, long_noidx{f}#11, emp_no{r}#2 AS salary]]
1314-
* \_FieldExtractExec[_meta_field{f}#8, gender{f}#4, job{f}#9, job.raw{f}..]
1311+
* \_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
1312+
* ges{f}#5, last_name{f}#6, long_noidx{f}#12, salary{f}#7],false]
1313+
* \_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
1314+
* ges{f}#5, first_name{r}#3 AS last_name, long_noidx{f}#12, emp_no{r}#2 AS salary]]
1315+
* \_FieldExtractExec[_meta_field{f}#8, gender{f}#4, hire_date{f}#9, job{..]<[],[]>
13151316
* \_EvalExec[[null[INTEGER] AS emp_no, null[KEYWORD] AS first_name]]
1316-
* \_EsQueryExec[test], query[][_doc{f}#12], limit[1000], sort[] estimatedRowSize[270]
1317+
* \_EsQueryExec[test], indexMode[standard], query[][_doc{f}#13], limit[1000], sort[] estimatedRowSize[278]
13171318
*/
13181319
public void testMissingFieldsDoNotGetExtracted() {
13191320
var stats = EsqlTestUtils.statsForMissingField("first_name", "last_name", "emp_no", "salary");
@@ -1340,9 +1341,9 @@ public void testMissingFieldsDoNotGetExtracted() {
13401341
)
13411342
);
13421343
// emp_no
1343-
assertThat(projections.get(1), instanceOf(FieldAttribute.class));
1344+
assertThat(projections.get(1), instanceOf(ReferenceAttribute.class));
13441345
// first_name
1345-
assertThat(projections.get(2), instanceOf(FieldAttribute.class));
1346+
assertThat(projections.get(2), instanceOf(ReferenceAttribute.class));
13461347

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

0 commit comments

Comments
 (0)