sunchao commented on PR #5472:
URL: 
https://github.com/apache/datafusion-comet/pull/5472#issuecomment-5442621941

   Updated in 
[4a151c4c7](https://github.com/apache/datafusion-comet/commit/4a151c4c706b7753f15dc6715db0b1c7c826b338).
   
   I addressed the membership issue in this PR rather than deferring it. `IN`, 
`InSet`, and fused `NOT IN` now normalize scalar FLOAT/DOUBLE values and list 
candidates. Literal normalization is evaluated during serialization, keeping 
literal lists scalar so DataFusion can still build its static membership 
filter. Non-floating expressions retain the existing path. This membership bug 
predates the original two-line NaN-literal fix; it was not introduced by it.
   
   The regressions preserve multiple candidates and pin the optimized 
expression to `In` or `InSet` by varying Spark's conversion threshold, so a 
singleton rewrite to equality cannot give a false pass. They cover both widths, 
NaN payloads/signs, both zero signs, null values/candidates, and negated 
membership in projections and filters. Unary negation creates negative NaNs 
after the Parquet scan to exercise normalization of the value as well as the 
list. Spark folds `NOT IN (..., NULL)` filters to an empty relation; those 
cases check the result without incorrectly requiring a native filter.
   
   I also expanded the original comparison matrix: all seven comparison forms, 
both operand orders, and both widths now check surviving row identities through 
`CometFilterExec` with Parquet pushdown disabled, alongside the Boolean 
projection assertions.
   
   I kept the scope to the confirmed scalar comparison and membership paths. 
The literal guards are still needed for ordinary comparisons; constant folding 
was an optional optimization there, while folding the membership literals is 
necessary to retain the static IN filter. Existing join/window/grouping 
normalization is governed by its own Spark/planner paths, so this is not 
evidence that every such operator was affected by the original literal 
shortcut. Nested floating-point ordering is separate work, not silently covered 
by this scalar fix.
   
   Validation: full Spark 4.1.3 JVM reactor, Spotless, and Scalastyle passed; 
all four expanded tests passed against the rebuilt JVM code. Restoring only the 
old predicate implementation makes both new membership tests fail with the 
expected Boolean result mismatch. Local runs reused the existing OSS native 
library, and no full native rebuild is claimed. Fresh CI remains pending.
   


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