shinzoxD opened a new pull request, #24499:
URL: https://github.com/apache/datafusion/pull/24499

   ## Which issue does this PR close?
   
   - Closes #24431
   
   ## Rationale for this change
   
   `ScalarValue::eq_array` is documented as an optimized equivalent of 
extracting an array element with `try_from_array` and comparing the resulting 
scalars. For floating-point values this was not true: `eq_array` used IEEE 
equality while `ScalarValue::PartialEq` compares bit patterns.
   
   That produced inconsistent results for hash-table key comparisons and any 
other caller of `eq_array`:
   
   - Identical NaN bit patterns compared equal as `ScalarValue`s but unequal 
through `eq_array`.
   - `+0.0` and `-0.0` compared unequal as `ScalarValue`s but equal through 
`eq_array`.
   
   Changing `PartialEq` to IEEE equality would break `Eq` reflexivity for NaNs 
and require coordinated hash/order changes. The correct fix is for `eq_array` 
to match the existing `PartialEq` contract.
   
   ## What changes are included in this PR?
   
   - Compare `Float16`, `Float32`, and `Float64` in `eq_array` using 
`to_bits()`, matching `ScalarValue::PartialEq`.
   - Document the bit-pattern equality semantics on `eq_array`.
   - Add unit tests for NaN payloads and signed zeros on all three float widths.
   
   ## Are these changes tested?
   
   Yes. `test_eq_array_float_nan_and_signed_zero` checks that `eq_array` 
matches `try_from_array` + `PartialEq` for Float16/32/64, including:
   
   - identical NaN bits are equal
   - distinct NaN payloads remain unequal
   - `+0.0` and `-0.0` remain distinct
   - nulls and ordinary values still compare correctly
   
   Without the `to_bits()` change the new test fails (identical NaN: `eq_array` 
false vs `PartialEq` true).
   
   ## Are there any user-facing changes?
   
   Yes. Callers of `ScalarValue::eq_array` on floating-point values now get the 
same results as `ScalarValue::eq`. This is a behavior fix for the documented 
API contract; `PartialEq` itself is unchanged.
   
   No documentation site updates are needed beyond the rustdoc note on 
`eq_array`.
   


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