Skip to content

Commit 8c0c305

Browse files
committed
Remove limit optimization - data node optimizer will take care of it
1 parent 3f58415 commit 8c0c305

File tree

2 files changed

+11
-9
lines changed

2 files changed

+11
-9
lines changed

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/mapper/Mapper.java

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -126,13 +126,7 @@ else if (aggregate.groupings()
126126
}
127127

128128
if (unary instanceof Limit limit) {
129-
// Avoid extra limit for remote Enrich, since it has already been applied before Enrich if it's duplicate.
130-
// TODO: can we do this in the optimizer instead?
131-
if (limit.duplicated() && limit.child() instanceof Enrich) {
132-
mappedChild = addExchangeForFragment(limit.child(), mappedChild);
133-
} else {
134-
mappedChild = addExchangeForFragment(limit, mappedChild);
135-
}
129+
mappedChild = addExchangeForFragment(limit, mappedChild);
136130
return new LimitExec(limit.source(), mappedChild, limit.limit(), null);
137131
}
138132

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7017,11 +7017,15 @@ public void testEnrichBeforeLimit() {
70177017
var finalLimit = as(plan, LimitExec.class);
70187018
var exchange = as(finalLimit.child(), ExchangeExec.class);
70197019
var fragment = as(exchange.child(), FragmentExec.class);
7020-
var enrich = as(fragment.fragment(), Enrich.class);
7020+
var enrichLimit = as(fragment.fragment(), Limit.class);
7021+
assertThat(Foldables.limitValue(enrichLimit.limit(), enrichLimit.sourceText()), equalTo(10));
7022+
var enrich = as(enrichLimit.child(), Enrich.class);
70217023
assertThat(enrich.mode(), equalTo(Enrich.Mode.REMOTE));
70227024
assertThat(enrich.concreteIndices(), equalTo(Map.of("cluster_1", ".enrich-departments-2")));
70237025
var evalFragment = as(enrich.child(), Eval.class);
70247026
var partialLimit = as(evalFragment.child(), Limit.class);
7027+
assertThat(Foldables.limitValue(partialLimit.limit(), partialLimit.sourceText()), equalTo(10));
7028+
assertTrue(partialLimit.local());
70257029
as(partialLimit.child(), EsRelation.class);
70267030
}
70277031
}
@@ -7075,11 +7079,15 @@ public void testLimitThenEnrichRemote() {
70757079
var finalLimit = as(plan, LimitExec.class);
70767080
var exchange = as(finalLimit.child(), ExchangeExec.class);
70777081
var fragment = as(exchange.child(), FragmentExec.class);
7078-
var enrich = as(fragment.fragment(), Enrich.class);
7082+
var enrichLimit = as(fragment.fragment(), Limit.class);
7083+
assertThat(Foldables.limitValue(enrichLimit.limit(), enrichLimit.sourceText()), equalTo(10));
7084+
var enrich = as(enrichLimit.child(), Enrich.class);
70797085
assertThat(enrich.mode(), equalTo(Enrich.Mode.REMOTE));
70807086
assertThat(enrich.concreteIndices(), equalTo(Map.of("cluster_1", ".enrich-departments-2")));
70817087
var evalFragment = as(enrich.child(), Eval.class);
70827088
var partialLimit = as(evalFragment.child(), Limit.class);
7089+
assertThat(Foldables.limitValue(enrichLimit.limit(), enrichLimit.sourceText()), equalTo(10));
7090+
assertTrue(partialLimit.local());
70837091
as(partialLimit.child(), EsRelation.class);
70847092
}
70857093

0 commit comments

Comments
 (0)