ORCA: don't push LOJ ON-pred onto its own outer in PushThruOuterChild#1836
Open
yjhjstz wants to merge 1 commit into
Open
ORCA: don't push LOJ ON-pred onto its own outer in PushThruOuterChild#1836yjhjstz wants to merge 1 commit into
yjhjstz wants to merge 1 commit into
Conversation
When CNormalizer::PushThruOuterChild is invoked from PushThruSelect with a predicate that happens to be (or contain) the LOJ's own ON predicate, the SplitConjunct / FPushable structural check would classify it as pushable to the outer relation -- because FPushable only verifies that the predicate's columns are a subset of the outer's output columns, and an LOJ ON-pred that references only outer-side columns trivially satisfies that. The consequence: the ON-pred is duplicated as a scan filter on the outer relation, which silently violates LOJ semantics. Outer rows that don't satisfy the ON-pred (and would have been null-padded in the result) get discarded at the scan, so a downstream "WHERE inner.col IS NULL" can no longer match them. Trigger pattern: two or more chained LEFT JOINs whose ON-clauses use the same boolean column from the outer relation, with a WHERE on top. Fix: at the entry of PushThruOuterChild, strip from the incoming conjunct any conjunct that structurally matches a conjunct of the LOJ's own ON predicate before handing it to SplitConjunct. The stripped conjunct is already enforced by the join itself for matching rows and must not become a filter on the outer for non-matching rows. Add a minimal regression case to bfv_joins covering the two-LOJ-on-same- boolean-column pattern with a WHERE on the inner side.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix a wrong-result bug in ORCA where a LEFT JOIN's own ON predicate ends up duplicated as a scan filter on the join's outer relation, discarding outer rows
that LOJ semantics require to be null-padded.
Repro
Seq Scan on xFilter: c1❌Trigger: two or more chained LEFT JOINs whose ON-clauses use the same boolean column from the outer relation, with a
WHEREon top.Root cause
CNormalizer::PushThruOuterChildis invoked fromPushThruSelectwith a predicate that happens to be (or contain) the LOJ's own ON predicate.SplitConjunct/FPushableaccept it as pushable to the outer relation, becauseFPushableonly checks that the predicate's columns are a subset of theouter's output columns — an LOJ ON-pred that references only outer-side columns trivially satisfies that.
Consequence: ORCA wraps the outer with
Select(outer, on_pred), producingLOJ(Select(x, c1), inner, x.c1). The LOJ's ON-pred is preserved asJoin Filter, but a redundantFilter: c1is also planted on the outer scan, which discards outer rows that don't satisfy the ON-pred — exactly the rows LOJmust null-pad and keep.
Fixes #ISSUE_Number
What does this PR do?
Type of Change
Breaking Changes
Test Plan
make installcheckmake -C src/test installcheck-cbdb-parallelImpact
Performance:
User-facing changes:
Dependencies:
Checklist
Additional Context
CI Skip Instructions