Skip to content

Commit c35397d

Browse files
bpinteabpintea
committed
Revert elastic#135446's diff
1 parent ea2321b commit c35397d

File tree

6 files changed

+15
-84
lines changed

6 files changed

+15
-84
lines changed

‎x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -253,11 +253,7 @@ public static Range rangeOf(Expression value, Expression lower, boolean includeL
253253
}
254254

255255
public static EsRelation relation() {
256-
return relation(IndexMode.STANDARD);
257-
}
258-
259-
public static EsRelation relation(IndexMode mode) {
260-
return new EsRelation(EMPTY, new EsIndex(randomAlphaOfLength(8), emptyMap()), mode);
256+
return new EsRelation(EMPTY, new EsIndex(randomAlphaOfLength(8), emptyMap()), IndexMode.STANDARD);
261257
}
262258

263259
/**

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import org.elasticsearch.xpack.esql.core.expression.AttributeMap;
1111
import org.elasticsearch.xpack.esql.core.expression.AttributeSet;
1212
import org.elasticsearch.xpack.esql.core.expression.Expression;
13+
import org.elasticsearch.xpack.esql.core.expression.Expressions;
1314
import org.elasticsearch.xpack.esql.optimizer.LogicalOptimizerContext;
1415
import org.elasticsearch.xpack.esql.optimizer.rules.RuleUtils;
1516
import org.elasticsearch.xpack.esql.optimizer.rules.logical.OptimizerRules;
@@ -58,6 +59,6 @@ private static LogicalPlan replaceJoin(Join join) {
5859
}
5960
var aliasedNulls = RuleUtils.aliasedNulls(joinRightOutput, a -> true);
6061
var eval = new Eval(join.source(), join.left(), aliasedNulls.v1());
61-
return new Project(join.source(), eval, join.computeOutputExpressions(join.left().output(), aliasedNulls.v2()));
62+
return new Project(join.source(), eval, join.computeOutput(join.left().output(), Expressions.asAttributes(aliasedNulls.v2())));
6263
}
6364
}

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Lookup.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
import org.elasticsearch.xpack.esql.core.capabilities.Resolvables;
1616
import org.elasticsearch.xpack.esql.core.expression.Attribute;
1717
import org.elasticsearch.xpack.esql.core.expression.Expression;
18-
import org.elasticsearch.xpack.esql.core.expression.Expressions;
1918
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
2019
import org.elasticsearch.xpack.esql.core.tree.Source;
2120
import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput;
@@ -140,7 +139,7 @@ public List<Attribute> output() {
140139
if (localRelation == null) {
141140
throw new IllegalStateException("Cannot determine output of LOOKUP with unresolved table");
142141
}
143-
lazyOutput = Expressions.asAttributes(Join.computeOutputExpressions(child().output(), localRelation.output(), joinConfig()));
142+
lazyOutput = Join.computeOutput(child().output(), localRelation.output(), joinConfig());
144143
}
145144
return lazyOutput;
146145
}

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/InlineJoin.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ public Join replaceChildren(LogicalPlan left, LogicalPlan right) {
200200
}
201201

202202
@Override
203-
List<Attribute> computeOutput(List<Attribute> left, List<Attribute> right) {
203+
public List<Attribute> computeOutput(List<Attribute> left, List<Attribute> right) {
204204
JoinType joinType = config().type();
205205
List<Attribute> output;
206206
if (LEFT.equals(joinType)) {

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/Join.java‎

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import org.elasticsearch.xpack.esql.core.expression.AttributeSet;
1919
import org.elasticsearch.xpack.esql.core.expression.Expression;
2020
import org.elasticsearch.xpack.esql.core.expression.Expressions;
21-
import org.elasticsearch.xpack.esql.core.expression.NamedExpression;
2221
import org.elasticsearch.xpack.esql.core.expression.ReferenceAttribute;
2322
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
2423
import org.elasticsearch.xpack.esql.core.tree.Source;
@@ -65,7 +64,7 @@
6564
import static org.elasticsearch.xpack.esql.core.type.DataType.UNSIGNED_LONG;
6665
import static org.elasticsearch.xpack.esql.core.type.DataType.UNSUPPORTED;
6766
import static org.elasticsearch.xpack.esql.core.type.DataType.VERSION;
68-
import static org.elasticsearch.xpack.esql.expression.NamedExpressions.mergeOutputExpressions;
67+
import static org.elasticsearch.xpack.esql.expression.NamedExpressions.mergeOutputAttributes;
6968
import static org.elasticsearch.xpack.esql.plan.logical.join.JoinTypes.LEFT;
7069
import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.commonType;
7170

@@ -211,38 +210,30 @@ public List<Attribute> rightOutputFields() {
211210
return rightOutputFields;
212211
}
213212

214-
List<Attribute> computeOutput(List<Attribute> left, List<Attribute> right) {
215-
return Expressions.asAttributes(computeOutputExpressions(left, right, config));
216-
}
217-
218-
public List<NamedExpression> computeOutputExpressions(List<? extends NamedExpression> left, List<? extends NamedExpression> right) {
219-
return computeOutputExpressions(left, right, config);
213+
public List<Attribute> computeOutput(List<Attribute> left, List<Attribute> right) {
214+
return computeOutput(left, right, config);
220215
}
221216

222217
/**
223218
* Combine the two lists of attributes into one.
224219
* In case of (name) conflicts, specify which sides wins, that is overrides the other column - the left or the right.
225220
*/
226-
public static List<NamedExpression> computeOutputExpressions(
227-
List<? extends NamedExpression> leftOutput,
228-
List<? extends NamedExpression> rightOutput,
229-
JoinConfig config
230-
) {
221+
public static List<Attribute> computeOutput(List<Attribute> leftOutput, List<Attribute> rightOutput, JoinConfig config) {
231222
JoinType joinType = config.type();
232-
List<NamedExpression> output;
223+
List<Attribute> output;
233224
// TODO: make the other side nullable
234225
if (LEFT.equals(joinType)) {
235226
if (config.joinOnConditions() == null) {
236227
// right side becomes nullable and overrides left except for join keys, which we preserve from the left
237228
AttributeSet rightKeys = AttributeSet.of(config.rightFields());
238-
List<? extends NamedExpression> rightOutputWithoutMatchFields = rightOutput.stream()
239-
.filter(ne -> rightKeys.contains(ne.toAttribute()) == false)
229+
List<Attribute> rightOutputWithoutMatchFields = rightOutput.stream()
230+
.filter(attr -> rightKeys.contains(attr) == false)
240231
.toList();
241-
output = mergeOutputExpressions(rightOutputWithoutMatchFields, leftOutput);
232+
output = mergeOutputAttributes(rightOutputWithoutMatchFields, leftOutput);
242233
} else {
243234
// We don't allow any attributes in the joinOnConditions that don't have unique names
244235
// so right always overwrites left in case of name clashes
245-
output = mergeOutputExpressions(rightOutput, leftOutput);
236+
output = mergeOutputAttributes(rightOutput, leftOutput);
246237
}
247238

248239
} else {

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

Lines changed: 1 addition & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,6 @@
6161
import org.elasticsearch.xpack.esql.plan.logical.Project;
6262
import org.elasticsearch.xpack.esql.plan.logical.Row;
6363
import org.elasticsearch.xpack.esql.plan.logical.UnaryPlan;
64-
import org.elasticsearch.xpack.esql.plan.logical.join.Join;
65-
import org.elasticsearch.xpack.esql.plan.logical.join.JoinConfig;
66-
import org.elasticsearch.xpack.esql.plan.logical.join.JoinTypes;
6764
import org.elasticsearch.xpack.esql.plan.logical.local.EmptyLocalSupplier;
6865
import org.elasticsearch.xpack.esql.plan.logical.local.EsqlProject;
6966
import org.elasticsearch.xpack.esql.plan.logical.local.LocalRelation;
@@ -99,7 +96,6 @@
9996
import static org.elasticsearch.xpack.esql.EsqlTestUtils.withDefaultLimitWarning;
10097
import static org.elasticsearch.xpack.esql.core.tree.Source.EMPTY;
10198
import static org.elasticsearch.xpack.esql.core.type.DataType.INTEGER;
102-
import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD;
10399
import static org.elasticsearch.xpack.esql.optimizer.rules.logical.OptimizerRules.TransformDirection.DOWN;
104100
import static org.elasticsearch.xpack.esql.optimizer.rules.logical.OptimizerRules.TransformDirection.UP;
105101
import static org.hamcrest.Matchers.contains;
@@ -1023,58 +1019,6 @@ public List<Attribute> output() {
10231019
assertThat(e.getMessage(), containsString("Output has changed from"));
10241020
}
10251021

1026-
/**
1027-
* Input:
1028-
* Project[[key{f}#2, int{f}#3, field1{f}#7, field2{f}#8]]
1029-
* \_Join[LEFT,[key{f}#2],[key{f}#6],null]
1030-
* |_EsRelation[JLfQlKmn][key{f}#2, int{f}#3, field1{f}#4, field2{f}#5]
1031-
* \_EsRelation[HQtEBOWq][LOOKUP][key{f}#6, field1{f}#7, field2{f}#8]
1032-
*
1033-
* Output:
1034-
* Project[[key{r}#2, int{f}#3, field1{r}#7, field1{r}#7 AS field2#8]]
1035-
* \_Eval[[null[KEYWORD] AS key#2, null[INTEGER] AS field1#7]]
1036-
* \_EsRelation[JLfQlKmn][key{f}#2, int{f}#3, field1{f}#4, field2{f}#5]
1037-
*/
1038-
public void testPruneLeftJoinOnNullMatchingFieldAndShadowingAttributes() {
1039-
var keyLeft = getFieldAttribute("key", KEYWORD);
1040-
var intFieldLeft = getFieldAttribute("int");
1041-
var fieldLeft1 = getFieldAttribute("field1");
1042-
var fieldLeft2 = getFieldAttribute("field2");
1043-
var leftRelation = EsqlTestUtils.relation(IndexMode.STANDARD)
1044-
.withAttributes(List.of(keyLeft, intFieldLeft, fieldLeft1, fieldLeft2));
1045-
1046-
var keyRight = getFieldAttribute("key", KEYWORD);
1047-
var fieldRight1 = getFieldAttribute("field1");
1048-
var fieldRight2 = getFieldAttribute("field2");
1049-
var rightRelation = EsqlTestUtils.relation(IndexMode.LOOKUP).withAttributes(List.of(keyRight, fieldRight1, fieldRight2));
1050-
1051-
JoinConfig joinConfig = new JoinConfig(JoinTypes.LEFT, List.of(keyLeft), List.of(keyRight), null);
1052-
var join = new Join(EMPTY, leftRelation, rightRelation, joinConfig);
1053-
var project = new Project(EMPTY, join, List.of(keyLeft, intFieldLeft, fieldRight1, fieldRight2));
1054-
1055-
var testStats = statsForMissingField("key");
1056-
var localPlan = localPlan(project, testStats);
1057-
1058-
var projectOut = as(localPlan, Project.class);
1059-
var projectionsOut = projectOut.projections();
1060-
assertThat(Expressions.names(projectionsOut), contains("key", "int", "field1", "field2"));
1061-
assertThat(projectionsOut.get(0).id(), is(keyLeft.id()));
1062-
assertThat(projectionsOut.get(1).id(), is(intFieldLeft.id()));
1063-
assertThat(projectionsOut.get(2).id(), is(fieldRight1.id())); // id must remain from the RHS.
1064-
var aliasField2 = as(projectionsOut.get(3), Alias.class); // the projection must contain an alias ...
1065-
assertThat(aliasField2.id(), is(fieldRight2.id())); // ... with the same id as the original field.
1066-
1067-
var eval = as(projectOut.child(), Eval.class);
1068-
assertThat(Expressions.names(eval.fields()), contains("key", "field1"));
1069-
var keyEval = as(Alias.unwrap(eval.fields().get(0)), Literal.class);
1070-
assertThat(keyEval.value(), is(nullValue()));
1071-
assertThat(keyEval.dataType(), is(KEYWORD));
1072-
var field1Eval = as(Alias.unwrap(eval.fields().get(1)), Literal.class);
1073-
assertThat(field1Eval.value(), is(nullValue()));
1074-
assertThat(field1Eval.dataType(), is(INTEGER));
1075-
var source = as(eval.child(), EsRelation.class);
1076-
}
1077-
10781022
private IsNotNull isNotNull(Expression field) {
10791023
return new IsNotNull(EMPTY, field);
10801024
}
@@ -1134,6 +1078,6 @@ protected List<String> filteredWarnings() {
11341078
}
11351079

11361080
public static EsRelation relation() {
1137-
return EsqlTestUtils.relation(randomFrom(IndexMode.values()));
1081+
return new EsRelation(EMPTY, new EsIndex(randomAlphaOfLength(8), emptyMap()), randomFrom(IndexMode.values()));
11381082
}
11391083
}

0 commit comments

Comments
 (0)