LuciferYang opened a new pull request, #57558:
URL: https://github.com/apache/spark/pull/57558

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to