-
Notifications
You must be signed in to change notification settings - Fork 25.6k
ESQL: push down LIMIT past LOOKUP JOIN #118495
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
ef0e9f6
9d0772b
809334f
7a27516
846d596
eaacda3
a213b8a
07a6ce6
ba332c2
a4db1dc
6bfc807
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,7 +19,6 @@ | |
import org.elasticsearch.xpack.esql.plan.logical.UnaryPlan; | ||
import org.elasticsearch.xpack.esql.plan.logical.join.Join; | ||
import org.elasticsearch.xpack.esql.plan.logical.join.JoinTypes; | ||
import org.elasticsearch.xpack.esql.plan.logical.local.LocalRelation; | ||
|
||
public final class PushDownAndCombineLimits extends OptimizerRules.OptimizerRule<Limit> { | ||
|
||
|
@@ -63,8 +62,10 @@ public LogicalPlan rule(Limit limit) { | |
} | ||
} | ||
} else if (limit.child() instanceof Join join) { | ||
if (join.config().type() == JoinTypes.LEFT && join.right() instanceof LocalRelation) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wondering why the previous restriction to LocalRelation (even if that's what was expected before). Anyways, should be good this way now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That came from when we only had |
||
// This is a hash join from something like a lookup. | ||
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 lead are collected into multi-values. | ||
alex-spies marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
return join.replaceChildren(limit.replaceChild(join.left()), join.right()); | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,6 +41,7 @@ | |
import org.elasticsearch.xpack.esql.core.expression.predicate.logical.Or; | ||
import org.elasticsearch.xpack.esql.core.expression.predicate.nulls.IsNotNull; | ||
import org.elasticsearch.xpack.esql.core.expression.predicate.operator.comparison.BinaryComparison; | ||
import org.elasticsearch.xpack.esql.core.tree.Source; | ||
import org.elasticsearch.xpack.esql.core.type.DataType; | ||
import org.elasticsearch.xpack.esql.core.type.EsField; | ||
import org.elasticsearch.xpack.esql.core.util.Holder; | ||
|
@@ -113,7 +114,9 @@ | |
import org.elasticsearch.xpack.esql.plan.logical.UnaryPlan; | ||
import org.elasticsearch.xpack.esql.plan.logical.join.InlineJoin; | ||
import org.elasticsearch.xpack.esql.plan.logical.join.Join; | ||
import org.elasticsearch.xpack.esql.plan.logical.join.JoinConfig; | ||
import org.elasticsearch.xpack.esql.plan.logical.join.JoinTypes; | ||
import org.elasticsearch.xpack.esql.plan.logical.join.LookupJoin; | ||
import org.elasticsearch.xpack.esql.plan.logical.local.EsqlProject; | ||
import org.elasticsearch.xpack.esql.plan.logical.local.LocalRelation; | ||
import org.elasticsearch.xpack.esql.plan.logical.local.LocalSupplier; | ||
|
@@ -139,6 +142,7 @@ | |
import static org.elasticsearch.xpack.esql.EsqlTestUtils.TWO; | ||
import static org.elasticsearch.xpack.esql.EsqlTestUtils.as; | ||
import static org.elasticsearch.xpack.esql.EsqlTestUtils.emptySource; | ||
import static org.elasticsearch.xpack.esql.EsqlTestUtils.fieldAttribute; | ||
import static org.elasticsearch.xpack.esql.EsqlTestUtils.getFieldAttribute; | ||
import static org.elasticsearch.xpack.esql.EsqlTestUtils.loadMapping; | ||
import static org.elasticsearch.xpack.esql.EsqlTestUtils.localSource; | ||
|
@@ -1294,6 +1298,26 @@ public void testCombineLimits() { | |
); | ||
} | ||
|
||
public void testPushdownLimitsPastLeftJoin() { | ||
var leftChild = emptySource(); | ||
var rightChild = new LocalRelation(Source.EMPTY, List.of(fieldAttribute()), LocalSupplier.EMPTY); | ||
assertNotEquals(leftChild, rightChild); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gotta be very careful here, you never know. |
||
|
||
var joinConfig = new JoinConfig(JoinTypes.LEFT, List.of(), List.of(), List.of()); | ||
var join = switch (randomIntBetween(0, 2)) { | ||
case 0 -> new Join(EMPTY, leftChild, rightChild, joinConfig); | ||
case 1 -> new LookupJoin(EMPTY, leftChild, rightChild, joinConfig); | ||
case 2 -> new InlineJoin(EMPTY, leftChild, rightChild, joinConfig); | ||
default -> throw new IllegalArgumentException(); | ||
}; | ||
|
||
var limit = new Limit(EMPTY, L(10), join); | ||
|
||
var optimizedPlan = new PushDownAndCombineLimits().rule(limit); | ||
|
||
assertEquals(join.replaceChildren(limit.replaceChild(join.left()), join.right()), optimizedPlan); | ||
} | ||
|
||
public void testMultipleCombineLimits() { | ||
var numberOfLimits = randomIntBetween(3, 10); | ||
var minimum = randomIntBetween(10, 99); | ||
|
Uh oh!
There was an error while loading. Please reload this page.