Skip to content

[SPARK-58365][SQL] Build the NOT-IN-in-disjunction join condition from the deduplicated subquery output - #57558

Open
LuciferYang wants to merge 1 commit into
apache:masterfrom
LuciferYang:SPARK-58365-notin-dedup
Open

[SPARK-58365][SQL] Build the NOT-IN-in-disjunction join condition from the deduplicated subquery output#57558
LuciferYang wants to merge 1 commit into
apache:masterfrom
LuciferYang:SPARK-58365-notin-dedup

Conversation

@LuciferYang

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

In RewritePredicateSubquery.rewriteExistentialExprWithAttrs, the Not(InSubquery(...)) branch (the one handling a NOT IN nested inside a disjunction, e.g. v > 0 OR x NOT IN (...)) calls dedupSubqueryOnSelfJoin to alias the subquery's attributes when they conflict with the outer plan, but then builds the IN equality conditions from the pre-dedup sub.output instead of the deduplicated newSub.output. The join's right child uses newSub, so the condition can reference attributes that are no longer on the right side. This changes sub.output to newSub.output, matching the three sibling branches that already do this (the plain InSubquery branch in the same method, and both the top-level IN and NOT IN branches in apply).

Why are the changes needed?

When dedupSubqueryOnSelfJoin fires, it rebinds the conflicting subquery attributes to fresh exprIds. Building the condition from sub.output then uses the stale ids, which only exist on the outer side, so the null-aware anti-join condition collapses to trivially-true self-equalities like id#2 = id#2 and no longer references the join's right child. That is exactly the SPARK-26078 defect the dedupSubqueryOnSelfJoin call is there to prevent, so today that call is dead weight on this branch. Analysis-time DeduplicateRelations currently renews subquery exprIds before the optimizer runs, so this is not reachable from user SQL on current master and produces no wrong results today. It is a latent correctness hole: any future change that lets an outer/subquery exprId conflict reach this rule would silently return wrong NOT IN results, and the branch is the odd one out among four otherwise-consistent sites.

Does this PR introduce any user-facing change?

No. When dedup does not fire, newSub is the same object as sub, so the change is a no-op on every plan reachable from user SQL today.

How was this patch tested?

Added a RewriteSubquerySuite case that builds the colliding-attribute plan directly (bypassing the analyzer's DeduplicateRelations, which would otherwise renew the ids) and asserts the rewritten join condition references the deduplicated right-side output. It fails on the unfixed tree (the condition is (a#0 = a#0) OR isnull((a#0 = a#0)), referencing nothing on the right) and passes with the fix.

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

Generated-by: Claude Code (Opus 4.8)

…m the deduplicated subquery output

`RewritePredicateSubquery.rewriteExistentialExprWithAttrs`'s `Not(InSubquery(...))`
branch calls `dedupSubqueryOnSelfJoin` but then zips the IN equality conditions
against the pre-dedup `sub.output` instead of `newSub.output`. When dedup fires the
condition references stale exprIds absent from the join's right child. Use
`newSub.output`, matching the three sibling branches.
@uros-b

uros-b commented Jul 27, 2026

Copy link
Copy Markdown
Member

Thank you @LuciferYang!

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.

2 participants