opt: Fix rule cycle bug#28848
Conversation
The PushFilterIntoJoinLeftAndRight and TryDecorrelateSelect rules can cycle with one another in a rare case: 1. Right side of join has outer column due to being un-decorrelatable. 2. Filter conjunct is pushed down to both left and right side by mapping equivalencies in PushFilterIntoJoinLeftAndRight. 3. Left conjunct is pulled back up to join condition by TryDecorrelateSelect. Steps cockroachdb#2 and cockroachdb#3 will cycle with one another. Cycle detection is not possible in this case, because the left side keeps changing (because new conjuct is pushed down to it each time). The fix is simple: don't let PushFilterIntoJoinLeftAndRight push down filters if either the left or right side has outer column(s). This fixes cockroachdb#28818. Release note: None
|
This change is |
RaduBerinde
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (and 1 stale)
rytaft
left a comment
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1.
Reviewable status:
pkg/sql/opt/norm/testdata/rules/join, line 961 at r1 (raw file):
# Regression for issue 28818. Try to trigger undetectable cycle between the # PushFilterIntoJoinLeftAndRight and TryDecorrelateSelect rules.
I'm confused - how does this trigger the rule PushFilterIntoJoinLeftAndRight? I don't see any variable equality conditions....
andy-kimball
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/norm/testdata/rules/join, line 961 at r1 (raw file):
Previously, rytaft wrote…
I'm confused - how does this trigger the rule
PushFilterIntoJoinLeftAndRight? I don't see any variable equality conditions....
Because (SELECT s FROM a) = 'foo' is effectively constant. It gets pushed down to both sides since it can be trivially "mapped" to either side.
rytaft
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/norm/testdata/rules/join, line 961 at r1 (raw file):
Previously, andy-kimball (Andy Kimball) wrote…
Because
(SELECT s FROM a) = 'foo'is effectively constant. It gets pushed down to both sides since it can be trivially "mapped" to either side.
got it - thanks!
|
bors r+ |
28340: storage: make lease rebalancing decisions at the store level r=a-robinson a=a-robinson In order to better balance the QPS being served by each store to avoid overloaded nodes. Fixes #21419 Release note (performance improvement): Range leases will be automatically rebalanced throughout the cluster to even out the amount of QPS being handled by each node. 28848: opt: Fix rule cycle bug r=andy-kimball a=andy-kimball The PushFilterIntoJoinLeftAndRight and TryDecorrelateSelect rules can cycle with one another in a rare case: 1. Right side of join has outer column due to being un-decorrelatable. 2. Filter conjunct is pushed down to both left and right side by mapping equivalencies in PushFilterIntoJoinLeftAndRight. 3. Left conjunct is pulled back up to join condition by TryDecorrelateSelect. Steps #2 and #3 will cycle with one another. Cycle detection is not possible in this case, because the left side keeps changing (because new conjuct is pushed down to it each time). The fix is simple: don't let PushFilterIntoJoinLeftAndRight push down filters if either the left or right side has outer column(s). This fixes #28818. Release note: None Co-authored-by: Alex Robinson <alexdwanerobinson@gmail.com> Co-authored-by: Andrew Kimball <andyk@cockroachlabs.com>
Build succeeded |
The PushFilterIntoJoinLeftAndRight and TryDecorrelateSelect rules can cycle
with one another in a rare case:
equivalencies in PushFilterIntoJoinLeftAndRight.
Steps #2 and #3 will cycle with one another. Cycle detection is not possible
in this case, because the left side keeps changing (because new conjuct is
pushed down to it each time).
The fix is simple: don't let PushFilterIntoJoinLeftAndRight push down filters
if either the left or right side has outer column(s).
This fixes #28818.
Release note: None