diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java index f3b2ea0d864ff..217bf6692aa27 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java @@ -66,6 +66,7 @@ import org.elasticsearch.xpack.esql.parser.QueryParam; import org.elasticsearch.xpack.esql.plan.logical.Enrich; import org.elasticsearch.xpack.esql.plan.logical.EsRelation; +import org.elasticsearch.xpack.esql.plan.logical.Limit; import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan; import org.elasticsearch.xpack.esql.plan.logical.local.LocalRelation; import org.elasticsearch.xpack.esql.plan.logical.local.LocalSupplier; @@ -111,6 +112,7 @@ import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; import static java.util.Collections.unmodifiableMap; +import static org.elasticsearch.test.ESTestCase.assertEquals; import static org.elasticsearch.test.ESTestCase.between; import static org.elasticsearch.test.ESTestCase.randomAlphaOfLength; import static org.elasticsearch.test.ESTestCase.randomBoolean; @@ -403,6 +405,21 @@ public static T as(Object node, Class type) { return type.cast(node); } + public static Limit asLimit(Object node, Integer limitLiteral) { + return asLimit(node, limitLiteral, null); + } + + public static Limit asLimit(Object node, Integer limitLiteral, Boolean duplicated) { + Limit limit = as(node, Limit.class); + if (limitLiteral != null) { + assertEquals(as(limit.limit(), Literal.class).value(), limitLiteral); + } + if (duplicated != null) { + assertEquals(limit.duplicated(), duplicated); + } + return limit; + } + public static Map loadMapping(String name) { return LoadMapping.loadMapping(name); } diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec index dbeaedd7e0416..d4a98fdc70a9a 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec @@ -273,6 +273,58 @@ emp_no:integer 10001 ; + +lookupIndexInFromRepeatedRowBug +// Test for https://github.com/elastic/elasticsearch/issues/118852 +required_capability: join_lookup_v12 +FROM languages_lookup_non_unique_key +| WHERE language_code == 1 +| LOOKUP JOIN languages_lookup ON language_code +| KEEP language_code, language_name, country +| SORT language_code, language_name, country +; + +language_code:integer | language_name:keyword | country:text +1 | English | Canada +1 | English | United Kingdom +1 | English | United States of America +1 | English | null +; + +nonUniqueRightKeyOnTheCoordinatorLateLimit +required_capability: join_lookup_v12 +required_capability: join_lookup_fix_limit_pushdown + +FROM employees +| SORT emp_no +| EVAL language_code = emp_no % 10 +| LOOKUP JOIN languages_lookup_non_unique_key ON language_code +| KEEP emp_no, language_code, language_name, country +| LIMIT 4 +| SORT country +; + +emp_no:integer | language_code:integer | language_name:keyword | country:text +10001 | 1 | English | Canada +10001 | 1 | null | United Kingdom +10001 | 1 | English | United States of America +10001 | 1 | English | null +; + +nonUniqueRightKeyLateLimitWithEmptyRelation +required_capability: join_lookup_v12 +required_capability: join_lookup_fix_limit_pushdown + +ROW language_code = 1 +| WHERE language_code != 1 +| LOOKUP JOIN languages_lookup_non_unique_key ON language_code +| LIMIT 1 +| KEEP language_code, language_name +; + +language_code:integer | language_name:keyword +; + ########################################################################### # null and multi-value behavior with languages_lookup_non_unique_key index ########################################################################### @@ -1278,23 +1330,6 @@ ignoreOrder:true 2023-10-23T12:15:03.360Z | 172.21.2.162 | 3450233 | QA | null ; -lookupIndexInFromRepeatedRowBug -// Test for https://github.com/elastic/elasticsearch/issues/118852 -required_capability: join_lookup_v12 -FROM languages_lookup_non_unique_key -| WHERE language_code == 1 -| LOOKUP JOIN languages_lookup ON language_code -| KEEP language_code, language_name, country -| SORT language_code, language_name, country -; - -language_code:integer | language_name:keyword | country:text -1 | English | Canada -1 | English | United Kingdom -1 | English | United States of America -1 | English | null -; - lookupIndexQuoting required_capability: join_lookup_v12 FROM languages_lookup_non_unique_key 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 548fb30a51355..b8b911afe7fd4 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 @@ -711,6 +711,11 @@ public enum Cap { */ JOIN_LOOKUP_SKIP_MV_ON_LOOKUP_KEY(JOIN_LOOKUP_V12.isEnabled()), + /** + * Fix pushing down LIMIT past LOOKUP JOIN in case of multiple matching join keys. + */ + JOIN_LOOKUP_FIX_LIMIT_PUSHDOWN(JOIN_LOOKUP_V12.isEnabled()), + /** * Fix for https://github.com/elastic/elasticsearch/issues/117054 */ diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java index 4f5ff35b84054..fd98b2717eae0 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java @@ -549,8 +549,7 @@ private LogicalPlan resolveMvExpand(MvExpand p, List childrenOutput) resolved, resolved.resolved() ? new ReferenceAttribute(resolved.source(), resolved.name(), resolved.dataType(), resolved.nullable(), null, false) - : resolved, - p.limit() + : resolved ); } return p; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/AddDefaultTopN.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/AddDefaultTopN.java index 02815d45d2896..ef091686a4b38 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/AddDefaultTopN.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/AddDefaultTopN.java @@ -34,7 +34,7 @@ * | sort first_name * | limit 15 *

- * PushDownAndCombineLimits rule will copy the "limit 15" after "sort emp_no" if there is no filter on the expanded values + * {@link PushDownAndCombineLimits} will copy the "limit 15" after "sort emp_no" if there is no filter on the expanded values * OR if there is no sort between "limit" and "mv_expand". * But, since this type of query has such a filter, the "sort emp_no" will have no limit when it reaches the current rule. */ diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineLimits.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineLimits.java index 969a6bb713eca..dca4dfbd533df 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineLimits.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineLimits.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.esql.optimizer.rules.logical; -import org.elasticsearch.xpack.esql.core.expression.Literal; import org.elasticsearch.xpack.esql.optimizer.LogicalOptimizerContext; import org.elasticsearch.xpack.esql.plan.logical.Aggregate; import org.elasticsearch.xpack.esql.plan.logical.Enrich; @@ -21,6 +20,9 @@ import org.elasticsearch.xpack.esql.plan.logical.join.Join; import org.elasticsearch.xpack.esql.plan.logical.join.JoinTypes; +import java.util.ArrayList; +import java.util.List; + public final class PushDownAndCombineLimits extends OptimizerRules.ParameterizedOptimizerRule { public PushDownAndCombineLimits() { @@ -31,27 +33,18 @@ public PushDownAndCombineLimits() { public LogicalPlan rule(Limit limit, LogicalOptimizerContext ctx) { if (limit.child() instanceof Limit childLimit) { var limitSource = limit.limit(); - var l1 = (int) limitSource.fold(ctx.foldCtx()); - var l2 = (int) childLimit.limit().fold(ctx.foldCtx()); - return new Limit(limit.source(), Literal.of(limitSource, Math.min(l1, l2)), childLimit.child()); + var parentLimitValue = (int) limitSource.fold(ctx.foldCtx()); + var childLimitValue = (int) childLimit.limit().fold(ctx.foldCtx()); + // We want to preserve the duplicated() value of the smaller limit, so we'll use replaceChild. + return parentLimitValue < childLimitValue ? limit.replaceChild(childLimit.child()) : childLimit; } else if (limit.child() instanceof UnaryPlan unary) { if (unary instanceof Eval || unary instanceof Project || unary instanceof RegexExtract || unary instanceof Enrich) { return unary.replaceChild(limit.replaceChild(unary.child())); - } else if (unary instanceof MvExpand mvx) { + } else if (unary instanceof MvExpand) { // MV_EXPAND can increase the number of rows, so we cannot just push the limit down // (we also have to preserve the LIMIT afterwards) - // - // To avoid infinite loops, ie. - // | MV_EXPAND | LIMIT -> | LIMIT | MV_EXPAND | LIMIT -> ... | MV_EXPAND | LIMIT - // we add an inner limit to MvExpand and just push down the existing limit, ie. - // | MV_EXPAND | LIMIT N -> | LIMIT N | MV_EXPAND (with limit N) - var limitSource = limit.limit(); - var limitVal = (int) limitSource.fold(ctx.foldCtx()); - Integer mvxLimit = mvx.limit(); - if (mvxLimit == null || mvxLimit > limitVal) { - mvx = new MvExpand(mvx.source(), mvx.child(), mvx.target(), mvx.expanded(), limitVal); - } - return mvx.replaceChild(limit.replaceChild(mvx.child())); + // To avoid repeating this infinitely, we have to set duplicated = true. + return duplicateLimitAsFirstGrandchild(limit); } // check if there's a 'visible' descendant limit lower than the current one // and if so, align the current limit since it adds no value @@ -62,17 +55,15 @@ public LogicalPlan rule(Limit limit, LogicalOptimizerContext ctx) { var l1 = (int) limit.limit().fold(ctx.foldCtx()); var l2 = (int) descendantLimit.limit().fold(ctx.foldCtx()); if (l2 <= l1) { - return new Limit(limit.source(), Literal.of(limit.limit(), l2), limit.child()); + return limit.withLimit(descendantLimit.limit()); } } } - } else if (limit.child() instanceof Join join) { - if (join.config().type() == JoinTypes.LEFT) { - // NOTE! This is only correct because our LEFT JOINs preserve the number of rows from the left hand side. - // This deviates from SQL semantics. In SQL, multiple matches on the right hand side lead to multiple rows in the output. - // For us, multiple matches on the right hand side are collected into multi-values. - return join.replaceChildren(limit.replaceChild(join.left()), join.right()); - } + } else if (limit.child() instanceof Join join && join.config().type() == JoinTypes.LEFT) { + // Left joins increase the number of rows if any join key has multiple matches from the right hand side. + // Therefore, we cannot simply push down the limit - but we can add another limit before the join. + // To avoid repeating this infinitely, we have to set duplicated = true. + return duplicateLimitAsFirstGrandchild(limit); } return limit; } @@ -100,4 +91,27 @@ private static Limit descendantLimit(UnaryPlan unary) { } return null; } + + /** + * Duplicate the limit past its child if it wasn't duplicated yet. The duplicate is placed on top of its leftmost grandchild. + * Idempotent. (Sets {@link Limit#duplicated()} to {@code true} on the limit that remains at the top.) + */ + private static Limit duplicateLimitAsFirstGrandchild(Limit limit) { + if (limit.duplicated()) { + return limit; + } + + List grandChildren = limit.child().children(); + LogicalPlan firstGrandChild = grandChildren.getFirst(); + LogicalPlan newFirstGrandChild = limit.replaceChild(firstGrandChild); + + List newGrandChildren = new ArrayList<>(); + newGrandChildren.add(newFirstGrandChild); + for (int i = 1; i < grandChildren.size(); i++) { + newGrandChildren.add(grandChildren.get(i)); + } + + LogicalPlan newChild = limit.child().replaceChildren(newGrandChildren); + return limit.replaceChild(newChild).withDuplicated(true); + } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Limit.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Limit.java index ea64b7687f4c0..09879e47859c9 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Limit.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Limit.java @@ -21,21 +21,52 @@ public class Limit extends UnaryPlan { public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(LogicalPlan.class, "Limit", Limit::new); private final Expression limit; - + /** + * Important for optimizations. This should be {@code false} in most cases, which allows this instance to be duplicated past a child + * plan node that increases the number of rows, like for LOOKUP JOIN and MV_EXPAND. + * Needs to be set to {@code true} in {@link org.elasticsearch.xpack.esql.optimizer.rules.logical.PushDownAndCombineLimits} to avoid + * infinite loops from adding a duplicate of the limit past the child over and over again. + */ + private final transient boolean duplicated; + + /** + * Default way to create a new instance. Do not use this to copy an existing instance, as this sets {@link Limit#duplicated} to + * {@code false}. + */ public Limit(Source source, Expression limit, LogicalPlan child) { + this(source, limit, child, false); + } + + public Limit(Source source, Expression limit, LogicalPlan child, boolean duplicated) { super(source, child); this.limit = limit; + this.duplicated = duplicated; } + /** + * Omits reading {@link Limit#duplicated}, c.f. {@link Limit#writeTo}. + */ private Limit(StreamInput in) throws IOException { - this(Source.readFrom((PlanStreamInput) in), in.readNamedWriteable(Expression.class), in.readNamedWriteable(LogicalPlan.class)); + this( + Source.readFrom((PlanStreamInput) in), + in.readNamedWriteable(Expression.class), + in.readNamedWriteable(LogicalPlan.class), + false + ); } + /** + * Omits serializing {@link Limit#duplicated} because when sent to a data node, this should always be {@code false}. + * That's because if it's true, this means a copy of this limit was pushed down below an MvExpand or Join, and thus there's + * another pipeline breaker further upstream - we're already on the coordinator node. + */ @Override public void writeTo(StreamOutput out) throws IOException { Source.EMPTY.writeTo(out); out.writeNamedWriteable(limit()); out.writeNamedWriteable(child()); + // Let's make sure we notice during tests if we ever serialize a duplicated Limit. + assert duplicated == false; } @Override @@ -45,18 +76,30 @@ public String getWriteableName() { @Override protected NodeInfo info() { - return NodeInfo.create(this, Limit::new, limit, child()); + return NodeInfo.create(this, Limit::new, limit, child(), duplicated); } @Override public Limit replaceChild(LogicalPlan newChild) { - return new Limit(source(), limit, newChild); + return new Limit(source(), limit, newChild, duplicated); } public Expression limit() { return limit; } + public Limit withLimit(Expression limit) { + return new Limit(source(), limit, child(), duplicated); + } + + public boolean duplicated() { + return duplicated; + } + + public Limit withDuplicated(boolean duplicated) { + return new Limit(source(), limit, child(), duplicated); + } + @Override public String commandName() { return "LIMIT"; @@ -69,7 +112,7 @@ public boolean expressionsResolved() { @Override public int hashCode() { - return Objects.hash(limit, child()); + return Objects.hash(limit, child(), duplicated); } @Override @@ -83,6 +126,6 @@ public boolean equals(Object obj) { Limit other = (Limit) obj; - return Objects.equals(limit, other.limit) && Objects.equals(child(), other.child()); + return Objects.equals(limit, other.limit) && Objects.equals(child(), other.child()) && (duplicated == other.duplicated); } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/MvExpand.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/MvExpand.java index 949e4906e5033..9b0168ddd739d 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/MvExpand.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/MvExpand.java @@ -27,19 +27,13 @@ public class MvExpand extends UnaryPlan { private final NamedExpression target; private final Attribute expanded; - private final Integer limit; private List output; public MvExpand(Source source, LogicalPlan child, NamedExpression target, Attribute expanded) { - this(source, child, target, expanded, null); - } - - public MvExpand(Source source, LogicalPlan child, NamedExpression target, Attribute expanded, Integer limit) { super(source, child); this.target = target; this.expanded = expanded; - this.limit = limit; } private MvExpand(StreamInput in) throws IOException { @@ -47,8 +41,7 @@ private MvExpand(StreamInput in) throws IOException { Source.readFrom((PlanStreamInput) in), in.readNamedWriteable(LogicalPlan.class), in.readNamedWriteable(NamedExpression.class), - in.readNamedWriteable(Attribute.class), - null // we only need this on the coordinator + in.readNamedWriteable(Attribute.class) ); } @@ -58,7 +51,6 @@ public void writeTo(StreamOutput out) throws IOException { out.writeNamedWriteable(child()); out.writeNamedWriteable(target()); out.writeNamedWriteable(expanded()); - assert limit == null; } @Override @@ -86,10 +78,6 @@ public Attribute expanded() { return expanded; } - public Integer limit() { - return limit; - } - @Override protected AttributeSet computeReferences() { return target.references(); @@ -105,8 +93,8 @@ public boolean expressionsResolved() { } @Override - public UnaryPlan replaceChild(LogicalPlan newChild) { - return new MvExpand(source(), newChild, target, expanded, limit); + public MvExpand replaceChild(LogicalPlan newChild) { + return new MvExpand(source(), newChild, target, expanded); } @Override @@ -119,12 +107,12 @@ public List output() { @Override protected NodeInfo info() { - return NodeInfo.create(this, MvExpand::new, child(), target, expanded, limit); + return NodeInfo.create(this, MvExpand::new, child(), target, expanded); } @Override public int hashCode() { - return Objects.hash(super.hashCode(), target, expanded, limit); + return Objects.hash(super.hashCode(), target, expanded); } @Override @@ -133,6 +121,6 @@ public boolean equals(Object obj) { return false; } MvExpand other = ((MvExpand) obj); - return Objects.equals(target, other.target) && Objects.equals(expanded, other.expanded) && Objects.equals(limit, other.limit); + return Objects.equals(target, other.target) && Objects.equals(expanded, other.expanded); } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/mapper/MapperUtils.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/mapper/MapperUtils.java index b8f539ea307c9..f358a77a08aec 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/mapper/MapperUtils.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/mapper/MapperUtils.java @@ -12,9 +12,6 @@ import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException; import org.elasticsearch.xpack.esql.core.expression.Attribute; import org.elasticsearch.xpack.esql.core.expression.FoldContext; -import org.elasticsearch.xpack.esql.core.expression.Literal; -import org.elasticsearch.xpack.esql.core.tree.Source; -import org.elasticsearch.xpack.esql.core.type.DataType; import org.elasticsearch.xpack.esql.plan.logical.Aggregate; import org.elasticsearch.xpack.esql.plan.logical.Dissect; import org.elasticsearch.xpack.esql.plan.logical.Enrich; @@ -34,7 +31,6 @@ import org.elasticsearch.xpack.esql.plan.physical.EvalExec; import org.elasticsearch.xpack.esql.plan.physical.FilterExec; import org.elasticsearch.xpack.esql.plan.physical.GrokExec; -import org.elasticsearch.xpack.esql.plan.physical.LimitExec; import org.elasticsearch.xpack.esql.plan.physical.LocalSourceExec; import org.elasticsearch.xpack.esql.plan.physical.MvExpandExec; import org.elasticsearch.xpack.esql.plan.physical.PhysicalPlan; @@ -99,13 +95,7 @@ static PhysicalPlan mapUnary(UnaryPlan p, PhysicalPlan child) { } if (p instanceof MvExpand mvExpand) { - MvExpandExec result = new MvExpandExec(mvExpand.source(), child, mvExpand.target(), mvExpand.expanded()); - if (mvExpand.limit() != null) { - // MvExpand could have an inner limit - // see PushDownAndCombineLimits rule - return new LimitExec(result.source(), result, new Literal(Source.EMPTY, mvExpand.limit(), DataType.INTEGER)); - } - return result; + return new MvExpandExec(mvExpand.source(), child, mvExpand.target(), mvExpand.expanded()); } return unsupported(p); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java index 310d680cfbf41..c9821aea343bf 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java @@ -66,6 +66,7 @@ import static org.elasticsearch.xpack.esql.EsqlTestUtils.THREE; import static org.elasticsearch.xpack.esql.EsqlTestUtils.TWO; import static org.elasticsearch.xpack.esql.EsqlTestUtils.as; +import static org.elasticsearch.xpack.esql.EsqlTestUtils.asLimit; import static org.elasticsearch.xpack.esql.EsqlTestUtils.getFieldAttribute; import static org.elasticsearch.xpack.esql.EsqlTestUtils.greaterThanOf; import static org.elasticsearch.xpack.esql.EsqlTestUtils.loadMapping; @@ -75,7 +76,6 @@ import static org.elasticsearch.xpack.esql.EsqlTestUtils.withDefaultLimitWarning; import static org.elasticsearch.xpack.esql.core.tree.Source.EMPTY; import static org.hamcrest.Matchers.contains; -import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; @@ -196,10 +196,11 @@ public void testMissingFieldInSort() { /** * Expects - * EsqlProject[[first_name{f}#9, last_name{r}#18]] - * \_MvExpand[last_name{f}#12,last_name{r}#18,1000] - * \_Limit[1000[INTEGER]] - * \_EsRelation[test][_meta_field{f}#14, emp_no{f}#8, first_name{f}#9, ge..] + * EsqlProject[[first_name{f}#7, last_name{r}#17]] + * \_Limit[1000[INTEGER],true] + * \_MvExpand[last_name{f}#10,last_name{r}#17] + * \_Limit[1000[INTEGER],false] + * \_EsRelation[test][_meta_field{f}#12, emp_no{f}#6, first_name{f}#7, ge..] */ public void testMissingFieldInMvExpand() { var plan = plan(""" @@ -215,9 +216,9 @@ public void testMissingFieldInMvExpand() { var projections = project.projections(); assertThat(Expressions.names(projections), contains("first_name", "last_name")); - var mvExpand = as(project.child(), MvExpand.class); - assertThat(mvExpand.limit(), equalTo(1000)); - var limit2 = as(mvExpand.child(), Limit.class); + var limit1 = asLimit(project.child(), 1000, true); + var mvExpand = as(limit1.child(), MvExpand.class); + var limit2 = asLimit(mvExpand.child(), 1000, false); as(limit2.child(), EsRelation.class); } @@ -269,7 +270,6 @@ protected NodeInfo info() { } } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/110150") public void testMissingFieldInNewCommand() { var testStats = statsForMissingField("last_name"); localPlan( 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 7ceaaa740b802..c80e374540d09 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 @@ -149,6 +149,7 @@ import static org.elasticsearch.xpack.esql.EsqlTestUtils.THREE; import static org.elasticsearch.xpack.esql.EsqlTestUtils.TWO; import static org.elasticsearch.xpack.esql.EsqlTestUtils.as; +import static org.elasticsearch.xpack.esql.EsqlTestUtils.asLimit; import static org.elasticsearch.xpack.esql.EsqlTestUtils.emptySource; import static org.elasticsearch.xpack.esql.EsqlTestUtils.fieldAttribute; import static org.elasticsearch.xpack.esql.EsqlTestUtils.getFieldAttribute; @@ -1315,6 +1316,8 @@ public void testCombineLimits() { } public void testPushdownLimitsPastLeftJoin() { + var rule = new PushDownAndCombineLimits(); + var leftChild = emptySource(); var rightChild = new LocalRelation(Source.EMPTY, List.of(fieldAttribute()), LocalSupplier.EMPTY); assertNotEquals(leftChild, rightChild); @@ -1329,9 +1332,16 @@ public void testPushdownLimitsPastLeftJoin() { var limit = new Limit(EMPTY, L(10), join); - var optimizedPlan = new PushDownAndCombineLimits().rule(limit, logicalOptimizerCtx); + var optimizedPlan = rule.apply(limit, logicalOptimizerCtx); + + assertEquals( + new Limit(limit.source(), limit.limit(), join.replaceChildren(limit.replaceChild(join.left()), join.right()), true), + optimizedPlan + ); - assertEquals(join.replaceChildren(limit.replaceChild(join.left()), join.right()), optimizedPlan); + var optimizedTwice = rule.apply(optimizedPlan, logicalOptimizerCtx); + // We mustn't create the limit after the JOIN multiple times when the rule is applied multiple times, that'd lead to infinite loops. + assertEquals(optimizedPlan, optimizedTwice); } public void testMultipleCombineLimits() { @@ -1851,10 +1861,11 @@ public void testDontCombineOrderByThroughMvExpand() { /** * Expected - * MvExpand[x{r}#4,x{r}#18,1000] - * \_EsqlProject[[first_name{f}#9 AS x]] - * \_Limit[1000[INTEGER]] - * \_EsRelation[test][_meta_field{f}#14, emp_no{f}#8, first_name{f}#9, ge..] + * Limit[1000[INTEGER],true] + * \_MvExpand[x{r}#4,x{r}#19] + * \_EsqlProject[[first_name{f}#9 AS x]] + * \_Limit[1000[INTEGER],false] + * \_EsRelation[test][_meta_field{f}#14, emp_no{f}#8, first_name{f}#9, ge..] */ public void testCopyDefaultLimitPastMvExpand() { LogicalPlan plan = optimizedPlan(""" @@ -1864,20 +1875,44 @@ public void testCopyDefaultLimitPastMvExpand() { | mv_expand x """); - var mvExpand = as(plan, MvExpand.class); - assertThat(mvExpand.limit(), equalTo(1000)); + var limit = asLimit(plan, 1000, true); + var mvExpand = as(limit.child(), MvExpand.class); var keep = as(mvExpand.child(), EsqlProject.class); - var limitPastMvExpand = as(keep.child(), Limit.class); - assertThat(limitPastMvExpand.limit().fold(FoldContext.small()), equalTo(1000)); + var limitPastMvExpand = asLimit(keep.child(), 1000, false); as(limitPastMvExpand.child(), EsRelation.class); } /** * Expected - * MvExpand[first_name{f}#7,first_name{r}#16,10] - * \_EsqlProject[[first_name{f}#7, last_name{f}#10]] - * \_Limit[1[INTEGER]] - * \_EsRelation[test][_meta_field{f}#12, emp_no{f}#6, first_name{f}#7, ge..] + * Limit[1000[INTEGER],true] + * \_Join[LEFT,[language_code{r}#4],[language_code{r}#4],[language_code{f}#18]] + * |_EsqlProject[[languages{f}#10 AS language_code]] + * | \_Limit[1000[INTEGER],false] + * | \_EsRelation[test][_meta_field{f}#13, emp_no{f}#7, first_name{f}#8, ge..] + * \_EsRelation[languages_lookup][LOOKUP][language_code{f}#18, language_name{f}#19] + */ + public void testCopyDefaultLimitPastLookupJoin() { + LogicalPlan plan = optimizedPlan(""" + from test + | rename languages AS language_code + | keep language_code + | lookup join languages_lookup ON language_code + """); + + var limit = asLimit(plan, 1000, true); + var join = as(limit.child(), Join.class); + var keep = as(join.left(), EsqlProject.class); + var limitPastMvExpand = asLimit(keep.child(), 1000, false); + as(limitPastMvExpand.child(), EsRelation.class); + } + + /** + * Expected + * Limit[10[INTEGER],true] + * \_MvExpand[first_name{f}#7,first_name{r}#17] + * \_EsqlProject[[first_name{f}#7, last_name{f}#10]] + * \_Limit[1[INTEGER],false] + * \_EsRelation[test][_meta_field{f}#12, emp_no{f}#6, first_name{f}#7, ge..] */ public void testDontPushDownLimitPastMvExpand() { LogicalPlan plan = optimizedPlan(""" @@ -1885,28 +1920,56 @@ public void testDontPushDownLimitPastMvExpand() { | limit 1 | keep first_name, last_name | mv_expand first_name - | limit 10"""); + | limit 10 + """); - var mvExpand = as(plan, MvExpand.class); - assertThat(mvExpand.limit(), equalTo(10)); + var limit = asLimit(plan, 10, true); + var mvExpand = as(limit.child(), MvExpand.class); var project = as(mvExpand.child(), EsqlProject.class); - var limit = as(project.child(), Limit.class); - assertThat(limit.limit().fold(FoldContext.small()), equalTo(1)); - as(limit.child(), EsRelation.class); + var limit2 = asLimit(project.child(), 1, false); + as(limit2.child(), EsRelation.class); } /** * Expected - * EsqlProject[[emp_no{f}#19, first_name{r}#29, languages{f}#22, lll{r}#9, salary{r}#30]] - * \_TopN[[Order[salary{r}#30,DESC,FIRST]],5[INTEGER]] - * \_MvExpand[salary{f}#24,salary{r}#30,5] - * \_Eval[[languages{f}#22 + 5[INTEGER] AS lll]] - * \_Limit[5[INTEGER]] - * \_Filter[languages{f}#22 > 1[INTEGER]] - * \_MvExpand[first_name{f}#20,first_name{r}#29,10] - * \_TopN[[Order[emp_no{f}#19,DESC,FIRST]],10[INTEGER]] - * \_Filter[emp_no{f}#19 ≤ 10006[INTEGER]] - * \_EsRelation[test][_meta_field{f}#25, emp_no{f}#19, first_name{f}#20, ..] + * Limit[10[INTEGER],true] + * \_Join[LEFT,[language_code{r}#4],[language_code{r}#4],[language_code{f}#19]] + * |_EsqlProject[[languages{f}#11 AS language_code, last_name{f}#12]] + * | \_Limit[1[INTEGER],false] + * | \_EsRelation[test][_meta_field{f}#14, emp_no{f}#8, first_name{f}#9, ge..] + * \_EsRelation[languages_lookup][LOOKUP][language_code{f}#19, language_name{f}#20] + */ + public void testDontPushDownLimitPastLookupJoin() { + LogicalPlan plan = optimizedPlan(""" + from test + | limit 1 + | rename languages AS language_code + | keep language_code, last_name + | lookup join languages_lookup on language_code + | limit 10 + """); + + var limit = asLimit(plan, 10, true); + var join = as(limit.child(), Join.class); + var project = as(join.left(), EsqlProject.class); + var limit2 = asLimit(project.child(), 1, false); + as(limit2.child(), EsRelation.class); + } + + /** + * Expected + * EsqlProject[[emp_no{f}#19, first_name{r}#30, languages{f}#22, lll{r}#9, salary{r}#31]] + * \_TopN[[Order[salary{r}#31,DESC,FIRST]],5[INTEGER]] + * \_Limit[5[INTEGER],true] + * \_MvExpand[salary{f}#24,salary{r}#31] + * \_Eval[[languages{f}#22 + 5[INTEGER] AS lll]] + * \_Limit[5[INTEGER],false] + * \_Filter[languages{f}#22 > 1[INTEGER]] + * \_Limit[10[INTEGER],true] + * \_MvExpand[first_name{f}#20,first_name{r}#30] + * \_TopN[[Order[emp_no{f}#19,DESC,FIRST]],10[INTEGER]] + * \_Filter[emp_no{f}#19 ≤ 10006[INTEGER]] + * \_EsRelation[test][_meta_field{f}#25, emp_no{f}#19, first_name{f}#20, ..] */ public void testMultipleMvExpandWithSortAndLimit() { LogicalPlan plan = optimizedPlan(""" @@ -1921,25 +1984,86 @@ public void testMultipleMvExpandWithSortAndLimit() { | limit 5 | sort first_name | keep emp_no, first_name, languages, lll, salary - | sort salary desc"""); + | sort salary desc + """); var keep = as(plan, EsqlProject.class); var topN = as(keep.child(), TopN.class); assertThat(topN.limit().fold(FoldContext.small()), equalTo(5)); assertThat(orderNames(topN), contains("salary")); - var mvExp = as(topN.child(), MvExpand.class); - assertThat(mvExp.limit(), equalTo(5)); + var limit5Before = asLimit(topN.child(), 5, true); + var mvExp = as(limit5Before.child(), MvExpand.class); var eval = as(mvExp.child(), Eval.class); - var limit5 = as(eval.child(), Limit.class); + var limit5 = asLimit(eval.child(), 5, false); var filter = as(limit5.child(), Filter.class); - mvExp = as(filter.child(), MvExpand.class); - assertThat(mvExp.limit(), equalTo(10)); + var limit10Before = asLimit(filter.child(), 10, true); + mvExp = as(limit10Before.child(), MvExpand.class); topN = as(mvExp.child(), TopN.class); assertThat(topN.limit().fold(FoldContext.small()), equalTo(10)); filter = as(topN.child(), Filter.class); as(filter.child(), EsRelation.class); } + /** + * Expected + * EsqlProject[[emp_no{f}#24, first_name{f}#25, languages{f}#27, lll{r}#11, salary{f}#29, language_name{f}#38]] + * \_TopN[[Order[salary{f}#29,DESC,FIRST]],5[INTEGER]] + * \_Limit[5[INTEGER],true] + * \_Join[LEFT,[language_code{r}#14],[language_code{r}#14],[language_code{f}#37]] + * |_Project[[_meta_field{f}#30, emp_no{f}#24, first_name{f}#25, gender{f}#26, hire_date{f}#31, job{f}#32, job.raw{f}#33, l + * anguages{f}#27, last_name{f}#28, long_noidx{f}#34, salary{f}#29, language_name{f}#36, lll{r}#11, salary{f}#29 AS language_code]] + * | \_Eval[[languages{f}#27 + 5[INTEGER] AS lll]] + * | \_Limit[5[INTEGER],false] + * | \_Filter[languages{f}#27 > 1[INTEGER]] + * | \_Limit[10[INTEGER],true] + * | \_Join[LEFT,[language_code{r}#6],[language_code{r}#6],[language_code{f}#35]] + * | |_Project[[_meta_field{f}#30, emp_no{f}#24, first_name{f}#25, gender{f}#26, hire_date{f}#31, job{f}#32, + * | | | job.raw{f}#33, languages{f}#27, last_name{f}#28, long_noidx{f}#34, salary{f}#29, + * | | | languages{f}#27 AS language_code]] + * | | \_TopN[[Order[emp_no{f}#24,DESC,FIRST]],10[INTEGER]] + * | | \_Filter[emp_no{f}#24 ≤ 10006[INTEGER]] + * | | \_EsRelation[test][_meta_field{f}#30, emp_no{f}#24, first_name{f}#25, ..] + * | \_EsRelation[languages_lookup][LOOKUP][language_code{f}#35, language_name{f}#36] + * \_EsRelation[languages_lookup][LOOKUP][language_code{f}#37, language_name{f}#38] + */ + public void testMultipleLookupJoinWithSortAndLimit() { + LogicalPlan plan = optimizedPlan(""" + from test + | where emp_no <= 10006 + | sort emp_no desc + | eval language_code = languages + | lookup join languages_lookup on language_code + | limit 10 + | where languages > 1 + | eval lll = languages + 5 + | eval language_code = salary::integer + | lookup join languages_lookup on language_code + | limit 5 + | sort first_name + | keep emp_no, first_name, languages, lll, salary, language_name + | sort salary desc + """); + + var keep = as(plan, EsqlProject.class); + var topN = as(keep.child(), TopN.class); + assertThat(topN.limit().fold(FoldContext.small()), equalTo(5)); + assertThat(orderNames(topN), contains("salary")); + var limit5Before = asLimit(topN.child(), 5, true); + var join = as(limit5Before.child(), Join.class); + var project = as(join.left(), Project.class); + var eval = as(project.child(), Eval.class); + var limit5 = asLimit(eval.child(), 5, false); + var filter = as(limit5.child(), Filter.class); + var limit10Before = asLimit(filter.child(), 10, true); + join = as(limit10Before.child(), Join.class); + project = as(join.left(), Project.class); + topN = as(project.child(), TopN.class); + assertThat(topN.limit().fold(FoldContext.small()), equalTo(10)); + assertThat(orderNames(topN), contains("emp_no")); + filter = as(topN.child(), Filter.class); + as(filter.child(), EsRelation.class); + } + /** * Expected * EsqlProject[[emp_no{f}#350, first_name{f}#351, salary{f}#352]] @@ -2038,12 +2162,13 @@ public void testDontPushDownLimitPastAggregate_AndMvExpand() { * TODO: Push down the filter correctly https://github.com/elastic/elasticsearch/issues/115311 * * Expected - * Limit[5[INTEGER]] - * \_Filter[ISNOTNULL(first_name{r}#22)] - * \_Aggregate[STANDARD,[first_name{r}#22],[MAX(salary{f}#17,true[BOOLEAN]) AS max_s, first_name{r}#22]] - * \_MvExpand[first_name{f}#13,first_name{r}#22,50] - * \_Limit[50[INTEGER]] - * \_EsRelation[test][_meta_field{f}#18, emp_no{f}#12, first_name{f}#13, ..] + * Limit[5[INTEGER],false] + * \_Filter[ISNOTNULL(first_name{r}#23)] + * \_Aggregate[STANDARD,[first_name{r}#23],[MAX(salary{f}#17,true[BOOLEAN]) AS max_s, first_name{r}#23]] + * \_Limit[50[INTEGER],true] + * \_MvExpand[first_name{f}#13,first_name{r}#23] + * \_Limit[50[INTEGER],false] + * \_EsRelation[test][_meta_field{f}#18, emp_no{f}#12, first_name{f}#13, ..] */ public void testPushDown_TheRightLimit_PastMvExpand() { LogicalPlan plan = optimizedPlan(""" @@ -2055,14 +2180,48 @@ public void testPushDown_TheRightLimit_PastMvExpand() { | where first_name is not null | limit 5"""); - var limit = as(plan, Limit.class); - assertThat(limit.limit().fold(FoldContext.small()), equalTo(5)); + var limit = asLimit(plan, 5, false); var filter = as(limit.child(), Filter.class); var agg = as(filter.child(), Aggregate.class); - var mvExp = as(agg.child(), MvExpand.class); - assertThat(mvExp.limit(), equalTo(50)); - limit = as(mvExp.child(), Limit.class); - assertThat(limit.limit().fold(FoldContext.small()), equalTo(50)); + var limit50Before = asLimit(agg.child(), 50, true); + var mvExp = as(limit50Before.child(), MvExpand.class); + limit = asLimit(mvExp.child(), 50, false); + as(limit.child(), EsRelation.class); + } + + /** + * TODO: Push down the filter correctly https://github.com/elastic/elasticsearch/issues/115311 + * + * Expected + * Limit[5[INTEGER],false] + * \_Filter[ISNOTNULL(first_name{f}#15)] + * \_Aggregate[STANDARD,[first_name{f}#15],[MAX(salary{f}#19,true[BOOLEAN]) AS max_s, first_name{f}#15]] + * \_Limit[50[INTEGER],true] + * \_Join[LEFT,[language_code{r}#4],[language_code{r}#4],[language_code{f}#25]] + * |_EsqlProject[[_meta_field{f}#20, emp_no{f}#14, first_name{f}#15, gender{f}#16, hire_date{f}#21, job{f}#22, job.raw{f}#23, l + * anguages{f}#17 AS language_code, last_name{f}#18, long_noidx{f}#24, salary{f}#19]] + * | \_Limit[50[INTEGER],false] + * | \_EsRelation[test][_meta_field{f}#20, emp_no{f}#14, first_name{f}#15, ..] + * \_EsRelation[languages_lookup][LOOKUP][language_code{f}#25] + */ + public void testPushDown_TheRightLimit_PastLookupJoin() { + LogicalPlan plan = optimizedPlan(""" + from test + | rename languages as language_code + | lookup join languages_lookup on language_code + | limit 50 + | keep emp_no, first_name, salary + | stats max_s = max(salary) by first_name + | where first_name is not null + | limit 5"""); + + var limit = asLimit(plan, 5, false); + var filter = as(limit.child(), Filter.class); + var agg = as(filter.child(), Aggregate.class); + var limit50Before = asLimit(agg.child(), 50, true); + var join = as(limit50Before.child(), Join.class); + var project = as(join.left(), Project.class); + limit = asLimit(project.child(), 50, false); as(limit.child(), EsRelation.class); } @@ -2131,10 +2290,11 @@ public void testAddDefaultLimit_BeforeMvExpand_WithFilterOnExpandedField_ResultT /** * Expected * - * MvExpand[first_name{f}#7,first_name{r}#16,10] - * \_TopN[[Order[emp_no{f}#6,DESC,FIRST]],10[INTEGER]] - * \_Filter[emp_no{f}#6 ≤ 10006[INTEGER]] - * \_EsRelation[test][_meta_field{f}#12, emp_no{f}#6, first_name{f}#7, ge..] + * Limit[10[INTEGER],true] + * \_MvExpand[first_name{f}#7,first_name{r}#17] + * \_TopN[[Order[emp_no{f}#6,DESC,FIRST]],10[INTEGER]] + * \_Filter[emp_no{f}#6 ≤ 10006[INTEGER]] + * \_EsRelation[test][_meta_field{f}#12, emp_no{f}#6, first_name{f}#7, ge..] */ public void testFilterWithSortBeforeMvExpand() { LogicalPlan plan = optimizedPlan(""" @@ -2144,8 +2304,8 @@ public void testFilterWithSortBeforeMvExpand() { | mv_expand first_name | limit 10"""); - var mvExp = as(plan, MvExpand.class); - assertThat(mvExp.limit(), equalTo(10)); + var limit = asLimit(plan, 10, true); + var mvExp = as(limit.child(), MvExpand.class); var topN = as(mvExp.child(), TopN.class); assertThat(topN.limit().fold(FoldContext.small()), equalTo(10)); assertThat(orderNames(topN), contains("emp_no")); @@ -2153,6 +2313,36 @@ public void testFilterWithSortBeforeMvExpand() { as(filter.child(), EsRelation.class); } + /** + * Expected + * Limit[10[INTEGER],true] + * \_Join[LEFT,[language_code{r}#6],[language_code{r}#6],[language_code{f}#19]] + * |_EsqlProject[[_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, lan + * guages{f}#11 AS language_code, last_name{f}#12, long_noidx{f}#18, salary{f}#13]] + * | \_TopN[[Order[emp_no{f}#8,DESC,FIRST]],10[INTEGER]] + * | \_Filter[emp_no{f}#8 ≤ 10006[INTEGER]] + * | \_EsRelation[test][_meta_field{f}#14, emp_no{f}#8, first_name{f}#9, ge..] + * \_EsRelation[languages_lookup][LOOKUP][language_code{f}#19, language_name{f}#20] + */ + public void testFilterWithSortBeforeLookupJoin() { + LogicalPlan plan = optimizedPlan(""" + from test + | where emp_no <= 10006 + | sort emp_no desc + | rename languages as language_code + | lookup join languages_lookup on language_code + | limit 10"""); + + var limit = asLimit(plan, 10, true); + var join = as(limit.child(), Join.class); + var project = as(join.left(), Project.class); + var topN = as(project.child(), TopN.class); + assertThat(topN.limit().fold(FoldContext.small()), equalTo(10)); + assertThat(orderNames(topN), contains("emp_no")); + var filter = as(topN.child(), Filter.class); + as(filter.child(), EsRelation.class); + } + /** * Expected * @@ -2184,12 +2374,17 @@ public void testMultiMvExpand_SortDownBelow() { /** * Expected * - * MvExpand[c{r}#7,c{r}#16,10000] - * \_EsqlProject[[c{r}#7, a{r}#3]] - * \_TopN[[Order[a{r}#3,ASC,FIRST]],7300[INTEGER]] - * \_MvExpand[b{r}#5,b{r}#15,7300] - * \_Limit[7300[INTEGER]] - * \_Row[[null[NULL] AS a, 123[INTEGER] AS b, 234[INTEGER] AS c]] + * Limit[10000[INTEGER],true] + * \_MvExpand[c{r}#7,c{r}#16] + * \_EsqlProject[[c{r}#7, a{r}#3]] + * \_TopN[[Order[a{r}#3,ASC,FIRST]],7300[INTEGER]] + * \_Limit[7300[INTEGER],true] + * \_MvExpand[b{r}#5,b{r}#15] + * \_Limit[7300[INTEGER],false] + * \_LocalRelation[[a{r}#3, b{r}#5, c{r}#7],[ConstantNullBlock[positions=1], + * IntVectorBlock[vector=ConstantIntVector[positions=1, value=123]], + * IntVectorBlock[vector=ConstantIntVector[positions=1, value=234]]]] + * */ public void testLimitThenSortBeforeMvExpand() { LogicalPlan plan = optimizedPlan(""" @@ -2200,15 +2395,53 @@ public void testLimitThenSortBeforeMvExpand() { | sort a NULLS FIRST | mv_expand c"""); - var mvExpand = as(plan, MvExpand.class); - assertThat(mvExpand.limit(), equalTo(10000)); + var limit10kBefore = asLimit(plan, 10000, true); + var mvExpand = as(limit10kBefore.child(), MvExpand.class); var project = as(mvExpand.child(), EsqlProject.class); var topN = as(project.child(), TopN.class); assertThat(topN.limit().fold(FoldContext.small()), equalTo(7300)); assertThat(orderNames(topN), contains("a")); - mvExpand = as(topN.child(), MvExpand.class); - var limit = as(mvExpand.child(), Limit.class); - assertThat(limit.limit().fold(FoldContext.small()), equalTo(7300)); + var limit7300Before = asLimit(topN.child(), 7300, true); + mvExpand = as(limit7300Before.child(), MvExpand.class); + var limit = asLimit(mvExpand.child(), 7300, false); + as(limit.child(), LocalRelation.class); + } + + /** + * Expects + * Limit[10000[INTEGER],true] + * \_Join[LEFT,[language_code{r}#14],[language_code{r}#14],[language_code{f}#18]] + * |_EsqlProject[[c{r}#7 AS language_code, a{r}#3]] + * | \_TopN[[Order[a{r}#3,ASC,FIRST]],7300[INTEGER]] + * | \_Limit[7300[INTEGER],true] + * | \_Join[LEFT,[language_code{r}#5],[language_code{r}#5],[language_code{f}#16]] + * | |_Limit[7300[INTEGER],false] + * | | \_LocalRelation[[a{r}#3, language_code{r}#5, c{r}#7],[ConstantNullBlock[positions=1], + * IntVectorBlock[vector=ConstantIntVector[positions=1, value=123]], + * IntVectorBlock[vector=ConstantIntVector[positions=1, value=234]]]] + * | \_EsRelation[languages_lookup][LOOKUP][language_code{f}#16] + * \_EsRelation[languages_lookup][LOOKUP][language_code{f}#18, language_name{f}#19] + */ + public void testLimitThenSortBeforeLookupJoin() { + LogicalPlan plan = optimizedPlan(""" + row a = null, language_code = 123, c = 234 + | lookup join languages_lookup on language_code + | limit 7300 + | keep c, a + | sort a NULLS FIRST + | rename c as language_code + | lookup join languages_lookup on language_code + """); + + var limit10kBefore = asLimit(plan, 10000, true); + var join = as(limit10kBefore.child(), Join.class); + var project = as(join.left(), EsqlProject.class); + var topN = as(project.child(), TopN.class); + assertThat(topN.limit().fold(FoldContext.small()), equalTo(7300)); + assertThat(orderNames(topN), contains("a")); + var limit7300Before = asLimit(topN.child(), 7300, true); + join = as(limit7300Before.child(), Join.class); + var limit = asLimit(join.left(), 7300, false); as(limit.child(), LocalRelation.class); } @@ -2341,27 +2574,51 @@ public void testAddDefaultLimit_BeforeMvExpand_WithFilterOnExpandedFieldAlias() /** * Expected: - * MvExpand[a{r}#1402,a{r}#1406,1000] - * \_TopN[[Order[a{r}#1402,ASC,LAST]],1000[INTEGER]] - * \_Row[[1[INTEGER] AS a]] + * Limit[1000[INTEGER],true] + * \_MvExpand[a{r}#3,a{r}#7] + * \_TopN[[Order[a{r}#3,ASC,LAST]],1000[INTEGER]] + * \_LocalRelation[[a{r}#3],[IntVectorBlock[vector=ConstantIntVector[positions=1, value=1]]]] */ public void testSortMvExpand() { LogicalPlan plan = optimizedPlan(""" row a = 1 | sort a - | mv_expand a"""); + | mv_expand a + """); - var expand = as(plan, MvExpand.class); - assertThat(expand.limit(), equalTo(1000)); + var limit = asLimit(plan, 1000, true); + var expand = as(limit.child(), MvExpand.class); var topN = as(expand.child(), TopN.class); var row = as(topN.child(), LocalRelation.class); } /** * Expected: - * MvExpand[emp_no{f}#5,emp_no{r}#15,20] - * \_TopN[[Order[emp_no{f}#5,ASC,LAST]],20[INTEGER]] - * \_EsRelation[test][_meta_field{f}#11, emp_no{f}#5, first_name{f}#6, ge..] + * Limit[1000[INTEGER],true] + * \_Join[LEFT,[language_code{r}#3],[language_code{r}#3],[language_code{f}#6]] + * |_TopN[[Order[language_code{r}#3,ASC,LAST]],1000[INTEGER]] + * | \_LocalRelation[[language_code{r}#3],[IntVectorBlock[vector=ConstantIntVector[positions=1, value=1]]]] + * \_EsRelation[languages_lookup][LOOKUP][language_code{f}#6, language_name{f}#7] + */ + public void testSortLookupJoin() { + LogicalPlan plan = optimizedPlan(""" + row language_code = 1 + | sort language_code + | lookup join languages_lookup on language_code + """); + + var limit = asLimit(plan, 1000, true); + var join = as(limit.child(), Join.class); + var topN = as(join.left(), TopN.class); + var row = as(topN.child(), LocalRelation.class); + } + + /** + * Expected: + * Limit[20[INTEGER],true] + * \_MvExpand[emp_no{f}#5,emp_no{r}#16] + * \_TopN[[Order[emp_no{f}#5,ASC,LAST]],20[INTEGER]] + * \_EsRelation[test][_meta_field{f}#11, emp_no{f}#5, first_name{f}#6, ge..] */ public void testSortMvExpandLimit() { LogicalPlan plan = optimizedPlan(""" @@ -2370,8 +2627,8 @@ public void testSortMvExpandLimit() { | mv_expand emp_no | limit 20"""); - var expand = as(plan, MvExpand.class); - assertThat(expand.limit(), equalTo(20)); + var limit = asLimit(plan, 20, true); + var expand = as(limit.child(), MvExpand.class); var topN = as(expand.child(), TopN.class); assertThat(topN.limit().fold(FoldContext.small()), is(20)); var row = as(topN.child(), EsRelation.class); @@ -2379,9 +2636,37 @@ public void testSortMvExpandLimit() { /** * Expected: - * MvExpand[b{r}#5,b{r}#9,1000] - * \_Limit[1000[INTEGER]] - * \_Row[[1[INTEGER] AS a, -15[INTEGER] AS b]] + * Limit[20[INTEGER],true] + * \_Join[LEFT,[language_code{r}#5],[language_code{r}#5],[language_code{f}#18]] + * |_EsqlProject[[_meta_field{f}#13, emp_no{f}#7 AS language_code, first_name{f}#8, gender{f}#9, hire_date{f}#14, job{f}#15, jo + * b.raw{f}#16, languages{f}#10, last_name{f}#11, long_noidx{f}#17, salary{f}#12]] + * | \_TopN[[Order[emp_no{f}#7,ASC,LAST]],20[INTEGER]] + * | \_EsRelation[test][_meta_field{f}#13, emp_no{f}#7, first_name{f}#8, ge..] + * \_EsRelation[languages_lookup][LOOKUP][language_code{f}#18, language_name{f}#19] + */ + public void testSortLookupJoinLimit() { + LogicalPlan plan = optimizedPlan(""" + from test + | sort emp_no + | rename emp_no as language_code + | lookup join languages_lookup on language_code + | limit 20"""); + + var limit = asLimit(plan, 20, true); + var join = as(limit.child(), Join.class); + var project = as(join.left(), Project.class); + var topN = as(project.child(), TopN.class); + assertThat(topN.limit().fold(FoldContext.small()), is(20)); + var row = as(topN.child(), EsRelation.class); + } + + /** + * Expected: + * Limit[1000[INTEGER],true] + * \_MvExpand[b{r}#5,b{r}#9] + * \_Limit[1000[INTEGER],false] + * \_LocalRelation[[a{r}#3, b{r}#5],[IntVectorBlock[vector=ConstantIntVector[positions=1, value=1]], + * IntVectorBlock[vector=ConstantIntVector[positions=1, value=-15]]]] * * see https://github.com/elastic/elasticsearch/issues/102084 */ @@ -2389,15 +2674,90 @@ public void testWhereMvExpand() { LogicalPlan plan = optimizedPlan(""" row a = 1, b = -15 | where b < 3 - | mv_expand b"""); + | mv_expand b + """); + + var limit = asLimit(plan, 1000, true); + var expand = as(limit.child(), MvExpand.class); + var limit2 = asLimit(expand.child(), 1000, false); + var row = as(limit2.child(), LocalRelation.class); + } + + /** + * Expected: + * Limit[1000[INTEGER],true] + * \_Join[LEFT,[language_code{r}#5],[language_code{r}#5],[language_code{f}#8]] + * |_Limit[1000[INTEGER],false] + * | \_LocalRelation[[a{r}#3, language_code{r}#5],[IntVectorBlock[vector=ConstantIntVector[positions=1, value=1]], IntVectorBlock[ve + * ctor=ConstantIntVector[positions=1, value=-15]]]] + * \_EsRelation[languages_lookup][LOOKUP][language_code{f}#8, language_name{f}#9] + */ + public void testWhereLookupJoin() { + LogicalPlan plan = optimizedPlan(""" + row a = 1, language_code = -15 + | where language_code < 3 + | lookup join languages_lookup on language_code + """); - var expand = as(plan, MvExpand.class); - assertThat(expand.limit(), equalTo(1000)); - var limit2 = as(expand.child(), Limit.class); - assertThat(limit2.limit().fold(FoldContext.small()), is(1000)); + var limit = asLimit(plan, 1000, true); + var join = as(limit.child(), Join.class); + var limit2 = asLimit(join.left(), 1000, false); var row = as(limit2.child(), LocalRelation.class); } + /** + * Expects + * TopN[[Order[language_code{r}#7,ASC,LAST]],1[INTEGER]] + * \_Limit[1[INTEGER],true] + * \_MvExpand[language_code{r}#3,language_code{r}#7] + * \_Limit[1[INTEGER],false] + * \_LocalRelation[[language_code{r}#3],[IntVectorBlock[vector=ConstantIntVector[positions=1, value=1]]]] + * + * Notice that the `TopN` at the very top has limit 1, not 3! + */ + public void testDescendantLimitMvExpand() { + LogicalPlan plan = optimizedPlan(""" + ROW language_code = 1 + | MV_EXPAND language_code + | LIMIT 1 + | SORT language_code + | LIMIT 3 + """); + + var topn = as(plan, TopN.class); + var limitAfter = asLimit(topn.child(), 1, true); + var mvExpand = as(limitAfter.child(), MvExpand.class); + var limitBefore = asLimit(mvExpand.child(), 1, false); + var localRelation = as(limitBefore.child(), LocalRelation.class); + } + + /** + * Expects + * TopN[[Order[language_code{r}#3,ASC,LAST]],1[INTEGER]] + * \_Limit[1[INTEGER],true] + * \_Join[LEFT,[language_code{r}#3],[language_code{r}#3],[language_code{f}#6]] + * |_Limit[1[INTEGER],false] + * | \_LocalRelation[[language_code{r}#3],[IntVectorBlock[vector=ConstantIntVector[positions=1, value=1]]]] + * \_EsRelation[languages_lookup][LOOKUP][language_code{f}#6, language_name{f}#7] + * + * Notice that the `TopN` at the very top has limit 1, not 3! + */ + public void testDescendantLimitLookupJoin() { + LogicalPlan plan = optimizedPlan(""" + ROW language_code = 1 + | LOOKUP JOIN languages_lookup ON language_code + | LIMIT 1 + | SORT language_code + | LIMIT 3 + """); + + var topn = as(plan, TopN.class); + var limitAfter = asLimit(topn.child(), 1, true); + var join = as(limitAfter.child(), Join.class); + var limitBefore = asLimit(join.left(), 1, false); + var localRelation = as(limitBefore.child(), LocalRelation.class); + } + private static List orderNames(TopN topN) { return topN.order().stream().map(o -> as(o.child(), NamedExpression.class).name()).toList(); } @@ -4930,7 +5290,17 @@ public void testPlanSanityCheck() throws Exception { assertThat(e.getMessage(), containsString(" optimized incorrectly due to missing references [salary")); } - public void testPlanSanityCheckWithBinaryPlans() throws Exception { + /** + * Expects + * Limit[1000[INTEGER],true] + * \_Join[LEFT,[language_code{r}#4],[language_code{r}#4],[language_code{f}#17]] + * |_EsqlProject[[_meta_field{f}#12, emp_no{f}#6, first_name{f}#7, gender{f}#8, hire_date{f}#13, job{f}#14, job.raw{f}#15, lang + * uages{f}#9 AS language_code, last_name{f}#10, long_noidx{f}#16, salary{f}#11]] + * | \_Limit[1000[INTEGER],false] + * | \_EsRelation[test][_meta_field{f}#12, emp_no{f}#6, first_name{f}#7, ge..] + * \_EsRelation[languages_lookup][LOOKUP][language_code{f}#17, language_name{f}#18] + */ + public void testPlanSanityCheckWithBinaryPlans() { assumeTrue("Requires LOOKUP JOIN", EsqlCapabilities.Cap.JOIN_LOOKUP_V12.isEnabled()); var plan = optimizedPlan(""" @@ -4939,7 +5309,8 @@ public void testPlanSanityCheckWithBinaryPlans() throws Exception { | LOOKUP JOIN languages_lookup ON language_code """); - var join = as(plan, Join.class); + var upperLimit = asLimit(plan, null, true); + var join = as(upperLimit.child(), Join.class); var joinWithInvalidLeftPlan = join.replaceChildren(join.right(), join.right()); IllegalStateException e = expectThrows(IllegalStateException.class, () -> logicalOptimizer.optimize(joinWithInvalidLeftPlan)); @@ -5995,15 +6366,15 @@ public void testLookupStats() { /** * Filter on join keys should be pushed down * Expects - * Project[[_meta_field{f}#13, emp_no{f}#7, first_name{f}#8, gender{f}#9, hire_date{f}#14, job{f}#15, job.raw{f}#16, lang - * uage_code{r}#4, last_name{f}#11, long_noidx{f}#17, salary{f}#12, language_name{f}#19]] - * \_Join[LEFT,[language_code{r}#4],[language_code{r}#4],[language_code{f}#18]] - * |_EsqlProject[[_meta_field{f}#13, emp_no{f}#7, first_name{f}#8, gender{f}#9, hire_date{f}#14, job{f}#15, job.raw{f}#16, lang + * + * Limit[1000[INTEGER],true] + * \_Join[LEFT,[language_code{r}#4],[language_code{r}#4],[language_code{f}#18]] + * |_EsqlProject[[_meta_field{f}#13, emp_no{f}#7, first_name{f}#8, gender{f}#9, hire_date{f}#14, job{f}#15, job.raw{f}#16, lang * uages{f}#10 AS language_code, last_name{f}#11, long_noidx{f}#17, salary{f}#12]] - * | \_Limit[1000[INTEGER]] - * | \_Filter[languages{f}#10 > 1[INTEGER]] - * | \_EsRelation[test][_meta_field{f}#13, emp_no{f}#7, first_name{f}#8, ge..] - * \_EsRelation[languages_lookup][LOOKUP][language_code{f}#18, language_name{f}#19] + * | \_Limit[1000[INTEGER],false] + * | \_Filter[languages{f}#10 > 1[INTEGER]] + * | \_EsRelation[test][_meta_field{f}#13, emp_no{f}#7, first_name{f}#8, ge..] + * \_EsRelation[languages_lookup][LOOKUP][language_code{f}#18, language_name{f}#19] */ public void testLookupJoinPushDownFilterOnJoinKeyWithRename() { assumeTrue("Requires LOOKUP JOIN", EsqlCapabilities.Cap.JOIN_LOOKUP_V12.isEnabled()); @@ -6016,11 +6387,11 @@ public void testLookupJoinPushDownFilterOnJoinKeyWithRename() { """; var plan = optimizedPlan(query); - var join = as(plan, Join.class); + var upperLimit = asLimit(plan, 1000, true); + var join = as(upperLimit.child(), Join.class); assertThat(join.config().type(), equalTo(JoinTypes.LEFT)); var project = as(join.left(), Project.class); - var limit = as(project.child(), Limit.class); - assertThat(limit.limit().fold(FoldContext.small()), equalTo(1000)); + var limit = asLimit(project.child(), 1000, false); var filter = as(limit.child(), Filter.class); // assert that the rename has been undone var op = as(filter.condition(), GreaterThan.class); @@ -6037,15 +6408,14 @@ public void testLookupJoinPushDownFilterOnJoinKeyWithRename() { /** * Filter on on left side fields (outside the join key) should be pushed down * Expects - * Project[[_meta_field{f}#13, emp_no{f}#7, first_name{f}#8, gender{f}#9, hire_date{f}#14, job{f}#15, job.raw{f}#16, lang - * uage_code{r}#4, last_name{f}#11, long_noidx{f}#17, salary{f}#12, language_name{f}#19]] - * \_Join[LEFT,[language_code{r}#4],[language_code{r}#4],[language_code{f}#18]] - * |_EsqlProject[[_meta_field{f}#13, emp_no{f}#7, first_name{f}#8, gender{f}#9, hire_date{f}#14, job{f}#15, job.raw{f}#16, lang + * Limit[1000[INTEGER],true] + * \_Join[LEFT,[language_code{r}#4],[language_code{r}#4],[language_code{f}#18]] + * |_EsqlProject[[_meta_field{f}#13, emp_no{f}#7, first_name{f}#8, gender{f}#9, hire_date{f}#14, job{f}#15, job.raw{f}#16, lang * uages{f}#10 AS language_code, last_name{f}#11, long_noidx{f}#17, salary{f}#12]] - * | \_Limit[1000[INTEGER]] - * | \_Filter[emp_no{f}#7 > 1[INTEGER]] - * | \_EsRelation[test][_meta_field{f}#13, emp_no{f}#7, first_name{f}#8, ge..] - * \_EsRelation[languages_lookup][LOOKUP][language_code{f}#18, language_name{f}#19] + * | \_Limit[1000[INTEGER],false] + * | \_Filter[emp_no{f}#7 > 1[INTEGER]] + * | \_EsRelation[test][_meta_field{f}#13, emp_no{f}#7, first_name{f}#8, ge..] + * \_EsRelation[languages_lookup][LOOKUP][language_code{f}#18, language_name{f}#19] */ public void testLookupJoinPushDownFilterOnLeftSideField() { assumeTrue("Requires LOOKUP JOIN", EsqlCapabilities.Cap.JOIN_LOOKUP_V12.isEnabled()); @@ -6059,12 +6429,12 @@ public void testLookupJoinPushDownFilterOnLeftSideField() { var plan = optimizedPlan(query); - var join = as(plan, Join.class); + var upperLimit = asLimit(plan, 1000, true); + var join = as(upperLimit.child(), Join.class); assertThat(join.config().type(), equalTo(JoinTypes.LEFT)); var project = as(join.left(), Project.class); - var limit = as(project.child(), Limit.class); - assertThat(limit.limit().fold(FoldContext.small()), equalTo(1000)); + var limit = asLimit(project.child(), 1000, false); var filter = as(limit.child(), Filter.class); var op = as(filter.condition(), GreaterThan.class); var field = as(op.left(), FieldAttribute.class); @@ -6226,14 +6596,16 @@ public void testLookupJoinPushDownDisabledForDisjunctionBetweenLeftAndRightField /** * When dropping lookup fields, the lookup relation shouldn't include them. * At least until we can implement InsertFieldExtract there. + * * Expects - * EsqlProject[[languages{f}#10]] - * \_Join[LEFT,[language_code{r}#4],[language_code{r}#4],[language_code{f}#18]] - * |_Project[[_meta_field{f}#13, emp_no{f}#7, first_name{f}#8, gender{f}#9, hire_date{f}#14, job{f}#15, job.raw{f}#16, lang - * uages{f}#10, last_name{f}#11, long_noidx{f}#17, salary{f}#12, languages{f}#10 AS language_code]] - * | \_Limit[1000[INTEGER]] - * | \_EsRelation[test][_meta_field{f}#13, emp_no{f}#7, first_name{f}#8, ge..] - * \_EsRelation[languages_lookup][LOOKUP][language_code{f}#18] + * EsqlProject[[languages{f}#21]] + * \_Limit[1000[INTEGER],true] + * \_Join[LEFT,[language_code{r}#4],[language_code{r}#4],[language_code{f}#29]] + * |_Project[[_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, l + * anguages{f}#21, last_name{f}#22, long_noidx{f}#28, salary{f}#23, languages{f}#21 AS language_code]] + * | \_Limit[1000[INTEGER],false] + * | \_EsRelation[test][_meta_field{f}#24, emp_no{f}#18, first_name{f}#19, ..] + * \_EsRelation[languages_lookup][LOOKUP][language_code{f}#29] */ public void testLookupJoinKeepNoLookupFields() { assumeTrue("Requires LOOKUP JOIN", EsqlCapabilities.Cap.JOIN_LOOKUP_V12.isEnabled()); @@ -6255,7 +6627,9 @@ public void testLookupJoinKeepNoLookupFields() { assertThat(project.projections().size(), equalTo(1)); assertThat(project.projections().get(0).name(), equalTo("languages")); - var join = as(project.child(), Join.class); + var limit = asLimit(project.child(), 1000, true); + + var join = as(limit.child(), Join.class); var joinRightRelation = as(join.right(), EsRelation.class); assertThat(joinRightRelation.output().size(), equalTo(1)); @@ -6266,13 +6640,15 @@ public void testLookupJoinKeepNoLookupFields() { * Ensure a JOIN shadowed by another JOIN doesn't request the shadowed fields. * * Expected - * Join[LEFT,[language_code{r}#4],[language_code{r}#4],[language_code{f}#20]] - * |_Join[LEFT,[language_code{r}#4],[language_code{r}#4],[language_code{f}#18]] - * | |_Eval[[languages{f}#10 AS language_code]] - * | | \_Limit[1000[INTEGER]] - * | | \_EsRelation[test][_meta_field{f}#13, emp_no{f}#7, first_name{f}#8, ge..] - * | \_EsRelation[languages_lookup][LOOKUP][language_code{f}#18] - * \_EsRelation[languages_lookup][LOOKUP][language_code{f}#20, language_name{f}#21] + * Limit[1000[INTEGER],true] + * \_Join[LEFT,[language_code{r}#4],[language_code{r}#4],[language_code{f}#20]] + * |_Limit[1000[INTEGER],true] + * | \_Join[LEFT,[language_code{r}#4],[language_code{r}#4],[language_code{f}#18]] + * | |_Eval[[languages{f}#10 AS language_code]] + * | | \_Limit[1000[INTEGER],false] + * | | \_EsRelation[test][_meta_field{f}#13, emp_no{f}#7, first_name{f}#8, ge..] + * | \_EsRelation[languages_lookup][LOOKUP][language_code{f}#18] + * \_EsRelation[languages_lookup][LOOKUP][language_code{f}#20, language_name{f}#21] */ public void testMultipleLookupShadowing() { assumeTrue("Requires LOOKUP JOIN", EsqlCapabilities.Cap.JOIN_LOOKUP_V12.isEnabled()); @@ -6286,18 +6662,25 @@ public void testMultipleLookupShadowing() { var plan = optimizedPlan(query); - var finalJoin = as(plan, Join.class); + var limit1 = asLimit(plan, 1000, true); + + var finalJoin = as(limit1.child(), Join.class); var finalJoinRightRelation = as(finalJoin.right(), EsRelation.class); assertThat(finalJoinRightRelation.output().size(), equalTo(2)); assertThat(finalJoinRightRelation.output().get(0).name(), equalTo("language_code")); assertThat(finalJoinRightRelation.output().get(1).name(), equalTo("language_name")); - var initialJoin = as(finalJoin.left(), Join.class); + var limit2 = asLimit(finalJoin.left(), 1000, true); + + var initialJoin = as(limit2.child(), Join.class); var initialJoinRightRelation = as(initialJoin.right(), EsRelation.class); assertThat(initialJoinRightRelation.output().size(), equalTo(1)); assertThat(initialJoinRightRelation.output().get(0).name(), equalTo("language_code")); + + var eval = as(initialJoin.left(), Eval.class); + var limit3 = asLimit(eval.child(), 1000, false); } // diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java index dcc549057b77a..af0a9c2f97961 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java @@ -1695,8 +1695,7 @@ public void testParamForIdentifier() { List.of(new Order(EMPTY, attribute("f.11..f.12.*"), Order.OrderDirection.ASC, Order.NullsPosition.LAST)) ), attribute("f.*.13.f.14*"), - attribute("f.*.13.f.14*"), - null + attribute("f.*.13.f.14*") ), statement( """ diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plan/AbstractNodeSerializationTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plan/AbstractNodeSerializationTests.java index e6faa9a253d76..998b895a4e005 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plan/AbstractNodeSerializationTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plan/AbstractNodeSerializationTests.java @@ -51,7 +51,7 @@ public static List randomFieldAttributes(int min, int max, boolean on } @Override - protected final T copyInstance(T instance, TransportVersion version) throws IOException { + protected T copyInstance(T instance, TransportVersion version) throws IOException { return copyInstance( instance, getNamedWriteableRegistry(), diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plan/logical/LimitSerializationTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plan/logical/LimitSerializationTests.java index 5d994eb2880ba..b1ffb9c5f8ba8 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plan/logical/LimitSerializationTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plan/logical/LimitSerializationTests.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.esql.plan.logical; +import org.elasticsearch.TransportVersion; import org.elasticsearch.xpack.esql.core.expression.Expression; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.expression.function.FieldAttributeTests; @@ -19,23 +20,33 @@ protected Limit createTestInstance() { Source source = randomSource(); Expression limit = FieldAttributeTests.createFieldAttribute(0, false); LogicalPlan child = randomChild(0); - return new Limit(source, limit, child); + return new Limit(source, limit, child, randomBoolean()); } @Override protected Limit mutateInstance(Limit instance) throws IOException { Expression limit = instance.limit(); LogicalPlan child = instance.child(); - if (randomBoolean()) { - limit = randomValueOtherThan(limit, () -> FieldAttributeTests.createFieldAttribute(0, false)); - } else { - child = randomValueOtherThan(child, () -> randomChild(0)); + boolean duplicated = instance.duplicated(); + switch (randomIntBetween(0, 2)) { + case 0 -> limit = randomValueOtherThan(limit, () -> FieldAttributeTests.createFieldAttribute(0, false)); + case 1 -> child = randomValueOtherThan(child, () -> randomChild(0)); + case 2 -> duplicated = duplicated == false; + default -> throw new IllegalStateException("Should never reach here"); } - return new Limit(instance.source(), limit, child); + return new Limit(instance.source(), limit, child, duplicated); } @Override protected boolean alwaysEmptySource() { return true; } + + @Override + protected Limit copyInstance(Limit instance, TransportVersion version) throws IOException { + // Limit#duplicated() is ALWAYS false when being serialized and we assert that in Limit#writeTo(). + // So, we need to manually simulate this situation. + Limit deserializedCopy = super.copyInstance(instance.withDuplicated(false), version); + return deserializedCopy.withDuplicated(instance.duplicated()); + } }