[SPARK-58384][SQL] Preserve null semantics in OptimizeJoinCondition - #57791
[SPARK-58384][SQL] Preserve null semantics in OptimizeJoinCondition#57791MrHappyEnding wants to merge 1 commit into
Conversation
|
Thank you @MrHappyEnding! cc @cloud-fan who merged the original |
cloud-fan
left a comment
There was a problem hiding this comment.
0 blocking, 1 non-blocking, 0 nits.
The semantic fix and its tests are sound; one non-blocking optimizer-allocation regression should be tightened.
Suggestions (1)
- sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/OptimizeJoinCondition.scala:48: Preserve tree-pattern pruning and unchanged Boolean nodes so unrelated join conditions are not traversed and rebuilt. -- see inline
Verification
I traced all truth-value cases for the rewrite. Both forms agree for equal values, unequal non-null values, and two nulls; with exactly one null, the rewrite changes NULL to FALSE. At a join root and through AND/OR this cannot create a new TRUE result, and the helper's catch-all prevents the unsafe propagation beneath NOT.
| || (l.semanticEquals(c2) && r.semanticEquals(c1)) => | ||
| EqualNullSafe(l, r) | ||
| case And(left, right) => | ||
| And(optimizeCondition(left), optimizeCondition(right)) |
There was a problem hiding this comment.
Please keep the OR tree-pattern guard and return the original AND/OR node when neither child changes. Without those checks, this rule now traverses and allocates a fresh Boolean tree for every unrelated join condition that the previous transformWithPruning skipped.
What changes were proposed in this pull request?
Restrict
OptimizeJoinConditionto rewriting null-safe equality patterns only at the root of a join condition or beneathAND/OR.The rule no longer performs this rewrite beneath
NOT, where the difference betweenNULLandFALSEis observable.Why are the changes needed?
When exactly one operand is
NULL, the original pattern returnsNULL, while<=>returnsFALSE.This difference does not matter at the root of a join condition, but under
NOTit can cause Spark to incorrectly keep extra rows.Does this PR introduce any user-facing change?
Yes. Join conditions containing this pattern beneath
NOTnow return the correct rows.For the SPARK-58384 reproduction, the result changes from four rows to only:
How was this patch tested?
Added tests covering:
NOT.ANDandOR.Ran:
Was this patch authored or co-authored using generative AI tooling?
Generated-by: Codex (GPT-5)