yadavay-amzn commented on PR #56244:
URL: https://github.com/apache/spark/pull/56244#issuecomment-4846468053

   Thanks @peter-toth - addressed all three:
   
   - Folded the EqualTo/EqualNullSafe arms into one `case Equality(l, r)` 
guarded by `canDecompose && filterEquivalentToConjunction`. Adding the guard to 
the `<=>` path is safe: `canDecompose` requires a foldable (hence non-nullable) 
operand, so both-nullable is unreachable and falls through to 
`decomposeEqualNullSafe`; the two-nullable-columns test confirms it.
   - Untested paths: you were right the Not/NULL e2e tests were vacuous - 
`withColumn("s", struct(...))` inlines to a non-cheap `CreateNamedStruct`, so 
`canDecompose` was false and the rule never fired. Replaced them with 
Parquet-backed tables where `s` is a nullable struct attribute, and added 
catalyst tests under `Not`/`Or` that actually reach `decomposeEqualTo`'s 
nullable `If(anyNull, NULL, conjunction)` branch and `decomposeEqualNullSafe`'s 
branches, asserting NULL-vs-FALSE parity across whole-null / all-null-fields / 
non-null rows (verified by breaking each branch).
   - Refreshed the PR description: scope narrowing (foldable + isCheap), the 
tuple-not-decomposed note, the IsNotNull-guarded pushable form, and the 
How-tested list.
   
   PTAL.


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