andygrove opened a new issue, #5270:
URL: https://github.com/apache/datafusion-comet/issues/5270
## Describe the bug
`arrays_overlap` over a flat `array<double>` or `array<float>` column
disagrees with Spark when either side holds a NaN whose bit pattern is not the
canonical positive NaN.
Comet's flat path keys its hash set on the raw bits:
```rust
// native/spark-expr/src/array_funcs/arrays_overlap.rs
impl OverlapKey for f64 {
type Key = u64;
fn overlap_key(self) -> u64 {
self.to_bits()
}
}
```
Spark's `ArraysOverlap` uses `fastEval` for atomic element types, which is a
`java.util.HashSet` over boxed values. `java.lang.Double.equals` compares
`doubleToLongBits`, and that collapses every NaN payload and sign onto one
pattern. So Spark treats `-NaN` and `NaN` as the same element and Comet does
not.
## Steps to reproduce
The negation has to happen at query time. A `-double('NaN')` literal is
canonicalized before it reaches Parquet, so putting one in the data does not
reproduce it.
```sql
statement
CREATE TABLE t_flat(x double) USING parquet
statement
INSERT INTO t_flat VALUES (double('NaN')), (0.0), (1.0)
query
SELECT arrays_overlap(array(-x), array(double('NaN'))) FROM t_flat
```
```
!== Spark Answer - 3 == == Comet Answer - 3 ==
[false] [false]
[false] [false]
![true] [false]
```
Reproduced on `apache/main` at `f262b13d2`, Spark 4.1 profile, JDK 17, macOS.
## What the fix looks like
`overlap_key` needs to canonicalize NaN before taking the bits. It must
**not** use `normalize_float` from `math_funcs/internal/normalize_nan.rs`,
because that also folds `-0.0` into `0.0`, and the flat path is required to
keep those distinct.
That last part is easy to get wrong, so spelling it out. Spark's flat and
nested paths genuinely differ on signed zero:
| | signed zero | NaN sign / payload |
|---|---|---|
| flat (`fastEval`, `HashSet` of boxed `Double`) | `-0.0` and `0.0` are
**distinct** | all NaN are **equal** |
| nested (`bruteForceEval`, `ordering.equiv` ->
`SQLOrderingUtil.compareDoubles`) | `-0.0` and `0.0` are **equal** | all NaN
are **equal** |
Verified directly against the JDK:
```
HashSet{0.0}.contains(-0.0) = false
HashSet{NaN}.contains(-NaN runtime) = true
compareDoubles(-0.0, 0.0) = 0
compareDoubles(-NaN, NaN) = 0
```
Comet's flat path already matches Spark on signed zero. Only the NaN column
is wrong. The nested path is being fixed for both in #5235.
## Additional context
`array_position` does not have this problem. `position_float` uses
`(search_is_nan && v.is_nan()) || v == search_val`, which matches
`compareDoubles` on both signed zero and NaN.
Found while reviewing #5235.
--
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]
