Skip to content

[SPARK-58384][SQL] Preserve null semantics in OptimizeJoinCondition - #57791

Open
MrHappyEnding wants to merge 1 commit into
apache:masterfrom
MrHappyEnding:SPARK-58384
Open

[SPARK-58384][SQL] Preserve null semantics in OptimizeJoinCondition#57791
MrHappyEnding wants to merge 1 commit into
apache:masterfrom
MrHappyEnding:SPARK-58384

Conversation

@MrHappyEnding

Copy link
Copy Markdown

What changes were proposed in this pull request?

Restrict OptimizeJoinCondition to rewriting null-safe equality patterns only at the root of a join condition or beneath AND/OR.

The rule no longer performs this rewrite beneath NOT, where the difference between NULL and FALSE is observable.

Why are the changes needed?

When exactly one operand is NULL, the original pattern returns NULL, while <=> returns FALSE.

This difference does not matter at the root of a join condition, but under NOT it can cause Spark to incorrectly keep extra rows.

Does this PR introduce any user-facing change?

Yes. Join conditions containing this pattern beneath NOT now return the correct rows.

For the SPARK-58384 reproduction, the result changes from four rows to only:

[0,10,1,22]

How was this patch tested?

Added tests covering:

  • No rewrite beneath NOT.
  • Continued rewriting beneath AND and OR.
  • The end-to-end join result with null values.

Ran:

build/sbt 'catalyst/testOnly org.apache.spark.sql.catalyst.optimizer.OptimizeJoinConditionSuite'
build/sbt 'sql/testOnly org.apache.spark.sql.DataFrameJoinSuite -- -z "SPARK-58384"'
build/sbt 'catalyst/scalastyle' 'catalyst/Test/scalastyle' 'sql/Test/scalastyle'

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Codex (GPT-5)

@uros-b

uros-b commented Aug 5, 2026

Copy link
Copy Markdown
Member

Thank you @MrHappyEnding! cc @cloud-fan who merged the original OptimizeJoinCondition rule

@cloud-fan cloud-fan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants