feat: eliminate LEFT/RIGHT JOINs with redundant sides - #23566
feat: eliminate LEFT/RIGHT JOINs with redundant sides#23566simonvandel wants to merge 8 commits into
Conversation
Extend the `EliminateJoin` rule to remove a LEFT JOIN entirely (replacing it with its left input) when: 1. none of the right side's columns are referenced above the join, and 2. the join cannot multiply left rows: either the right side is provably unique on the equi-join keys (PRIMARY KEY / UNIQUE constraints, GROUP BY, DISTINCT), or the join's ancestors are duplicate-insensitive. A LEFT JOIN preserves every left row whether or not it matches, so under these conditions the join has no observable effect. A join filter does not block the rewrite: for a left join it only decides whether a row is matched or null-padded, and either way the row is emitted. Such joins commonly appear in generated SQL and in queries over views that join in lookup tables the query does not read. This reuses the live-column tracking, duplicate-insensitivity context, and join-key uniqueness analysis that `EliminateJoin` already uses to rewrite inner joins to semi joins. Includes sqllogictest coverage for the eliminated and preserved cases, including result correctness for unmatched and duplicated rows.
github-actions
Bot
added
optimizer
neilconway
left a comment
neilconway
left a comment
There was a problem hiding this comment.
Thanks for working on this -- lgtm overall!
Nit: The PR description will be the git commit message when this is squash-merged, so it's confusing to talk about "first commit" vs "second commit" there.
The existing analysis has both unit tests and SLTs; can you take a look at whether any of the existing unit tests should be extended to cover outer joins?
Allows us to share the `can_remove_right` computation
simonvandel
changed the title
|
Thank you for your review @neilconway. I have responded to your comments, and adjusted the PR description to not mention individual commits.
It's true that eliminate_join.rs has no unit tests added in this PR, so in that sense there is a gap. But I have added SLT tests for the optimization which show positive/negative cases. Do you still want unit tests as well, even if that means we now test the code redundantly? |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #23566 +/- ##
==========================================
- Coverage 80.87% 80.87% -0.01%
==========================================
Files 1101 1101
Lines 375765 375790 +25
Branches 375765 375790 +25
==========================================
+ Hits 303915 303934 +19
- Misses 53747 53750 +3
- Partials 18103 18106 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@simonvandel Sorry for the delay in responding here!
It's probably fine to skip, although the existing If you can you fix the merge conflict and make a call on if you'd like to add unit tests, and I think we're good to land this. |
github-actions
Bot
added
documentation
93c7a0b to
f17cbfa
Compare
github-actions
Bot
removed
documentation
f17cbfa to
d2b56bc
Compare
|
@neilconway I have resolved the conflicts and decided to keep the tests as-is (coverage in SLT) |
4 participants
Which issue does this PR close?
Rationale for this change
Join elimination is useful in e.g. generated queries or views, where joined columns can end up not being used.
An explanation of when the optimization is valid, can be seen in the docs update for
EliminateJoin.What changes are included in this PR?
Builds upon the analysis that #22652 introduced for the
EliminateJoinoptimization pass. We use it to detect when the right-side of a left-join can be removed. Same symmetrical rule for right-join elimination.Are these changes tested?
Yes, SLT additions that test the feature end-to-end.
Are there any user-facing changes?
Yes, faster queries!