From efbca9b64f7f825738cf466ebf29f558ea0a86fb Mon Sep 17 00:00:00 2001 From: Bogdan Pintea Date: Mon, 4 Aug 2025 20:28:09 +0200 Subject: [PATCH 1/8] Add optimisation rule to pull OrderBy above InlineJoin --- .../src/main/resources/inlinestats.csv-spec | 106 +++++++++- .../esql/optimizer/LogicalPlanOptimizer.java | 2 + .../PullUpOrderByBeforeInlineJoin.java | 69 ++++++ .../xpack/esql/plan/logical/Sample.java | 2 +- .../optimizer/LogicalPlanOptimizerTests.java | 199 ++++++++++++++++++ 5 files changed, 369 insertions(+), 9 deletions(-) create mode 100644 x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PullUpOrderByBeforeInlineJoin.java diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/inlinestats.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/inlinestats.csv-spec index b90135f43a417..65dccf3cb5e82 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/inlinestats.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/inlinestats.csv-spec @@ -1,7 +1,3 @@ -// -// TODO: re-enable the commented tests once the Join functionality stabilizes -// - allFieldsReturned required_capability: inlinestats_v9 @@ -881,7 +877,6 @@ emp_no:integer | languages:integer | gender:keyword | max_lang:integer | y:keywo 10014 | 5 | null | 5 | null ; -// fails with AssertionError at org.elasticsearch.xpack.esql.plan.logical.Limit.writeTo(Limit.java:70) groupByMultipleRenamedColumns_AndOneExpression_Last required_capability: inlinestats_v9 @@ -905,7 +900,6 @@ emp_no:integer | languages:integer | gender:keyword|first_name:keyword|max_lang: 10010 |4 |null |Duangkaew |4 |null |4 |D ; -// fails with AssertionError at org.elasticsearch.xpack.esql.plan.logical.Limit.writeTo(Limit.java:70) groupByMultipleRenamedColumns_AndTwoExpressions required_capability: inlinestats_v9 @@ -929,7 +923,6 @@ emp_no:integer | languages:integer | gender:keyword|first_name:keyword|max_lang: 10010 |4 |null |Duangkaew |4 |D |null |D |4 ; -// fails with AssertionError at org.elasticsearch.xpack.esql.plan.logical.Limit.writeTo(Limit.java:70) groupByMultipleRenamedColumns_AndMultipleRenames required_capability: inlinestats_v9 @@ -954,7 +947,6 @@ emp_no:integer | languages:integer | gender:keyword| f:keyword |max_lang: 10010 |4 |null |Duangkaew |4 |null |4 |D ; -// fails with AssertionError at org.elasticsearch.xpack.esql.plan.logical.Limit.writeTo(Limit.java:70) groupByMultipleRenamedColumns_AndSameNameExpressionGroupingOverride required_capability: inlinestats_v9 @@ -1477,3 +1469,101 @@ ROW salary = 12300, emp_no = 5, gender = "F" emp_no:integer 5 ; + +sortBeforeInlinestats1 +required_capability: inlinestats_v9 + +ROW salary = 12300, emp_no = 5, gender = "F" +| EVAL salaryK = salary/1000 +| SORT salaryK DESC +| INLINESTATS sum = SUM(salaryK) BY gender +| KEEP emp_no +; + +emp_no:integer +5 +; + +sortBeforeInlinestats2 +required_capability: inlinestats_v9 + +FROM employees +| SORT emp_no +| EVAL salaryK = salary/1000 +| INLINESTATS count = COUNT(*) BY salaryK +| KEEP emp_no, still_hired, count +| LIMIT 5 +; + +emp_no:integer |still_hired:boolean|count:long +10001 |true |1 +10002 |true |3 +10003 |false |2 +10004 |true |2 +10005 |true |1 +; + +// fails with: java.lang.AssertionError: expected no concrete indices without data node plan +sortBeforeInlinestats3-Ignore +required_capability: inlinestats_v9 + +FROM employees +| SORT languages DESC +| EVAL salaryK = salary/1000 +| INLINESTATS count = COUNT(*) BY salaryK +| SORT emp_no +| INLINESTATS min = MIN(MV_COUNT(languages)) BY salaryK +| KEEP emp_no, still_hired, count +| LIMIT 5 +; + +emp_no:integer |still_hired:boolean|count:long +10001 |true |1 +10002 |true |3 +10003 |false |2 +10004 |true |2 +10005 |true |1 +; + +// same as `afterLookup`, swapped SORT position +sortBeforeInlinestatsAndLookupJoin +required_capability: inlinestats_v9 +required_capability: join_lookup_v12 + +FROM airports +| EVAL backup_scalerank = scalerank +| RENAME scalerank AS language_code +| SORT abbrev DESC +| LOOKUP JOIN languages_lookup ON language_code +| RENAME language_name as scalerank +| DROP language_code +| INLINESTATS count=COUNT(*) BY scalerank +| KEEP abbrev, *scalerank +| LIMIT 5 +; + +abbrev:keyword |backup_scalerank:integer| scalerank:keyword +null |8 |null +null |8 |null +null |8 |null +ZRH |3 |Spanish +ZNZ |4 |German +; + +// same as `shadowingAggregateByNextGrouping`, swapped SORT position +sortBeforeInlinestats +required_capability: inlinestats_v9 + +FROM employees +| KEEP gender, languages, emp_no, salary +| SORT emp_no +| INLINESTATS gender = count_distinct(gender) BY languages +| INLINESTATS avg(salary) BY gender +| LIMIT 3 +; + +emp_no:integer |salary:integer |languages:integer|avg(salary):double|gender:long +10001 |57305 |2 |48248.55 |2 +10002 |56371 |5 |48248.55 |2 +10003 |61805 |4 |48248.55 |2 +; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizer.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizer.java index dac533f872022..d1995de088026 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizer.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizer.java @@ -34,6 +34,7 @@ import org.elasticsearch.xpack.esql.optimizer.rules.logical.PruneRedundantOrderBy; import org.elasticsearch.xpack.esql.optimizer.rules.logical.PruneRedundantSortClauses; import org.elasticsearch.xpack.esql.optimizer.rules.logical.PruneUnusedIndexMode; +import org.elasticsearch.xpack.esql.optimizer.rules.logical.PullUpOrderByBeforeInlineJoin; import org.elasticsearch.xpack.esql.optimizer.rules.logical.PushDownAndCombineFilters; import org.elasticsearch.xpack.esql.optimizer.rules.logical.PushDownAndCombineLimits; import org.elasticsearch.xpack.esql.optimizer.rules.logical.PushDownAndCombineOrderBy; @@ -203,6 +204,7 @@ protected static Batch operators(boolean local) { new PushDownAndCombineOrderBy(), new PruneRedundantOrderBy(), new PruneRedundantSortClauses(), + new PullUpOrderByBeforeInlineJoin(), new PruneLeftJoinOnNullMatchingField() ); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PullUpOrderByBeforeInlineJoin.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PullUpOrderByBeforeInlineJoin.java new file mode 100644 index 0000000000000..a961ce1a0d903 --- /dev/null +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PullUpOrderByBeforeInlineJoin.java @@ -0,0 +1,69 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ +package org.elasticsearch.xpack.esql.optimizer.rules.logical; + +import org.elasticsearch.xpack.esql.plan.logical.Limit; +import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan; +import org.elasticsearch.xpack.esql.plan.logical.OrderBy; +import org.elasticsearch.xpack.esql.plan.logical.SortAgnostic; +import org.elasticsearch.xpack.esql.plan.logical.join.InlineJoin; + +/** + * Pulls "up" an {@link OrderBy} node that is not preceded by a {@link Limit}, but is preceded by an {@link InlineJoin}. + * The InlineJoin is {@link SortAgnostic}, so the OrderBy can be pulled up without affecting the semantics of the join. + * This is needed since otherwise the OrderBy would remain to be executed unbounded, which isn't supported. + * If it's preceded by a {@link Limit}, it will be merged into a {@link org.elasticsearch.xpack.esql.plan.logical.TopN} later in the + * "cleanup" optimization stage. + */ +public final class PullUpOrderByBeforeInlineJoin extends OptimizerRules.OptimizerRule { + + @Override + protected LogicalPlan rule(LogicalPlan plan) { + return plan.transformUp(LogicalPlan.class, PullUpOrderByBeforeInlineJoin::pullUpOrderByBeforeInlineJoin); + } + + private static LogicalPlan pullUpOrderByBeforeInlineJoin(LogicalPlan plan) { + if (plan instanceof InlineJoin inlineJoin) { + OrderBy orderBy = findOrderByNotPrecededByLimit(inlineJoin); + if (orderBy != null) { + LogicalPlan newInlineJoin = removeOrderBy(inlineJoin, orderBy); + return new OrderBy(orderBy.source(), newInlineJoin, orderBy.order()); + } + } + return plan; + } + + // Finds an OrderBy node in the subtree of the provided plan that is not preceded by a Limit + private static OrderBy findOrderByNotPrecededByLimit(LogicalPlan plan) { + if (plan instanceof Limit) { + return null; + } + if (plan instanceof OrderBy orderBy) { + return orderBy; + } + for (LogicalPlan child : plan.children()) { + if (child instanceof SortAgnostic) { + OrderBy found = findOrderByNotPrecededByLimit(child); + if (found != null) { + return found; + } + } + } + return null; + } + + // Removes the found OrderBy node from its current position in the subtree + private static LogicalPlan removeOrderBy(LogicalPlan plan, OrderBy orderBy) { + if (plan == orderBy) { + return orderBy.child(); + } + if (plan.children().isEmpty()) { + return plan; + } + return plan.replaceChildren(plan.children().stream().map(child -> removeOrderBy(child, orderBy)).toList()); + } +} diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Sample.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Sample.java index c9c139bf77148..1fe88aab56935 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Sample.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Sample.java @@ -18,7 +18,7 @@ import java.io.IOException; import java.util.Objects; -public class Sample extends UnaryPlan implements TelemetryAware { +public class Sample extends UnaryPlan implements SortAgnostic, TelemetryAware { public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(LogicalPlan.class, "Sample", Sample::new); private final Expression probability; diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java index 4c707ca977501..4b7810f31c069 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java @@ -6013,6 +6013,205 @@ public void testInlinestatsWithRow() { ); } + /* + * TopN[[Order[emp_no{f}#8,DESC,FIRST]],1000[INTEGER]] + * \_Filter[emp_no{f}#8 > 1000[INTEGER]] + * \_InlineJoin[LEFT,[emp_no{f}#8],[emp_no{f}#8],[emp_no{r}#8]] + * |_EsRelation[test][_meta_field{f}#14, emp_no{f}#8, first_name{f}#9, ge..] + * \_Project[[avg{r}#5, emp_no{f}#8]] + * \_Eval[[$$SUM$avg$0{r$}#19 / $$COUNT$avg$1{r$}#20 AS avg#5]] + * \_Aggregate[[emp_no{f}#8],[SUM(salary{f}#13,true[BOOLEAN]) AS $$SUM$avg$0#19, COUNT(salary{f}#13,true[BOOLEAN]) AS $$COUNT$ + * avg$1#20, emp_no{f}#8]] + * \_StubRelation[[_meta_field{f}#14, emp_no{f}#8, first_name{f}#9, gender{f}#10, hire_date{f}#15, job{f}#16, job.raw{f}#17, + * languages{f}#11, last_name{f}#12, long_noidx{f}#18, salary{f}#13]] + */ + public void testInlinestatsAfterSort() { + var query = """ + FROM employees + | SORT emp_no DESC + | INLINESTATS avg = AVG(salary) BY emp_no + | WHERE emp_no > 1000 // to avoid an existing issue with LIMIT injection past INLINESTATS (which masks the issue the test tests) + """; + if (releaseBuildForInlinestats(query)) { + return; + } + var plan = optimizedPlan(query); + + var topN = as(plan, TopN.class); + assertThat(topN.order().size(), is(1)); + var order = as(topN.order().get(0), Order.class); + assertThat(order.direction(), equalTo(Order.OrderDirection.DESC)); + assertThat(order.nullsPosition(), equalTo(Order.NullsPosition.FIRST)); + assertThat(Expressions.name(order.child()), equalTo("emp_no")); + assertThat(topN.limit().fold(FoldContext.small()), equalTo(1000)); + + var filter = as(topN.child(), Filter.class); + var filterCondition = as(filter.condition(), GreaterThan.class); + assertThat(Expressions.name(filterCondition.left()), equalTo("emp_no")); + assertThat(filterCondition.right().fold(FoldContext.small()), equalTo(1000)); + + var inlineJoin = as(filter.child(), InlineJoin.class); + assertThat(Expressions.names(inlineJoin.config().matchFields()), is(List.of("emp_no"))); + // Left + var relation = as(inlineJoin.left(), EsRelation.class); + assertThat(relation.concreteIndices(), is(Set.of("test"))); + // Right + var project = as(inlineJoin.right(), Project.class); + var eval = as(project.child(), Eval.class); + assertThat(Expressions.names(eval.fields()), is(List.of("avg"))); + var agg = as(eval.child(), Aggregate.class); + assertMap(Expressions.names(agg.output()), is(List.of("$$SUM$avg$0", "$$COUNT$avg$1", "emp_no"))); + var stub = as(agg.child(), StubRelation.class); + } + + /* + * TopN[[Order[emp_no{f}#18,DESC,FIRST]],1000[INTEGER]] + * \_Filter[emp_no{f}#18 > 1000[INTEGER]] + * \_InlineJoin[LEFT,[emp_no{f}#18],[emp_no{f}#18],[emp_no{r}#18]] + * |_EsqlProject[[_meta_field{f}#24, emp_no{f}#18, first_name{f}#19, gender{f}#20, hire_date{f}#25, job{f}#26, job.raw{f}#27, + * languages{r}#29, last_name{f}#22 AS lName#12, long_noidx{f}#28, salary{f}#23, msg{r}#5, salaryK{r}#9]] + * | \_Eval[[salary{f}#23 / 1000[INTEGER] AS salaryK#9]] + * | \_Dissect[first_name{f}#19,Parser[pattern=%{msg}, appendSeparator=, + * parser=org.elasticsearch.dissect.DissectParser@2aa687d7],[msg{r}#5]] + * | \_Sample[0.1[DOUBLE]] + * | \_MvExpand[languages{f}#21,languages{r}#29] + * | \_EsRelation[test][_meta_field{f}#24, emp_no{f}#18, first_name{f}#19, ..] + * \_Project[[avg{r}#15, emp_no{f}#18]] + * \_Eval[[$$SUM$avg$0{r$}#30 / $$COUNT$avg$1{r$}#31 AS avg#15]] + * \_Aggregate[[emp_no{f}#18],[SUM(salary{f}#23,true[BOOLEAN]) AS $$SUM$avg$0#30, COUNT(salary{f}#23,true[BOOLEAN]) AS + * $$COUNT$avg$1#31, emp_no{f}#18]] + * \_StubRelation[[_meta_field{f}#24, emp_no{f}#18, first_name{f}#19, gender{f}#20, hire_date{f}#25, job{f}#26, job.raw{f}#27, + * anguages{r}#29, lName{r}#12, long_noidx{f}#28, salary{f}#23, msg{r}#5, salaryK{r}#9]] + */ + public void testInlinestatsAfterSortAndSortAgnostic() { + var query = """ + FROM employees + | SORT emp_no DESC + | MV_EXPAND languages + | SAMPLE .1 + | DISSECT first_name "%{msg}" + | EVAL salaryK = salary / 1000 + | RENAME last_name AS lName + | INLINESTATS avg = AVG(salary) BY emp_no + | WHERE emp_no > 1000 // to avoid an existing issue with LIMIT injection past INLINESTATS (which masks the issue the test tests) + """; + if (releaseBuildForInlinestats(query)) { + return; + } + var plan = optimizedPlan(query); + + var topN = as(plan, TopN.class); + assertThat(topN.order().size(), is(1)); + var order = as(topN.order().get(0), Order.class); + assertThat(order.direction(), equalTo(Order.OrderDirection.DESC)); + assertThat(order.nullsPosition(), equalTo(Order.NullsPosition.FIRST)); + var field = as(order.child(), FieldAttribute.class); + assertThat(field.name(), equalTo("emp_no")); + assertThat(topN.limit().fold(FoldContext.small()), equalTo(1000)); + + var filter = as(topN.child(), Filter.class); + var filterCondition = as(filter.condition(), GreaterThan.class); + assertThat(Expressions.name(filterCondition.left()), equalTo("emp_no")); + assertThat(filterCondition.right().fold(FoldContext.small()), equalTo(1000)); + + var inlineJoin = as(filter.child(), InlineJoin.class); + assertThat(Expressions.names(inlineJoin.config().matchFields()), is(List.of("emp_no"))); + // Left + var esqlProject = as(inlineJoin.left(), EsqlProject.class); + + var eval = as(esqlProject.child(), Eval.class); + assertThat(Expressions.names(eval.fields()), is(List.of("salaryK"))); + + var dissect = as(eval.child(), Dissect.class); + assertThat(dissect.parser().pattern(), is("%{msg}")); + assertThat(Expressions.name(dissect.input()), is("first_name")); + + var sample = as(dissect.child(), Sample.class); + assertThat(sample.probability().fold(FoldContext.small()), equalTo(0.1)); + + var mvExpand = as(sample.child(), MvExpand.class); + assertThat(Expressions.name(mvExpand.target()), is("languages")); + + var esRelation = as(mvExpand.child(), EsRelation.class); + + // Right + var project = as(inlineJoin.right(), Project.class); + eval = as(project.child(), Eval.class); + assertThat(Expressions.names(eval.fields()), is(List.of("avg"))); + var agg = as(eval.child(), Aggregate.class); + assertMap(Expressions.names(agg.output()), is(List.of("$$SUM$avg$0", "$$COUNT$avg$1", "emp_no"))); + var stub = as(agg.child(), StubRelation.class); + } + + /* + * TopN[[Order[salary{f}#18,ASC,LAST]],1000[INTEGER]] + * \_Filter[emp_no{f}#13 > 1000[INTEGER]] + * \_InlineJoin[LEFT,[emp_no{f}#13],[emp_no{f}#13],[emp_no{r}#13]] + * |_InlineJoin[LEFT,[languages{f}#16],[languages{f}#16],[languages{r}#16]] + * | |_EsRelation[test][_meta_field{f}#19, emp_no{f}#13, first_name{f}#14, ..] + * | \_Aggregate[[languages{f}#16],[MIN(salary{f}#18,true[BOOLEAN]) AS min#5, languages{f}#16]] + * | \_StubRelation[[_meta_field{f}#19, emp_no{f}#13, first_name{f}#14, gender{f}#15, hire_date{f}#20, job{f}#21, job.raw{f}#22, + * languages{f}#16, last_name{f}#17, long_noidx{f}#23, salary{f}#18]] + * \_Project[[avg{r}#10, emp_no{f}#13]] + * \_Eval[[$$SUM$avg$0{r$}#24 / $$COUNT$avg$1{r$}#25 AS avg#10]] + * \_Aggregate[[emp_no{f}#13],[SUM(salary{f}#18,true[BOOLEAN]) AS $$SUM$avg$0#24, COUNT(salary{f}#18,true[BOOLEAN]) AS + * $$COUNT$avg$1#25, emp_no{f}#13]] + * \_StubRelation[[_meta_field{f}#19, emp_no{f}#13, first_name{f}#14, gender{f}#15, hire_date{f}#20, job{f}#21, job.raw{f}#22, + * ast_name{f}#17, long_noidx{f}#23, salary{f}#18, min{r}#5, languages{f}#16]] + */ + public void testInlinestatsAfterSortDoubled() { + var query = """ + FROM employees + | SORT emp_no DESC // going to be dropped + | INLINESTATS min = MIN(salary) BY languages + | SORT salary ASC + | INLINESTATS avg = AVG(salary) BY emp_no + | WHERE emp_no > 1000 // to avoid an existing issue with LIMIT injection past INLINESTATS (which masks the issue the test tests) + """; + if (releaseBuildForInlinestats(query)) { + return; + } + var plan = optimizedPlan(query); + + var topN = as(plan, TopN.class); + assertThat(topN.order().size(), is(1)); + var order = as(topN.order().get(0), Order.class); + assertThat(order.direction(), equalTo(Order.OrderDirection.ASC)); + assertThat(order.nullsPosition(), equalTo(Order.NullsPosition.LAST)); + var field = as(order.child(), FieldAttribute.class); + assertThat(field.name(), equalTo("salary")); + assertThat(topN.limit().fold(FoldContext.small()), equalTo(1000)); + + var filter = as(topN.child(), Filter.class); + var filterCondition = as(filter.condition(), GreaterThan.class); + assertThat(Expressions.name(filterCondition.left()), equalTo("emp_no")); + assertThat(filterCondition.right().fold(FoldContext.small()), equalTo(1000)); + + var inlineJoin = as(filter.child(), InlineJoin.class); + assertThat(Expressions.names(inlineJoin.config().matchFields()), is(List.of("emp_no"))); + // outer left + var inlineJoinLeft = as(inlineJoin.left(), InlineJoin.class); + // inner left + var relation = as(inlineJoinLeft.left(), EsRelation.class); + assertThat(relation.concreteIndices(), is(Set.of("test"))); + // inner right + var agg = as(inlineJoinLeft.right(), Aggregate.class); + var groupings = agg.groupings(); + assertThat(groupings.size(), is(1)); + var fieldAttribute = as(groupings.get(0), FieldAttribute.class); + assertThat(fieldAttribute.name(), is("languages")); + var aggs = agg.aggregates(); + assertThat(aggs.get(0).toString(), is("MIN(salary) AS min#5")); + var stub = as(agg.child(), StubRelation.class); + // outer right + var project = as(inlineJoin.right(), Project.class); + var eval = as(project.child(), Eval.class); + assertThat(Expressions.names(eval.fields()), is(List.of("avg"))); + agg = as(eval.child(), Aggregate.class); + assertMap(Expressions.names(agg.output()), is(List.of("$$SUM$avg$0", "$$COUNT$avg$1", "emp_no"))); + stub = as(agg.child(), StubRelation.class); + } + /** * Expects * From b470b007f056d1679eb60ee97ca763858038729f Mon Sep 17 00:00:00 2001 From: Bogdan Pintea Date: Tue, 5 Aug 2025 10:56:44 +0200 Subject: [PATCH 2/8] Fix existing test, bump cap --- .../xpack/esql/ccq/MultiClusterSpecIT.java | 4 +- .../src/main/resources/inlinestats.csv-spec | 186 +++++++++--------- .../xpack/esql/action/EsqlCapabilities.java | 2 +- .../xpack/esql/analysis/AnalyzerTests.java | 2 +- .../optimizer/LogicalPlanOptimizerTests.java | 52 ++++- .../logical/PropagateInlineEvalsTests.java | 4 +- .../esql/session/FieldNameUtilsTests.java | 30 +-- 7 files changed, 165 insertions(+), 115 deletions(-) diff --git a/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/MultiClusterSpecIT.java b/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/MultiClusterSpecIT.java index 5c6d0186cdf77..81fa3395f6bcd 100644 --- a/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/MultiClusterSpecIT.java +++ b/x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/MultiClusterSpecIT.java @@ -51,7 +51,7 @@ import static org.elasticsearch.xpack.esql.action.EsqlCapabilities.Cap.ENABLE_LOOKUP_JOIN_ON_REMOTE; import static org.elasticsearch.xpack.esql.action.EsqlCapabilities.Cap.FORK_V9; import static org.elasticsearch.xpack.esql.action.EsqlCapabilities.Cap.INLINESTATS; -import static org.elasticsearch.xpack.esql.action.EsqlCapabilities.Cap.INLINESTATS_V9; +import static org.elasticsearch.xpack.esql.action.EsqlCapabilities.Cap.INLINESTATS_V10; import static org.elasticsearch.xpack.esql.action.EsqlCapabilities.Cap.JOIN_LOOKUP_V12; import static org.elasticsearch.xpack.esql.action.EsqlCapabilities.Cap.JOIN_PLANNING_V1; import static org.elasticsearch.xpack.esql.action.EsqlCapabilities.Cap.METADATA_FIELDS_REMOTE_TEST; @@ -135,7 +135,7 @@ protected void shouldSkipTest(String testName) throws IOException { assumeTrue("Test " + testName + " is skipped on " + oldVersion, isEnabled(testName, instructions, oldVersion)); assumeFalse("INLINESTATS not yet supported in CCS", testCase.requiredCapabilities.contains(INLINESTATS.capabilityName())); assumeFalse("INLINESTATS not yet supported in CCS", testCase.requiredCapabilities.contains(JOIN_PLANNING_V1.capabilityName())); - assumeFalse("INLINESTATS not yet supported in CCS", testCase.requiredCapabilities.contains(INLINESTATS_V9.capabilityName())); + assumeFalse("INLINESTATS not yet supported in CCS", testCase.requiredCapabilities.contains(INLINESTATS_V10.capabilityName())); if (testCase.requiredCapabilities.contains(JOIN_LOOKUP_V12.capabilityName())) { assumeTrue( "LOOKUP JOIN not yet supported in CCS", diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/inlinestats.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/inlinestats.csv-spec index 65dccf3cb5e82..ea2ac8217cf6f 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/inlinestats.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/inlinestats.csv-spec @@ -1,5 +1,5 @@ allFieldsReturned -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM hosts METADATA _index | INLINESTATS c = COUNT(*) BY host_group @@ -12,7 +12,7 @@ eth0 |epsilon gw instance|epsilon |[fe80::cae2:65ff:fece:feb9, ; maxOfInt -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 // tag::max-languages[] FROM employees | KEEP emp_no, languages @@ -34,7 +34,7 @@ emp_no:integer | languages:integer | max_lang:integer ; maxOfIntByKeyword -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no, languages, gender @@ -52,7 +52,7 @@ emp_no:integer | languages:integer | max_lang:integer | gender:keyword ; maxOfLongByKeyword -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no, avg_worked_seconds, gender @@ -67,7 +67,7 @@ emp_no:integer | avg_worked_seconds:long | max_avg_worked_seconds:long | gender: ; maxOfLong -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no, avg_worked_seconds, gender @@ -80,7 +80,7 @@ emp_no:integer | avg_worked_seconds:long | gender:keyword | max_avg_worked_secon ; maxOfLongByCalculatedKeyword -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 // tag::longest-tenured-by-first[] FROM employees @@ -103,7 +103,7 @@ emp_no:integer | avg_worked_seconds:long | last_name:keyword | max_avg_worked_se ; maxOfLongByCalculatedNamedKeyword -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no, avg_worked_seconds, last_name @@ -122,7 +122,7 @@ emp_no:integer | avg_worked_seconds:long | last_name:keyword | max_avg_worked_se ; maxOfLongByCalculatedDroppedKeyword -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | INLINESTATS max_avg_worked_seconds = MAX(avg_worked_seconds) BY l = SUBSTRING(last_name, 0, 1) @@ -141,7 +141,7 @@ emp_no:integer | avg_worked_seconds:long | last_name:keyword | max_avg_worked_se ; maxOfLongByEvaledKeyword -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | EVAL l = SUBSTRING(last_name, 0, 1) @@ -161,7 +161,7 @@ emp_no:integer | avg_worked_seconds:long | max_avg_worked_seconds:long | l:keywo ; maxOfLongByInt -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no, avg_worked_seconds, languages @@ -179,7 +179,7 @@ emp_no:integer | avg_worked_seconds:long | max_avg_worked_seconds:long | languag ; maxOfLongByIntDouble -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no, avg_worked_seconds, languages, height @@ -197,7 +197,7 @@ emp_no:integer | avg_worked_seconds:long | max_avg_worked_seconds:long | languag ; two -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no, languages, avg_worked_seconds, gender @@ -221,7 +221,7 @@ emp_no:integer |avg_worked_seconds:long|avg_avg_worked_seconds:double|languages: ; three -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 // used to fail with AssertionError at org.elasticsearch.xpack.esql.plan.logical.Limit.writeTo(Limit.java:70) FROM employees @@ -247,9 +247,8 @@ emp_no:integer |avg_worked_seconds:long|avg_avg_worked_seconds:double|languages: 10023 |330870342 |3.181719481E8 |null |5 |3 |5 |F ; -// TODO: INLINESTATS unit test needed for this one pushDownSort_To_LeftSideOnly -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 from employees | sort emp_no @@ -258,16 +257,17 @@ from employees | keep emp_no, avg, languages, gender ; - emp_no:integer| avg:double |languages:integer|gender:keyword -10001 |57305.0 |2 |M -10002 |46272.5 |5 |F -10003 |61805.0 |4 |M -10004 |46272.5 |5 |M -10005 |63528.0 |1 |M + emp_no:integer| avg:double |languages:integer|gender:keyword +10001 |48178.84210526316 |2 |M +10002 |41680.76190476191 |5 |F +10003 |47733.0 |4 |M +10004 |41680.76190476191 |5 |M +10005 |50576.666666666664|1 |M + ; byMultivaluedSimple -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 // tag::mv-group[] FROM airports @@ -285,7 +285,7 @@ abbrev:keyword | type:keyword | scalerank:integer | min_scalerank:integer ; byMultivaluedMvExpand -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 // tag::mv-expand[] FROM airports @@ -305,7 +305,7 @@ GWL |9 |4 |military ; byMvExpand -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 // tag::extreme-airports[] FROM airports @@ -334,7 +334,7 @@ FROM airports ; mvMinMvExpand -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM airports | EVAL original_type = type @@ -357,7 +357,7 @@ ZAR |Zaria |POINT (7.7 11.0667) |Nigeria |POINT ( ; afterStats -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM airports | STATS count=COUNT(*) BY country @@ -380,7 +380,7 @@ count:long | country:keyword | avg:double ; afterWhere -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM airports | WHERE country != "United States" @@ -398,7 +398,7 @@ abbrev:keyword | country:keyword | count:long ; afterLookup -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 required_capability: join_lookup_v12 FROM airports @@ -422,7 +422,7 @@ ZNZ |4 |German ; afterEnrich -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 required_capability: enrich_load FROM airports @@ -443,7 +443,7 @@ abbrev:keyword | city:keyword | "COUNT(*)":long | region:text ; beforeStats -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM airports | EVAL lat = ST_Y(location) @@ -456,7 +456,7 @@ northern:long | southern:long ; beforeKeepSort -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | INLINESTATS max_salary = MAX(salary) by languages @@ -471,7 +471,7 @@ emp_no:integer | languages:integer | max_salary:integer ; beforeKeepWhere -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | INLINESTATS max_salary = MAX(salary) by languages @@ -484,7 +484,7 @@ emp_no:integer | languages:integer | max_salary:integer ; beforeEnrich -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 required_capability: enrich_load FROM airports @@ -503,7 +503,7 @@ ACA |Acapulco de Juárez|385 |major |Acapulco de ; beforeAndAfterEnrich -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 required_capability: enrich_load FROM airports @@ -526,7 +526,7 @@ ALL |Albenga |499 |mid |1 ; shadowing -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 ROW left = "left", client_ip = "172.21.0.5", env = "env", right = "right" | INLINESTATS env = VALUES(right) BY client_ip @@ -537,7 +537,7 @@ left | right | right | 172.21.0.5 ; shadowingMulti -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 ROW left = "left", airport = "Zurich Airport ZRH", city = "Zürich", middle = "middle", region = "North-East Switzerland", right = "right" | INLINESTATS airport=VALUES(left), region=VALUES(left), city_boundary=VALUES(left) BY city @@ -548,7 +548,7 @@ left | middle | right | left | left ; shadowingSelf -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 ROW city = "Raleigh" | INLINESTATS city = COUNT(city) @@ -559,7 +559,7 @@ city:long ; shadowingSelfBySelf -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 ROW city = "Raleigh" | INLINESTATS city = COUNT(city) BY city @@ -571,7 +571,7 @@ Raleigh ; shadowingInternal -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 ROW city = "Zürich" | INLINESTATS x = VALUES(city), x = VALUES(city) @@ -583,7 +583,7 @@ Zürich | Zürich ; multiInlinestatsWithRow -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 row x = 1 | inlinestats x = max(x) + min(x) @@ -597,7 +597,7 @@ row x = 1 ; ignoreUnusedEvaledValue_AndInlineStats -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 ROW x = 1 | INLINESTATS max(x) @@ -610,7 +610,7 @@ x:integer ; ignoreUnusedEvaledValue_AndInlineStats2 -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 ROW x = 1, z = 2 | INLINESTATS max(x) @@ -623,7 +623,7 @@ x:integer | z:integer ; ignoreUnusedEvaledValue_AndInlineStats3 -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 from employees | inlinestats max(salary) @@ -638,7 +638,7 @@ from employees ; ignoreUnusedEvaledValue_AndInlineStats4 -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 from employees | inlinestats max(salary), m = min(salary) by gender @@ -653,7 +653,7 @@ emp_no:integer ; ignoreUnusedEvaledValue_AndInlineStats5 -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 from employees | inlinestats max(salary), m = min(salary) by gender @@ -668,7 +668,7 @@ emp_no:integer ; shadowEntireInlinestats -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | INLINESTATS x = avg(salary), y = min(salary) BY emp_no @@ -683,7 +683,7 @@ x:integer |y:integer |emp_no:integer ; byConstant -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no, languages @@ -702,7 +702,7 @@ emp_no:integer | languages:integer | max_lang:integer | y:integer ; aggConstant -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no @@ -720,7 +720,7 @@ one:integer | emp_no:integer ; percentile -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no, salary @@ -739,7 +739,7 @@ emp_no:integer | salary:integer | ninety_fifth_salary:double ; byTwoCalculated -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM airports | WHERE abbrev IS NOT NULL @@ -759,7 +759,7 @@ abbrev:keyword | scalerank:integer | location:geo_point byTwoCalculatedSecondOverwrites required_capability: stats_alias_collision_warnings -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM airports | WHERE abbrev IS NOT NULL @@ -780,7 +780,7 @@ abbrev:keyword | scalerank:integer | location:geo_point byTwoCalculatedSecondOverwritesReferencingFirst required_capability: stats_alias_collision_warnings -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM airports | WHERE abbrev IS NOT NULL @@ -803,7 +803,7 @@ abbrev:keyword | scalerank:integer | location:geo_point groupShadowsAgg required_capability: stats_alias_collision_warnings -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM airports | WHERE abbrev IS NOT NULL @@ -823,7 +823,7 @@ abbrev:keyword | scalerank:integer | location:geo_point ; groupShadowsField -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no, salary, hire_date @@ -842,7 +842,7 @@ emp_no:integer | salary:integer | avg_salary:double | hire_date:datetime ; groupByExpression_And_ExistentField -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no, languages, gender | EVAL x = "ABC" @@ -860,7 +860,7 @@ emp_no:integer | languages:integer | x:keyword | max_lang:integer | y:keyword | ; groupByRenamedColumn -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no, languages, gender | INLINESTATS max_lang = MAX(languages) BY y = gender @@ -878,7 +878,7 @@ emp_no:integer | languages:integer | gender:keyword | max_lang:integer | y:keywo ; groupByMultipleRenamedColumns_AndOneExpression_Last -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no, languages, gender, first_name @@ -901,7 +901,7 @@ emp_no:integer | languages:integer | gender:keyword|first_name:keyword|max_lang: ; groupByMultipleRenamedColumns_AndTwoExpressions -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no, languages, gender, first_name @@ -924,7 +924,7 @@ emp_no:integer | languages:integer | gender:keyword|first_name:keyword|max_lang: ; groupByMultipleRenamedColumns_AndMultipleRenames -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no, languages, gender, first_name @@ -948,7 +948,7 @@ emp_no:integer | languages:integer | gender:keyword| f:keyword |max_lang: ; groupByMultipleRenamedColumns_AndSameNameExpressionGroupingOverride -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no, languages, gender, first_name @@ -972,7 +972,7 @@ emp_no:integer | languages:integer | gender:keyword|max_lang:integer| y:keyword ; twoAggregatesGroupedBy_AField_And_AnExpression -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no, languages, gender, last_name @@ -994,7 +994,7 @@ emp_no:integer |languages:integer|last_name:keyword|max_lang:integer|min_lang:in ; groupByMultipleRenamedColumns_InversedOrder -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no, languages, still_hired, gender @@ -1012,7 +1012,7 @@ emp_no:integer |languages:integer|still_hired:boolean| gender:keyword|max_lang:i ; groupByMultipleRenamedColumns_InversedOrder_ComplexEval -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no, languages, still_hired, gender @@ -1031,7 +1031,7 @@ emp_no:integer |languages:integer|still_hired:boolean| gender:keyword|multilingu ; groupByMultipleRenamedColumns_AndComplexEval -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no, languages, still_hired, gender @@ -1051,7 +1051,7 @@ emp_no:integer |languages:integer|still_hired:boolean| gender:keyword|multilingu // fails with AssertionError at org.elasticsearch.xpack.esql.plan.logical.Limit.writeTo(Limit.java:70) groupByMultipleRenamedColumns_AndConstantValue -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no, languages, gender, first_name @@ -1075,7 +1075,7 @@ emp_no:integer |languages:integer|gender:keyword |first_name:keyword | x:keyw ; groupByRenamedExpression -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP emp_no, languages, gender, last_name @@ -1097,7 +1097,7 @@ emp_no:integer |languages:integer|last_name:keyword|max_lang:integer|min_lang:in ; doubleFilterOnLeftAndRight_InlineStats_Sides -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | INLINESTATS max_salary = MAX(salary), min_salary = MIN(salary) by languages @@ -1118,7 +1118,7 @@ emp_no:integer |languages:integer|salary:integer |max_salary:integer|min_salary: ; filterOnInlineStatsAggs -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | INLINESTATS max_salary = MAX(salary), min_salary = MIN(salary) by languages @@ -1137,7 +1137,7 @@ emp_no:integer |languages:integer|salary:integer |max_salary:integer|min_salary: ; filterOnInlineStatsAggsValues_And_Groupings -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | INLINESTATS max_salary = MAX(salary), min_salary = MIN(salary) by languages @@ -1156,7 +1156,7 @@ emp_no:integer |languages:integer|salary:integer |max_salary:integer|min_salary: ; inlineStatsOverrideEVALed_FieldWithSameName -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM hosts METADATA _index | EVAL x = ip1 @@ -1170,7 +1170,7 @@ beta k8s server |beta |127.0.0.1 |hosts |127.0.0.2|2 ; doubleShadowing -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | INLINESTATS salary = min(salary) BY gender @@ -1189,7 +1189,7 @@ salary:integer |gender:keyword ; doubleShadowing_WithIntertwinedFilters -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | WHERE salary > 30000 @@ -1214,7 +1214,7 @@ salary:integer |gender:keyword ; shadowingAggregateByNextGrouping -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP gender, languages, emp_no, salary @@ -1231,7 +1231,7 @@ emp_no:integer |salary:integer |languages:integer|avg(salary):double|gender:long ; doubleShadowingWithEval -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 from employees | eval salary = salary/100 @@ -1251,7 +1251,7 @@ salary:integer|gender:keyword ; doubleShadowingWithDoubleStats -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 from employees | stats salary=min(salary) by gender @@ -1268,7 +1268,7 @@ M |25324 ; renamingGroupingWithItself -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | EVAL x = gender @@ -1287,7 +1287,7 @@ salary:integer |x:keyword|gender:keyword |min_sl:integer |emp_no:integer ; overridingGroupings -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | INLINESTATS min_sl = MIN(salary) BY x = gender, x = languages @@ -1306,7 +1306,7 @@ salary:integer |x:integer |gender:keyword |min_sl:integer |emp_no:integer ; overridingExpressionGroupings -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | INLINESTATS min_sl = MIN(salary) BY x = TO_LOWER(gender), x = CONCAT(gender, gender) @@ -1325,7 +1325,7 @@ salary:integer |x:keyword |gender:keyword |min_sl:integer |emp_no:integer ; reusingEvalExpressions_UsedInGroupings -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | KEEP salary, gender, emp_no @@ -1344,7 +1344,7 @@ salary:integer |gender:keyword |emp_no:integer |min_sl:integer | x:keyword ; statsBeforeInlinestatsWithTopAndBucket1 -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM books | STATS avg_rating = AVG(ratings) BY decade = BUCKET(year, 10) @@ -1364,7 +1364,7 @@ avg_rating:double | decade:double | decades:double ; statsBeforeInlinestatsWithTopAndBucket2 -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM sample_data | STATS total_duration = SUM(event_duration) BY day = BUCKET(@timestamp, 1 HOUR) @@ -1380,7 +1380,7 @@ total_duration:long | day:date | days:date evalBeforeInlinestatsAndKeepAfter1 -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | WHERE still_hired == false @@ -1400,7 +1400,7 @@ emp_no:integer |still_hired:boolean|totalK:long|count:long ; evalBeforeInlinestatsAndKeepAfter2 -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | EVAL salaryK = salary/1000 @@ -1420,7 +1420,7 @@ emp_no:integer |still_hired:boolean|total:long|count:long ; evalBeforeInlinestatsAndKeepAfter3 -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | EVAL salaryK = salary/1000 @@ -1439,7 +1439,7 @@ emp_no:integer |still_hired:boolean|total:long ; evalBeforeInlinestatsAndKeepAfter4 -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | EVAL salaryK = salary/1000 @@ -1458,7 +1458,7 @@ emp_no:integer |still_hired:boolean|count:long ; evalBeforeInlinestatsAndKeepAfter5 -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 ROW salary = 12300, emp_no = 5, gender = "F" | EVAL salaryK = salary/1000 @@ -1471,7 +1471,7 @@ emp_no:integer ; sortBeforeInlinestats1 -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 ROW salary = 12300, emp_no = 5, gender = "F" | EVAL salaryK = salary/1000 @@ -1485,7 +1485,7 @@ emp_no:integer ; sortBeforeInlinestats2 -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | SORT emp_no @@ -1503,9 +1503,9 @@ emp_no:integer |still_hired:boolean|count:long 10005 |true |1 ; -// fails with: java.lang.AssertionError: expected no concrete indices without data node plan +// TODO: fails with: java.lang.AssertionError: expected no concrete indices without data node plan sortBeforeInlinestats3-Ignore -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 FROM employees | SORT languages DESC @@ -1527,7 +1527,7 @@ emp_no:integer |still_hired:boolean|count:long // same as `afterLookup`, swapped SORT position sortBeforeInlinestatsAndLookupJoin -required_capability: inlinestats_v9 +required_capability: inlinestats_v10 required_capability: join_lookup_v12 FROM airports @@ -1551,8 +1551,8 @@ ZNZ |4 |German ; // same as `shadowingAggregateByNextGrouping`, swapped SORT position -sortBeforeInlinestats -required_capability: inlinestats_v9 +sortBeforeDoubleInlinestats +required_capability: inlinestats_v10 FROM employees | KEEP gender, languages, emp_no, salary diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java index 6a2b112b58deb..8d55ce37e44e5 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java @@ -952,7 +952,7 @@ public enum Cap { * Fixes a series of issues with inlinestats which had an incomplete implementation after lookup and inlinestats * were refactored. */ - INLINESTATS_V9(EsqlPlugin.INLINESTATS_FEATURE_FLAG), + INLINESTATS_V10(EsqlPlugin.INLINESTATS_FEATURE_FLAG), /** * Support partial_results diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java index 75607fc4260be..ba6d4d978e5f7 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java @@ -4181,7 +4181,7 @@ public void testGroupingOverridesInStats() { } public void testGroupingOverridesInInlinestats() { - assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V9.isEnabled()); + assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V10.isEnabled()); verifyUnsupported(""" from test | inlinestats MIN(salary) BY x = languages, x = x + 1 diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java index 4b7810f31c069..3a80fc6671a01 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java @@ -6013,6 +6013,56 @@ public void testInlinestatsWithRow() { ); } + /* + * EsqlProject[[emp_no{f}#11, avg{r}#5, languages{f}#14, gender{f}#13]] + * \_TopN[[Order[emp_no{f}#11,ASC,LAST]],5[INTEGER]] + * \_InlineJoin[LEFT,[languages{f}#14],[languages{f}#14],[languages{r}#14]] + * |_EsRelation[test][_meta_field{f}#17, emp_no{f}#11, first_name{f}#12, ..] + * \_Project[[avg{r}#5, languages{f}#14]] + * \_Eval[[$$SUM$avg$0{r$}#22 / $$COUNT$avg$1{r$}#23 AS avg#5]] + * \_Aggregate[[languages{f}#14],[SUM(salary{f}#16,true[BOOLEAN]) AS $$SUM$avg$0#22, COUNT(salary{f}#16,true[BOOLEAN]) AS + * $$COUNT$avg$1#23, languages{f}#14]] + * \_StubRelation[[_meta_field{f}#17, emp_no{f}#11, first_name{f}#12, gender{f}#13, hire_date{f}#18, job{f}#19, job.raw{f}#20, + * languages{f}#14, last_name{f}#15, long_noidx{f}#21, salary{f}#16]] + */ + public void testInlinestatsAfterSortAndBeforeLimit() { + var query = """ + FROM employees + | SORT emp_no + | INLINESTATS avg = AVG(salary) BY languages + | LIMIT 5 + | KEEP emp_no, avg, languages, gender + """; + if (releaseBuildForInlinestats(query)) { + return; + } + var plan = optimizedPlan(query); + + var esqlProject = as(plan, EsqlProject.class); + + var topN = as(esqlProject.child(), TopN.class); + assertThat(topN.order().size(), is(1)); + var order = as(topN.order().get(0), Order.class); + assertThat(order.direction(), equalTo(Order.OrderDirection.ASC)); + assertThat(order.nullsPosition(), equalTo(Order.NullsPosition.LAST)); + assertThat(Expressions.name(order.child()), equalTo("emp_no")); + assertThat(topN.limit().fold(FoldContext.small()), equalTo(5)); + + var inlineJoin = as(topN.child(), InlineJoin.class); + assertThat(Expressions.names(inlineJoin.config().matchFields()), is(List.of("languages"))); + // Left + var relation = as(inlineJoin.left(), EsRelation.class); + assertThat(relation.concreteIndices(), is(Set.of("test"))); + // Right + var project = as(inlineJoin.right(), Project.class); + assertThat(Expressions.names(project.projections()), is(List.of("avg", "languages"))); + var eval = as(project.child(), Eval.class); + assertThat(Expressions.names(eval.fields()), is(List.of("avg"))); + var agg = as(eval.child(), Aggregate.class); + assertMap(Expressions.names(agg.output()), is(List.of("$$SUM$avg$0", "$$COUNT$avg$1", "languages"))); + var stub = as(agg.child(), StubRelation.class); + } + /* * TopN[[Order[emp_no{f}#8,DESC,FIRST]],1000[INTEGER]] * \_Filter[emp_no{f}#8 > 1000[INTEGER]] @@ -6201,7 +6251,7 @@ public void testInlinestatsAfterSortDoubled() { var fieldAttribute = as(groupings.get(0), FieldAttribute.class); assertThat(fieldAttribute.name(), is("languages")); var aggs = agg.aggregates(); - assertThat(aggs.get(0).toString(), is("MIN(salary) AS min#5")); + assertThat(aggs.get(0).toString(), startsWith("MIN(salary) AS min")); var stub = as(agg.child(), StubRelation.class); // outer right var project = as(inlineJoin.right(), Project.class); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PropagateInlineEvalsTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PropagateInlineEvalsTests.java index 3b183c803ecd1..b9a469edae907 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PropagateInlineEvalsTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PropagateInlineEvalsTests.java @@ -83,7 +83,7 @@ public static void init() { * \_StubRelation[[emp_no{f}#11, languages{f}#14, gender{f}#13, y{r}#10]] */ public void testGroupingAliasingMoved_To_LeftSideOfJoin() { - assumeTrue("Requires INLINESTATS", EsqlCapabilities.Cap.INLINESTATS_V9.isEnabled()); + assumeTrue("Requires INLINESTATS", EsqlCapabilities.Cap.INLINESTATS_V10.isEnabled()); var plan = plan(""" from test | keep emp_no, languages, gender @@ -126,7 +126,7 @@ public void testGroupingAliasingMoved_To_LeftSideOfJoin() { * {r}#21]] */ public void testGroupingAliasingMoved_To_LeftSideOfJoin_WithExpression() { - assumeTrue("Requires INLINESTATS", EsqlCapabilities.Cap.INLINESTATS_V9.isEnabled()); + assumeTrue("Requires INLINESTATS", EsqlCapabilities.Cap.INLINESTATS_V10.isEnabled()); var plan = plan(""" from test | keep emp_no, languages, gender, last_name, first_name diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/FieldNameUtilsTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/FieldNameUtilsTests.java index f46d0e62cc6ca..3303d5589ca95 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/FieldNameUtilsTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/FieldNameUtilsTests.java @@ -35,7 +35,7 @@ public void testBasicFromCommand() { } public void testBasicFromCommandWithInlinestats() { - assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V9.isEnabled()); + assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V10.isEnabled()); assertFieldNames("from test | inlinestats max(salary) by gender", ALL_FIELDS); } @@ -44,7 +44,7 @@ public void testBasicFromCommandWithMetadata() { } public void testBasicFromCommandWithMetadata_AndInlinestats() { - assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V9.isEnabled()); + assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V10.isEnabled()); assertFieldNames("from test metadata _index, _id, _version | inlinestats max(salary)", ALL_FIELDS); } @@ -310,7 +310,7 @@ public void testLimitZero() { } public void testLimitZero_WithInlinestats() { - assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V9.isEnabled()); + assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V10.isEnabled()); assertFieldNames(""" FROM employees | INLINESTATS COUNT(*), MAX(salary) BY gender @@ -325,7 +325,7 @@ public void testDocsDropHeight() { } public void testDocsDropHeight_WithInlinestats() { - assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V9.isEnabled()); + assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V10.isEnabled()); assertFieldNames(""" FROM employees | DROP height @@ -341,7 +341,7 @@ public void testDocsDropHeightWithWildcard() { } public void testDocsDropHeightWithWildcard_AndInlinestats() { - assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V9.isEnabled()); + assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V10.isEnabled()); assertFieldNames(""" FROM employees | INLINESTATS MAX(salary) BY gender @@ -508,7 +508,7 @@ public void testSortWithLimitOne_DropHeight() { } public void testSortWithLimitOne_DropHeight_WithInlinestats() { - assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V9.isEnabled()); + assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V10.isEnabled()); assertFieldNames("from employees | inlinestats avg(salary) by languages | sort languages | limit 1 | drop height*", ALL_FIELDS); } @@ -808,7 +808,7 @@ public void testFilterById() { } public void testFilterById_WithInlinestats() { - assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V9.isEnabled()); + assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V10.isEnabled()); assertFieldNames("FROM apps metadata _id | INLINESTATS max(rate) | WHERE _id == \"4\"", ALL_FIELDS); } @@ -1279,7 +1279,7 @@ public void testProjectDropPattern() { } public void testProjectDropPattern_WithInlinestats() { - assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V9.isEnabled()); + assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V10.isEnabled()); assertFieldNames(""" from test | inlinestats max(foo) by bar @@ -1362,7 +1362,7 @@ public void testCountAllAndOtherStatGrouped() { } public void testCountAllAndOtherStatGrouped_WithInlinestats() { - assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V9.isEnabled()); + assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V10.isEnabled()); assertFieldNames(""" from test | inlinestats c = count(*), min = min(emp_no) by languages @@ -1401,7 +1401,7 @@ public void testCountAllWithEval() { } public void testCountAllWithEval_AndInlinestats() { - assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V9.isEnabled()); + assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V10.isEnabled()); assertFieldNames(""" from test | rename languages as l @@ -1414,7 +1414,7 @@ public void testCountAllWithEval_AndInlinestats() { } public void testKeepAfterEval_AndInlinestats() { - assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V9.isEnabled()); + assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V10.isEnabled()); assertFieldNames(""" from test | rename languages as l @@ -1427,7 +1427,7 @@ public void testKeepAfterEval_AndInlinestats() { } public void testKeepBeforeEval_AndInlinestats() { - assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V9.isEnabled()); + assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V10.isEnabled()); assertFieldNames(""" from test | rename languages as l @@ -1440,7 +1440,7 @@ public void testKeepBeforeEval_AndInlinestats() { } public void testStatsBeforeEval_AndInlinestats() { - assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V9.isEnabled()); + assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V10.isEnabled()); assertFieldNames(""" from test | rename languages as l @@ -1452,7 +1452,7 @@ public void testStatsBeforeEval_AndInlinestats() { } public void testStatsBeforeInlinestats() { - assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V9.isEnabled()); + assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V10.isEnabled()); assertFieldNames(""" from test | stats min = min(salary) by languages @@ -1461,7 +1461,7 @@ public void testStatsBeforeInlinestats() { } public void testKeepBeforeInlinestats() { - assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V9.isEnabled()); + assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V10.isEnabled()); assertFieldNames(""" from test | keep languages, salary From e9f177078af4e066facbd263f1b60a95f832ce3b Mon Sep 17 00:00:00 2001 From: Bogdan Pintea Date: Mon, 11 Aug 2025 16:26:59 +0200 Subject: [PATCH 3/8] Update docs/changelog/132417.yaml --- docs/changelog/132417.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/132417.yaml diff --git a/docs/changelog/132417.yaml b/docs/changelog/132417.yaml new file mode 100644 index 0000000000000..68bff77c53333 --- /dev/null +++ b/docs/changelog/132417.yaml @@ -0,0 +1,5 @@ +pr: 132417 +summary: Add optimisation rule to allow unbounded sort before INLINESTATS +area: ES|QL +type: enhancement +issues: [] From 39fc575a735b01b6b1166730f8ead81e8a0db527 Mon Sep 17 00:00:00 2001 From: Bogdan Pintea Date: Mon, 18 Aug 2025 11:17:08 +0200 Subject: [PATCH 4/8] review --- .../logical/PullUpOrderByBeforeInlineJoin.java | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PullUpOrderByBeforeInlineJoin.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PullUpOrderByBeforeInlineJoin.java index a961ce1a0d903..6702a9da6dfef 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PullUpOrderByBeforeInlineJoin.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PullUpOrderByBeforeInlineJoin.java @@ -30,7 +30,7 @@ private static LogicalPlan pullUpOrderByBeforeInlineJoin(LogicalPlan plan) { if (plan instanceof InlineJoin inlineJoin) { OrderBy orderBy = findOrderByNotPrecededByLimit(inlineJoin); if (orderBy != null) { - LogicalPlan newInlineJoin = removeOrderBy(inlineJoin, orderBy); + LogicalPlan newInlineJoin = inlineJoin.transformUp(OrderBy.class, ob -> ob == orderBy ? orderBy.child() : ob); return new OrderBy(orderBy.source(), newInlineJoin, orderBy.order()); } } @@ -55,15 +55,4 @@ private static OrderBy findOrderByNotPrecededByLimit(LogicalPlan plan) { } return null; } - - // Removes the found OrderBy node from its current position in the subtree - private static LogicalPlan removeOrderBy(LogicalPlan plan, OrderBy orderBy) { - if (plan == orderBy) { - return orderBy.child(); - } - if (plan.children().isEmpty()) { - return plan; - } - return plan.replaceChildren(plan.children().stream().map(child -> removeOrderBy(child, orderBy)).toList()); - } } From 64f0b7f59579c78d4cf1f8269e3017877a8913a1 Mon Sep 17 00:00:00 2001 From: Bogdan Pintea Date: Mon, 18 Aug 2025 11:45:31 +0200 Subject: [PATCH 5/8] auto-merge artefact --- .../xpack/esql/session/FieldNameUtilsTests.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/FieldNameUtilsTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/FieldNameUtilsTests.java index f4e6276d23239..128e940c9d540 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/FieldNameUtilsTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/FieldNameUtilsTests.java @@ -2846,7 +2846,7 @@ public void testForkAfterMvExpand() { } public void testForkBeforeInlineStatsIgnore() { - assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V9.isEnabled()); + assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V10.isEnabled()); assertTrue("FORK required", EsqlCapabilities.Cap.FORK_V9.isEnabled()); assertFieldNames(""" FROM employees @@ -2859,7 +2859,7 @@ public void testForkBeforeInlineStatsIgnore() { } public void testForkBranchWithInlineStatsIgnore() { - assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V9.isEnabled()); + assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V10.isEnabled()); assertTrue("FORK required", EsqlCapabilities.Cap.FORK_V9.isEnabled()); assertFieldNames(""" FROM employees @@ -2873,7 +2873,7 @@ public void testForkBranchWithInlineStatsIgnore() { } public void testForkAfterInlineStatsIgnore() { - assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V9.isEnabled()); + assumeTrue("INLINESTATS required", EsqlCapabilities.Cap.INLINESTATS_V10.isEnabled()); assertTrue("FORK required", EsqlCapabilities.Cap.FORK_V9.isEnabled()); assertFieldNames(""" FROM employees From a0fb3d340b86cce5cbe3fb5dc97d343ca65bdd19 Mon Sep 17 00:00:00 2001 From: Bogdan Pintea Date: Mon, 18 Aug 2025 12:05:43 +0200 Subject: [PATCH 6/8] Refactor to use forEachDownMayReturnEarly() --- .../PullUpOrderByBeforeInlineJoin.java | 31 +++++++------------ 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PullUpOrderByBeforeInlineJoin.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PullUpOrderByBeforeInlineJoin.java index 6702a9da6dfef..461031bf555f2 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PullUpOrderByBeforeInlineJoin.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PullUpOrderByBeforeInlineJoin.java @@ -6,6 +6,7 @@ */ package org.elasticsearch.xpack.esql.optimizer.rules.logical; +import org.elasticsearch.xpack.esql.core.util.Holder; import org.elasticsearch.xpack.esql.plan.logical.Limit; import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan; import org.elasticsearch.xpack.esql.plan.logical.OrderBy; @@ -28,7 +29,16 @@ protected LogicalPlan rule(LogicalPlan plan) { private static LogicalPlan pullUpOrderByBeforeInlineJoin(LogicalPlan plan) { if (plan instanceof InlineJoin inlineJoin) { - OrderBy orderBy = findOrderByNotPrecededByLimit(inlineJoin); + Holder orderByHolder = new Holder<>(); + inlineJoin.forEachDownMayReturnEarly((node, breakEarly) -> { + if (node instanceof OrderBy orderBy) { + orderByHolder.set(orderBy); + breakEarly.set(true); + } else { + breakEarly.set(node instanceof SortAgnostic == false); + } + }); + OrderBy orderBy = orderByHolder.get(); if (orderBy != null) { LogicalPlan newInlineJoin = inlineJoin.transformUp(OrderBy.class, ob -> ob == orderBy ? orderBy.child() : ob); return new OrderBy(orderBy.source(), newInlineJoin, orderBy.order()); @@ -36,23 +46,4 @@ private static LogicalPlan pullUpOrderByBeforeInlineJoin(LogicalPlan plan) { } return plan; } - - // Finds an OrderBy node in the subtree of the provided plan that is not preceded by a Limit - private static OrderBy findOrderByNotPrecededByLimit(LogicalPlan plan) { - if (plan instanceof Limit) { - return null; - } - if (plan instanceof OrderBy orderBy) { - return orderBy; - } - for (LogicalPlan child : plan.children()) { - if (child instanceof SortAgnostic) { - OrderBy found = findOrderByNotPrecededByLimit(child); - if (found != null) { - return found; - } - } - } - return null; - } } From ddeeebaa73133cfa1a7c07f779c09686578d6b03 Mon Sep 17 00:00:00 2001 From: Bogdan Pintea Date: Mon, 18 Aug 2025 12:08:57 +0200 Subject: [PATCH 7/8] finally, update javadoc --- .../rules/logical/PullUpOrderByBeforeInlineJoin.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PullUpOrderByBeforeInlineJoin.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PullUpOrderByBeforeInlineJoin.java index 461031bf555f2..a8a5717765661 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PullUpOrderByBeforeInlineJoin.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PullUpOrderByBeforeInlineJoin.java @@ -14,11 +14,11 @@ import org.elasticsearch.xpack.esql.plan.logical.join.InlineJoin; /** - * Pulls "up" an {@link OrderBy} node that is not preceded by a {@link Limit}, but is preceded by an {@link InlineJoin}. - * The InlineJoin is {@link SortAgnostic}, so the OrderBy can be pulled up without affecting the semantics of the join. + * Pulls "up" an {@link OrderBy} node that is not preceded by a not {@link SortAgnostic} node (such as {@link Limit}), but is preceded by an + * {@link InlineJoin}. The InlineJoin is {@link SortAgnostic}, so the OrderBy can be pulled up without affecting the semantics of the join. * This is needed since otherwise the OrderBy would remain to be executed unbounded, which isn't supported. - * If it's preceded by a {@link Limit}, it will be merged into a {@link org.elasticsearch.xpack.esql.plan.logical.TopN} later in the - * "cleanup" optimization stage. + * Specifically, if it's preceded by a {@link Limit}, it will be merged into a {@link org.elasticsearch.xpack.esql.plan.logical.TopN} later + * in the "cleanup" optimization stage. */ public final class PullUpOrderByBeforeInlineJoin extends OptimizerRules.OptimizerRule { From 24e055c1edd2e556e91365c705a42755f3945ddc Mon Sep 17 00:00:00 2001 From: Bogdan Pintea Date: Mon, 18 Aug 2025 13:34:22 +0200 Subject: [PATCH 8/8] compact code --- .../optimizer/rules/logical/PullUpOrderByBeforeInlineJoin.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PullUpOrderByBeforeInlineJoin.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PullUpOrderByBeforeInlineJoin.java index a8a5717765661..abf9b5caa4a5e 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PullUpOrderByBeforeInlineJoin.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PullUpOrderByBeforeInlineJoin.java @@ -40,8 +40,7 @@ private static LogicalPlan pullUpOrderByBeforeInlineJoin(LogicalPlan plan) { }); OrderBy orderBy = orderByHolder.get(); if (orderBy != null) { - LogicalPlan newInlineJoin = inlineJoin.transformUp(OrderBy.class, ob -> ob == orderBy ? orderBy.child() : ob); - return new OrderBy(orderBy.source(), newInlineJoin, orderBy.order()); + return orderBy.replaceChild(inlineJoin.transformUp(OrderBy.class, ob -> ob == orderBy ? orderBy.child() : ob)); } } return plan;