[mkleen]

Which issue does this PR close?

• Part of [EPIC] Handle -0.0 (and NaN) float keys in one place instead of per call site #26138 bringing the existing 0.0/-0.0 float handling into one place.

Rationale for this change

DataFusion treats -0.0 and 0.0 as the same number #22835. The code that makes this work is spread across multiple places. This PR moves the existing code into one module without changing its semantics.

The new module

DataFusion Comet has a similar module, native/spark-expr/src/float_semantics, which puts Spark's -0.0/NaN rules in one place. This PR follows its naming. The public API is a set of free functions:

/// `-0.0` becomes `0.0`; everything else, including NaN payloads, is kept.
pub fn normalize_float<T: FloatSemantics>(v: T) -> T;

/// The order DataFusion sorts floats in (IEEE 754 totalOrder, same as Arrow).
pub fn compare_floats<T: FloatSemantics>(left: T, right: T) -> Ordering;
pub fn float_lt<T: FloatSemantics>(left: T, right: T) -> bool;
pub fn float_gt<T: FloatSemantics>(left: T, right: T) -> bool;

/// The value hashed in place of a float.
pub fn hash_input<T: FloatSemantics>(v: T) -> T;

/// `normalize_float` applied to every float in an array (including nested ones) or a scalar.
pub fn normalize_floats(array: &ArrayRef) -> ArrayRef;
pub fn normalize_float_scalar(scalar: ScalarValue) -> ScalarValue;

pub fn has_float_leaf(data_type: &DataType) -> bool;

Comet's functions are generic over num::Float. DataFusion also has to cover f16, and half is built without its num-traits feature, so a small trait, FloatSemantics, serves as the bound. It is implemented for f16, f32 and f64:

pub trait FloatSemantics: Copy {
    /// Whether `normalize_float` would change this value.
    fn needs_normalization(self) -> bool;
    fn normalize(self) -> Self;
    fn compare(self, other: Self) -> Ordering;
}

There are two major differences from Comet. normalize_floats also reaches floats inside nested types, where Comet has a separate normalize_nested_floats. And normalization here only folds -0.0; NaN canonicalization and SQL ordering are not part of this PR. Once those are added, Comet's layout (normalize, compare and kernels submodules) is a good template for growing the module.

What changes are included in this PR?

A new module, datafusion_common::float_semantics, now decides when two floats count as the same value and how floats are ordered. All the following parts now use this module:

• hashing (hash joins/grouping)
• GROUP BY, DISTINCT and the array_* set functions
• comparisons like = and IS DISTINCT FROM
• the join types that compare keys
• min, max, ORDER BY ... LIMIT (TopK) and range repartitioning

What is the testing strategy for this PR?

• Query results don't change, so the existing tests are the main check.
• unit tests for the new module.

Are there any user-facing changes?

• Added datafusion_common::float_semantics with the functions above and the FloatSemantics trait.
• normalize_float_zero and normalize_float_zero_scalar in datafusion_common::utils are deprecated in favor of normalize_floats and normalize_float_scalar.
• No change in query behaviour.

LLM-generated code disclosure

This PR includes LLM-generated code and comments. All LLM-generated content has been manually reviewed.

[github-actions]

Thank you for opening this pull request!

Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch).

Details

     Cloning ebf48581898530f01d36e5580a29c8b8df3012de
    Building datafusion-common v55.1.0 (current)
       Built [  39.693s] (current)
     Parsing datafusion-common v55.1.0 (current)
      Parsed [   0.078s] (current)
    Building datafusion-common v55.1.0 (baseline)
       Built [   1.204s] (baseline)
     Parsing datafusion-common v55.1.0 (baseline)
      Parsed [   0.078s] (baseline)
    Checking datafusion-common v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.699s] 229 checks: 228 pass, 1 fail, 0 warn, 31 skip

--- failure function_marked_deprecated: function #[deprecated] added ---

Description:
A function is now #[deprecated]. Downstream crates will get a compiler warning when using this function.
        ref: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-deprecated-attribute
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.51.0/src/lints/function_marked_deprecated.ron

Failed in:
  function datafusion_common::utils::normalize_float_zero_scalar in /home/runner/work/datafusion/datafusion/datafusion/common/src/utils/mod.rs:1455
  function datafusion_common::utils::normalize_float_zero in /home/runner/work/datafusion/datafusion/datafusion/common/src/utils/mod.rs:1443

     Summary semver requires new minor version: 0 major and 1 minor checks failed
    Finished [  43.239s] datafusion-common
    Building datafusion-functions-aggregate v55.1.0 (current)
       Built [  23.036s] (current)
     Parsing datafusion-functions-aggregate v55.1.0 (current)
      Parsed [   0.057s] (current)
    Building datafusion-functions-aggregate v55.1.0 (baseline)
       Built [   7.714s] (baseline)
     Parsing datafusion-functions-aggregate v55.1.0 (baseline)
      Parsed [   0.058s] (baseline)
    Checking datafusion-functions-aggregate v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.188s] 229 checks: 229 pass, 31 skip
     Summary no semver update required
    Finished [  32.086s] datafusion-functions-aggregate
    Building datafusion-functions-aggregate-common v55.1.0 (current)
       Built [  13.623s] (current)
     Parsing datafusion-functions-aggregate-common v55.1.0 (current)
      Parsed [   0.018s] (current)
    Building datafusion-functions-aggregate-common v55.1.0 (baseline)
       Built [   3.206s] (baseline)
     Parsing datafusion-functions-aggregate-common v55.1.0 (baseline)
      Parsed [   0.019s] (baseline)
    Checking datafusion-functions-aggregate-common v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.155s] 229 checks: 229 pass, 31 skip
     Summary no semver update required
    Finished [  17.746s] datafusion-functions-aggregate-common
    Building datafusion-functions-nested v55.1.0 (current)
       Built [   9.067s] (current)
     Parsing datafusion-functions-nested v55.1.0 (current)
      Parsed [   0.042s] (current)
    Building datafusion-functions-nested v55.1.0 (baseline)
       Built [   9.084s] (baseline)
     Parsing datafusion-functions-nested v55.1.0 (baseline)
      Parsed [   0.043s] (baseline)
    Checking datafusion-functions-nested v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.217s] 229 checks: 229 pass, 31 skip
     Summary no semver update required
    Finished [  19.450s] datafusion-functions-nested
    Building datafusion-optimizer v55.1.0 (current)
       Built [  11.272s] (current)
     Parsing datafusion-optimizer v55.1.0 (current)
      Parsed [   0.036s] (current)
    Building datafusion-optimizer v55.1.0 (baseline)
       Built [   7.525s] (baseline)
     Parsing datafusion-optimizer v55.1.0 (baseline)
      Parsed [   0.038s] (baseline)
    Checking datafusion-optimizer v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.170s] 229 checks: 229 pass, 31 skip
     Summary no semver update required
    Finished [  19.822s] datafusion-optimizer
    Building datafusion-physical-expr v55.1.0 (current)
       Built [  12.460s] (current)
     Parsing datafusion-physical-expr v55.1.0 (current)
      Parsed [   0.056s] (current)
    Building datafusion-physical-expr v55.1.0 (baseline)
       Built [   8.631s] (baseline)
     Parsing datafusion-physical-expr v55.1.0 (baseline)
      Parsed [   0.060s] (baseline)
    Checking datafusion-physical-expr v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.351s] 229 checks: 229 pass, 31 skip
     Summary no semver update required
    Finished [  22.400s] datafusion-physical-expr
    Building datafusion-physical-expr-common v55.1.0 (current)
       Built [   5.401s] (current)
     Parsing datafusion-physical-expr-common v55.1.0 (current)
      Parsed [   0.022s] (current)
    Building datafusion-physical-expr-common v55.1.0 (baseline)
       Built [   5.393s] (baseline)
     Parsing datafusion-physical-expr-common v55.1.0 (baseline)
      Parsed [   0.023s] (baseline)
    Checking datafusion-physical-expr-common v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.237s] 229 checks: 229 pass, 31 skip
     Summary no semver update required
    Finished [  11.979s] datafusion-physical-expr-common
    Building datafusion-physical-plan v55.1.0 (current)
       Built [  14.229s] (current)
     Parsing datafusion-physical-plan v55.1.0 (current)
      Parsed [   0.230s] (current)
    Building datafusion-physical-plan v55.1.0 (baseline)
       Built [  13.441s] (baseline)
     Parsing datafusion-physical-plan v55.1.0 (baseline)
      Parsed [   0.233s] (baseline)
    Checking datafusion-physical-plan v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.673s] 229 checks: 229 pass, 31 skip
     Summary no semver update required
    Finished [  30.035s] datafusion-physical-plan

[mkleen]

@jayzhan211

[mkleen]

Btw, I have also some performance improvements in the pipeline, but i wanted to keep the scope of this pr small.

[jayzhan211]

@mkleen I compared each call site with the code it replaces, and they match bit for bit. Only -0.0 is folded, NaN payloads are kept, and compare_floats is still total_cmp, so min/max, TopK and range repartitioning order floats exactly as before. All of the #22835 call sites are covered. Clippy (also with force_hash_collisions), the unit tests of the touched crates, and the negative-zero, group-by, set-function and PWMJ sqllogictests all pass locally.

Since this touches hashing and grouping, I also ran a quick A/B against the merge base on an M4 Pro. Everything is within noise:

	before	this PR
hash 8192 f64 values	2.44 µs	2.51 µs
hash 8192 f32 values	2.46 µs	2.49 µs
hash 8192 f16 values	2.54 µs	2.48 µs
normalize f64, no -0.0	2.12 µs	2.14 µs
normalize f64, with -0.0	1.29 µs	1.29 µs
multi_group_by float16, vectorized, 60k groups	7.89 ms	7.95 ms
multi_group_by float16, row-based, 60k groups	7.56 ms	7.61 ms

For the perf work you mentioned (not something for this PR): when there is nothing to normalize, the pre-scan in normalize_primitive is the expensive part. any() exits early, so it doesn't vectorize; Comet ran into the same thing in apache/datafusion-comet#6590. In a standalone test, a reduction that doesn't short-circuit took the check on a clean 8192-value f64 array from about 2.1 µs to 0.5 µs:

-    if !arr.values().iter().any(|v| v.needs_normalization()) {
+    if !arr
+        .values()
+        .iter()
+        .fold(false, |acc, v| acc | v.needs_normalization())
+    {

I left one small doc suggestion inline.

[mkleen]

@jayzhan211 wow, thanks for the fast review and the great feedback!