Skip to content

Commit 53560f1

Browse files
authored
Merge branch 'main' into tvclean0
2 parents f8529e2 + c80fa9a commit 53560f1

File tree

2 files changed

+16
-4
lines changed

2 files changed

+16
-4
lines changed

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

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,20 @@ private static Limit combineLimits(Limit upper, Limit lower, FoldContext ctx) {
101101
// Keep the smallest limit
102102
var upperLimitValue = (int) upper.limit().fold(ctx);
103103
var lowerLimitValue = (int) lower.limit().fold(ctx);
104-
// We want to preserve the duplicated() value of the smaller limit.
105-
if (lowerLimitValue <= upperLimitValue) {
106-
return lower.withLocal(lower.local());
104+
/*
105+
* We always want to select the smaller of the limits, but with the local flag it gets a bit tricky.
106+
* If one of the limits is smaller, that's what we will choose. But if the limits are exactly equal,
107+
* then we can choose the local limit because this may enable some queries that would otherwise be prohibited
108+
* due to pipeline-breaking nature of non-local limits in combination with remote Enrich.
109+
* However this may not be true if we have more situations where local limits are generated which do not have
110+
* guarantees that local limit produced by remote enrich pushing provides.
111+
* See also: https://github.com/elastic/elasticsearch/pull/139399#pullrequestreview-3573026118
112+
*/
113+
if (lowerLimitValue < upperLimitValue) {
114+
return lower;
115+
} else if (lowerLimitValue == upperLimitValue) {
116+
// If any of them is local, we want the local limit
117+
return lower.local() ? lower : lower.withLocal(upper.local());
107118
} else {
108119
return new Limit(upper.source(), upper.limit(), lower.child(), upper.duplicated(), upper.local());
109120
}

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineLimitsTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,8 @@ public void testPushableLimitDuplicate() {
242242
int precedingLimitValue = randomIntBetween(1, 10_000);
243243
Limit precedingLimit = new Limit(EMPTY, new Literal(EMPTY, precedingLimitValue, INTEGER), relation);
244244
LogicalPlan duplicatingLimitTestPlan = duplicatingTestCase.buildPlan(precedingLimit, a);
245-
int upperLimitValue = randomIntBetween(1, precedingLimitValue);
245+
// Explicitly raise the probability of equal limits, to test for https://github.com/elastic/elasticsearch/issues/139250
246+
int upperLimitValue = randomBoolean() ? precedingLimitValue : randomIntBetween(1, precedingLimitValue);
246247
Limit upperLimit = new Limit(EMPTY, new Literal(EMPTY, upperLimitValue, INTEGER), duplicatingLimitTestPlan);
247248
Limit optimizedPlan = as(optimizePlan(upperLimit), Limit.class);
248249
duplicatingTestCase.checkOptimizedPlan(duplicatingLimitTestPlan, optimizedPlan.child());

0 commit comments

Comments
 (0)