From 6422671bc54d5c6c1fda34766ef12dfef3e8f2cc Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Fri, 16 Jan 2026 14:40:42 +0100 Subject: [PATCH 1/2] WIP: simplify PruneColumns --- .../optimizer/rules/logical/PruneColumns.java | 50 ++++++++++--------- .../xpack/esql/rule/RuleExecutor.java | 4 +- 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PruneColumns.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PruneColumns.java index f80bc973d3fc8..871726311b615 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PruneColumns.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PruneColumns.java @@ -79,10 +79,10 @@ private static LogicalPlan pruneColumns(LogicalPlan plan, AttributeSet.Builder u do { recheck.set(false); p = switch (p) { - case Aggregate agg -> pruneColumnsInAggregate(agg, used, inlineJoin); - case InlineJoin inj -> pruneColumnsInInlineJoinRight(inj, used, recheck); + case Aggregate agg -> pruneColumnsInAggregate(agg, used, false); + case InlineJoin inj -> pruneColumnsInInlineJoin(inj, used, recheck); case Eval eval -> pruneColumnsInEval(eval, used, recheck); - case Project project -> inlineJoin ? pruneColumnsInProject(project, used, recheck) : p; + case Project project -> pruneColumnsInProject(project, used, recheck); case EsRelation esr -> pruneColumnsInEsRelation(esr, used); case Fork fork -> { forkPresent.set(true); @@ -139,33 +139,35 @@ private static LogicalPlan pruneColumnsInAggregate(Aggregate aggregate, Attribut return p; } - private static LogicalPlan pruneColumnsInInlineJoinRight(InlineJoin ij, AttributeSet.Builder used, Holder recheck) { + private static LogicalPlan pruneColumnsInInlineJoin(InlineJoin ij, AttributeSet.Builder used, Holder recheck) { LogicalPlan p = ij; - var right = pruneColumns(ij.right(), used, true); - if (right.output().isEmpty() || isLocalEmptyRelation(right)) { + used.addAll(ij.references()); + + var right = pruneColumns(ij.right(), used, false); + if (right.outputSet().subtract(ij.references()).isEmpty() || isLocalEmptyRelation(right)) { p = pruneRightSideAndProject(ij); recheck.set(true); } else if (right != ij.right()) { - if (right.anyMatch(plan -> plan instanceof Aggregate) == false) {// there is no aggregation on the right side anymore - if (right instanceof StubRelation) {// right is just a StubRelation, meaning nothing is needed from the right side - p = pruneRightSideAndProject(ij); - } else { - // if the right has no aggregation anymore, but it still has some other plans (evals, projects), - // we keep those and integrate them into the main plan. The InlineJoin is also replaced entirely. - p = InlineJoin.replaceStub(ij.left(), right); - p = new Project(ij.source(), p, mergeOutputExpressions(p.output(), ij.left().output())); - } - } else { - // if the right side has been updated, replace it - p = ij.replaceRight(right); - } - recheck.set(true); +// if (right.anyMatch(plan -> plan instanceof Aggregate) == false) {// there is no aggregation on the right side anymore +// if (right instanceof StubRelation) {// right is just a StubRelation, meaning nothing is needed from the right side +// p = pruneRightSideAndProject(ij); +// } else { +// // if the right has no aggregation anymore, but it still has some other plans (evals, projects), +// // we keep those and integrate them into the main plan. The InlineJoin is also replaced entirely. +// p = InlineJoin.replaceStub(ij.left(), right); +// p = new Project(ij.source(), p, mergeOutputExpressions(p.output(), ij.left().output())); +// } +// } else { +// // if the right side has been updated, replace it +// p = ij.replaceRight(right); +// } +// recheck.set(true); } - if (recheck.get() == false) { - used.addAll(p.references()); - } +// if (recheck.get() == false) { +// used.addAll(p.references()); +// } return p; } @@ -204,7 +206,7 @@ private static LogicalPlan pruneColumnsInProject(Project project, AttributeSet.B var remaining = pruneUnusedAndAddReferences(project.projections(), used); if (remaining != null) { - p = remaining.isEmpty() ? emptyLocalRelation(project) : new Project(project.source(), project.child(), remaining); + p = new Project(project.source(), project.child(), remaining); recheck.set(true); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/rule/RuleExecutor.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/rule/RuleExecutor.java index 7ee70b029031d..57b96802e7554 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/rule/RuleExecutor.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/rule/RuleExecutor.java @@ -193,8 +193,8 @@ protected final ExecutionInfo executeWithInfo(TreeType plan) { if (tf.hasChanged()) { hasChanged = true; - if (changeLog.isTraceEnabled()) { - changeLog.trace("Rule {} applied with change\n{}", rule, NodeUtils.diffString(tf.before, tf.after)); + if (true) { + changeLog.info("Rule {} applied with change\n{}", rule, NodeUtils.diffString(tf.before, tf.after)); } } else { if (log.isTraceEnabled()) { From e5a060ffa4dc232219c9b18e2424dbea4c48f02d Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Fri, 16 Jan 2026 13:55:51 +0000 Subject: [PATCH 2/2] [CI] Auto commit changes from spotless --- .../optimizer/rules/logical/PruneColumns.java | 36 +++++++++---------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PruneColumns.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PruneColumns.java index 871726311b615..cb4cc66066d8f 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PruneColumns.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PruneColumns.java @@ -27,7 +27,6 @@ import org.elasticsearch.xpack.esql.plan.logical.UnaryPlan; import org.elasticsearch.xpack.esql.plan.logical.UnionAll; import org.elasticsearch.xpack.esql.plan.logical.join.InlineJoin; -import org.elasticsearch.xpack.esql.plan.logical.join.StubRelation; import org.elasticsearch.xpack.esql.plan.logical.local.LocalRelation; import org.elasticsearch.xpack.esql.plan.logical.local.LocalSupplier; import org.elasticsearch.xpack.esql.planner.PlannerUtils; @@ -39,7 +38,6 @@ import java.util.Set; import java.util.stream.Collectors; -import static org.elasticsearch.xpack.esql.expression.NamedExpressions.mergeOutputExpressions; import static org.elasticsearch.xpack.esql.optimizer.rules.logical.PruneEmptyPlans.skipPlan; /** @@ -149,25 +147,25 @@ private static LogicalPlan pruneColumnsInInlineJoin(InlineJoin ij, AttributeSet. p = pruneRightSideAndProject(ij); recheck.set(true); } else if (right != ij.right()) { -// if (right.anyMatch(plan -> plan instanceof Aggregate) == false) {// there is no aggregation on the right side anymore -// if (right instanceof StubRelation) {// right is just a StubRelation, meaning nothing is needed from the right side -// p = pruneRightSideAndProject(ij); -// } else { -// // if the right has no aggregation anymore, but it still has some other plans (evals, projects), -// // we keep those and integrate them into the main plan. The InlineJoin is also replaced entirely. -// p = InlineJoin.replaceStub(ij.left(), right); -// p = new Project(ij.source(), p, mergeOutputExpressions(p.output(), ij.left().output())); -// } -// } else { -// // if the right side has been updated, replace it -// p = ij.replaceRight(right); -// } -// recheck.set(true); + // if (right.anyMatch(plan -> plan instanceof Aggregate) == false) {// there is no aggregation on the right side anymore + // if (right instanceof StubRelation) {// right is just a StubRelation, meaning nothing is needed from the right side + // p = pruneRightSideAndProject(ij); + // } else { + // // if the right has no aggregation anymore, but it still has some other plans (evals, projects), + // // we keep those and integrate them into the main plan. The InlineJoin is also replaced entirely. + // p = InlineJoin.replaceStub(ij.left(), right); + // p = new Project(ij.source(), p, mergeOutputExpressions(p.output(), ij.left().output())); + // } + // } else { + // // if the right side has been updated, replace it + // p = ij.replaceRight(right); + // } + // recheck.set(true); } -// if (recheck.get() == false) { -// used.addAll(p.references()); -// } + // if (recheck.get() == false) { + // used.addAll(p.references()); + // } return p; }