Skip to content

Commit b5215da

Browse files
committed
Fix planning of dangling Project with InlineJoin
This fixes a planning error leaving a Project on top of an empty LocalRelation marker unpruned. The empty EmptyRelation is added in the plan as a marker, signaling that the right hand-side of the join can be pruned entirely (and thus the entire InlineJoin).
1 parent f126149 commit b5215da

File tree

5 files changed

+284
-32
lines changed

5 files changed

+284
-32
lines changed

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1386,6 +1386,45 @@ total_duration:long | day:date | days:date
13861386
17016205 |2023-10-23T13:00:00.000Z|[2023-10-23T12:00:00.000Z, 2023-10-23T13:00:00.000Z]
13871387
;
13881388

1389+
evalBeforeDoubleInlinestats1
1390+
required_capability: inlinestats_v9
1391+
1392+
FROM employees
1393+
| EVAL salaryK = salary/1000
1394+
| INLINESTATS count = COUNT(*) BY salaryK
1395+
| INLINESTATS min = MIN(MV_COUNT(languages)) BY salaryK
1396+
| SORT emp_no
1397+
| KEEP emp_no, still_hired, count
1398+
| LIMIT 5
1399+
;
1400+
1401+
emp_no:integer |still_hired:boolean|count:long
1402+
10001 |true |1
1403+
10002 |true |3
1404+
10003 |false |2
1405+
10004 |true |2
1406+
10005 |true |1
1407+
;
1408+
1409+
evalBeforeDoubleInlinestats2
1410+
required_capability: inlinestats_v9
1411+
1412+
FROM employees
1413+
| EVAL jobs = MV_COUNT(job_positions)
1414+
| INLINESTATS count = COUNT(*) BY jobs
1415+
| INLINESTATS min = MIN(MV_COUNT(languages)) BY jobs
1416+
| SORT emp_no
1417+
| KEEP emp_no, jobs, count, min
1418+
| LIMIT 5
1419+
;
1420+
1421+
emp_no:integer |jobs:integer|count:long|min:long
1422+
10001 |2 |18 |1
1423+
10002 |1 |27 |1
1424+
10003 |null |11 |1
1425+
10004 |4 |26 |1
1426+
10005 |null |11 |1
1427+
;
13891428

13901429
evalBeforeInlinestatsAndKeepAfter1
13911430
required_capability: inlinestats_v9

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import org.elasticsearch.xpack.esql.plan.logical.Aggregate;
2121
import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
2222
import org.elasticsearch.xpack.esql.plan.logical.UnaryPlan;
23-
import org.elasticsearch.xpack.esql.plan.logical.local.EmptyLocalSupplier;
2423
import org.elasticsearch.xpack.esql.plan.logical.local.LocalRelation;
2524
import org.elasticsearch.xpack.esql.plan.logical.local.LocalSupplier;
2625
import org.elasticsearch.xpack.esql.planner.PlannerUtils;
@@ -37,7 +36,7 @@ public PropagateEmptyRelation() {
3736
@Override
3837
protected LogicalPlan rule(UnaryPlan plan, LogicalOptimizerContext ctx) {
3938
LogicalPlan p = plan;
40-
if (plan.child() instanceof LocalRelation local && local.supplier() == EmptyLocalSupplier.EMPTY) {
39+
if (plan.child() instanceof LocalRelation local && local.hasEmptySupplier()) {
4140
// only care about non-grouped aggs might return something (count)
4241
if (plan instanceof Aggregate agg && agg.groupings().isEmpty()) {
4342
List<Block> emptyBlocks = aggsFromEmpty(ctx.foldCtx(), agg.aggregates());

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

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
import org.elasticsearch.xpack.esql.core.expression.Attribute;
1515
import org.elasticsearch.xpack.esql.core.expression.AttributeSet;
1616
import org.elasticsearch.xpack.esql.core.expression.Expressions;
17-
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
1817
import org.elasticsearch.xpack.esql.core.expression.NamedExpression;
1918
import org.elasticsearch.xpack.esql.core.util.Holder;
2019
import org.elasticsearch.xpack.esql.plan.logical.Aggregate;
@@ -79,7 +78,7 @@ private static LogicalPlan pruneColumns(LogicalPlan plan, AttributeSet.Builder u
7978
recheck.set(false);
8079
p = switch (p) {
8180
case Aggregate agg -> pruneColumnsInAggregate(agg, used, inlineJoin);
82-
case InlineJoin inj -> pruneColumnsInInlineJoin(inj, used, recheck);
81+
case InlineJoin inj -> pruneColumnsInInlineJoinRight(inj, used, recheck);
8382
case Eval eval -> pruneColumnsInEval(eval, used, recheck);
8483
case Project project -> inlineJoin ? pruneColumnsInProject(project, used) : p;
8584
case EsRelation esr -> pruneColumnsInEsRelation(esr, used);
@@ -148,12 +147,12 @@ private static LogicalPlan pruneColumnsInAggregate(Aggregate aggregate, Attribut
148147
return p;
149148
}
150149

151-
private static LogicalPlan pruneColumnsInInlineJoin(InlineJoin ij, AttributeSet.Builder used, Holder<Boolean> recheck) {
150+
private static LogicalPlan pruneColumnsInInlineJoinRight(InlineJoin ij, AttributeSet.Builder used, Holder<Boolean> recheck) {
152151
LogicalPlan p = ij;
153152

154153
used.addAll(ij.references());
155154
var right = pruneColumns(ij.right(), used, true);
156-
if (right.output().isEmpty()) {
155+
if (right.output().isEmpty() || isLocalEmptyRelation(right)) {
157156
p = ij.left();
158157
recheck.set(true);
159158
} else if (right != ij.right()) {
@@ -181,18 +180,19 @@ private static LogicalPlan pruneColumnsInEval(Eval eval, AttributeSet.Builder us
181180
return p;
182181
}
183182

183+
// Note: only run when the Project is a descendent of an InlineJoin.
184184
private static LogicalPlan pruneColumnsInProject(Project project, AttributeSet.Builder used) {
185185
LogicalPlan p = project;
186186

187+
// A project atop a stub relation will only output attributes which are already part of the INLINEJOIN left-hand side output.
188+
if (project.child() instanceof StubRelation) {
189+
// Use an empty relation as a marker for a subsequent pass over the InlineJoin.
190+
return emptyLocalRelation(project);
191+
}
192+
187193
var remaining = pruneUnusedAndAddReferences(project.projections(), used);
188194
if (remaining != null) {
189-
p = remaining.isEmpty() || remaining.stream().allMatch(FieldAttribute.class::isInstance)
190-
? emptyLocalRelation(project)
191-
: new Project(project.source(), project.child(), remaining);
192-
} else if (project.output().stream().allMatch(FieldAttribute.class::isInstance)) {
193-
// Use empty relation as a marker for a subsequent pass, in case the project is only outputting field attributes (which are
194-
// already part of the INLINEJOIN left-hand side output).
195-
p = emptyLocalRelation(project);
195+
p = remaining.isEmpty() ? emptyLocalRelation(project) : new Project(project.source(), project.child(), remaining);
196196
}
197197

198198
return p;
@@ -216,7 +216,11 @@ private static LogicalPlan pruneColumnsInEsRelation(EsRelation esr, AttributeSet
216216

217217
private static LogicalPlan emptyLocalRelation(LogicalPlan plan) {
218218
// create an empty local relation with no attributes
219-
return new LocalRelation(plan.source(), List.of(), EmptyLocalSupplier.EMPTY);
219+
return new LocalRelation(plan.source(), plan.output(), EmptyLocalSupplier.EMPTY);
220+
}
221+
222+
private static boolean isLocalEmptyRelation(LogicalPlan plan) {
223+
return plan instanceof LocalRelation local && local.hasEmptySupplier();
220224
}
221225

222226
/**

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/local/LocalRelation.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public void writeTo(StreamOutput out) throws IOException {
5555
if (out.getTransportVersion().onOrAfter(TransportVersions.ESQL_LOCAL_RELATION_WITH_NEW_BLOCKS)) {
5656
out.writeNamedWriteable(supplier);
5757
} else {
58-
if (supplier == EmptyLocalSupplier.EMPTY) {
58+
if (hasEmptySupplier()) {
5959
out.writeVInt(0);
6060
} else {// here we can only have an ImmediateLocalSupplier as this was the only implementation apart from EMPTY
6161
((ImmediateLocalSupplier) supplier).writeTo(out);
@@ -77,6 +77,10 @@ public LocalSupplier supplier() {
7777
return supplier;
7878
}
7979

80+
public boolean hasEmptySupplier() {
81+
return supplier == EmptyLocalSupplier.EMPTY;
82+
}
83+
8084
@Override
8185
public boolean expressionsResolved() {
8286
return true;

0 commit comments

Comments
 (0)