alamb opened a new pull request, #27202: URL: https://github.com/apache/datafusion/pull/27202
## Which issue does this PR close? - Part of #27121 - Split out of #27181, see https://github.com/apache/datafusion/pull/27181#issuecomment-6703921224 ## Rationale for this change Every comparison, hash and grouping boundary normalizes `-0.0` to `+0.0` via `float_semantics::normalize_floats(&ArrayRef)`. Because it borrows the array, any array that contains a `-0.0` is always copied into a new buffer, even when the caller is done with the input (for example a freshly evaluated expression). In those cases the rewrite can be done in place. `InListExpr` (#27181) and `=` (`datum.rs`) also both need "normalize whichever side of a `ColumnarValue` this is", and `datum.rs` had a private helper for it. Putting that on `ColumnarValue` lets the two share it. ## What changes are included in this PR? - Add `float_semantics::normalize(ArrayRef) -> ArrayRef`. When nothing needs normalizing the input is returned as is. When a primitive float array holds the only reference to its values buffer, `-0.0` is rewritten in place via `unary_mut`; otherwise it falls back to copying, as before. - Deprecate `normalize_floats(&ArrayRef)`, which now forwards to `normalize(Arc::clone(array))`. - Update all callers to `float_semantics::normalize`. Callers that own their array (as-of join match keys, piecewise merge join extreme, `datum.rs` nested comparison) pass it directly; the rest `Arc::clone`, which keeps their behavior identical. - Add `ColumnarValue::normalize`, which applies `float_semantics::normalize` to an array and `ScalarValue::normalize` to a scalar, and use it in `apply_cmp` in place of the private `normalize_cmp_input` helper. - Add a 57.0.0 upgrade guide entry. ## What is the testing strategy for this PR? - `normalize_floats_rewrites_in_place_when_unique` in `float_semantics.rs` checks that a uniquely owned buffer is reused (same values pointer), that a shared buffer is copied and the original left untouched, and the existing tests cover the "nothing to do" and nested paths. - `normalize_rewrites_negative_zero` in `columnar_value.rs` covers `ColumnarValue::normalize` for arrays, scalars and non-float values. - Existing unit tests and sqllogictests for `=`, grouping, joins and `array_union` / `array_intersect` cover the updated callers. ## Are there any user-facing changes? `float_semantics::normalize_floats` is deprecated in favor of `float_semantics::normalize`, and `ColumnarValue::normalize` is new. Documented in the 57.0.0 upgrade guide. -- 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]
