Yupsecous opened a new pull request, #83: URL: https://github.com/apache/datafusion-spark/pull/83
## Which issue does this PR close? - Addresses the float equality difference in #34. The untyped `NULL` argument difference mentioned in the comments there is not changed by this PR, so I left the issue open. ## Rationale for this change `array_contains` delegates the element comparison to `array_has`, which compares floating point values by their bits. Spark compares them with `-0.0 == 0.0` and `NaN == NaN`: `CodegenContext.genEqual` in Spark 4.2.0 generates `(java.lang.Double.isNaN(c1) && java.lang.Double.isNaN(c2)) || c1 == c2` (and the same for `Float`). So `array_contains(array(-0.0D), 0.0D)` returned `false` here and `true` in Spark, and two NaNs with different sign bits or payloads did not match, at any depth of list and struct elements. ## What changes are included in this PR? - `array_contains.rs` rewrites the float values of both arguments before calling `array_has`: `-0.0` becomes `0.0` and every NaN becomes the canonical NaN. It applies to scalar and array arguments, and to `Float32`/`Float64` at any depth of `List`, `LargeList`, `FixedSizeList` and `Struct`. Arrays that hold no floats are returned as they are. The existing null handling is unchanged. - `FixedSizeList` is rebuilt with `try_new_with_length`, so a zero-width list keeps its rows. - `array/array_contains.slt`: new queries for both zeros in both directions (`double` and `float`), NaN with a flipped sign bit, `LargeList` and `FixedSizeList` haystacks, nested lists and structs, null semantics, and a table with column arguments. - Unit tests in `array_contains.rs` for the normalization (scalar and nested shapes, zero-width lists, reuse of children without floats). An earlier attempt at this issue, apache/datafusion#26011, was closed before the move. Its review asked for a length-preserving rebuild of zero-width `FixedSizeList` and tests for it, which this PR includes. ## What is the testing strategy for this PR? - `cargo fmt --all -- --check`, `cargo clippy --all-targets --all-features -- -D warnings`, `cargo test -p datafusion-spark` (331 passed) and `cargo test -p datafusion-spark-sqllogictest` (245 of 245 files) pass. - With `array_contains.rs` from `main`, the new float queries fail. Run one by one, 9 of 14 probe queries fail on `main` (top-level `-0.0`, NaN with a flipped sign bit at every depth, `LargeList`/`FixedSizeList`); nested `-0.0` already compares equal on `main` and is kept as a guard. - Each branch of the new code is covered by at least one test that fails when the branch is removed (checked by hand: no normalization, NaN branch, zero branch for `f32` and `f64`, struct, list, scalar path, zero-width length, early return). ## Unknowns and assumptions - I could not run Spark here. The expected values come from Spark's source (`genEqual`, 4.2.0) and from the report in #34, not from a live PySpark session. - `arrays_overlap` (mentioned in #34) is not touched. ## Are there any user-facing changes? `array_contains` on float or double elements now matches Spark for `-0.0` and NaN. No API change. -- 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]
