[andygrove]

Which issue does this PR close?

Part of #6385. This is the "consolidate the native helpers into the shared module (no behavior change)" step.

Rationale for this change

Spark's -0.0 and NaN rules were re-implemented in each native expression that needs one: three copies of NormalizeNaNAndZero, three of compareDoubles, two nested comparators, and six inline -0.0 checks in the hash macros. Putting each rule in one place means the later steps in #6385 each change one function, not every copy. Those steps are canonicalizing NaN in hash/xxhash64, normalizing comparisons, fixing min/max and friends, and nested ordering.

This PR does not change behavior. The fixes come in the follow-up PRs.

What changes are included in this PR?

A new native/spark-expr/src/float_semantics/ module. Its module docs list which Spark rule each helper implements.

Helper	Spark rule	Replaces
normalize_float	NormalizeNaNAndZero	math_funcs/internal/normalize_nan.rs, the closures in canonicalize_float_ordering (max_min_by.rs), the mode.rs macro
canonicalize_nan	java.lang.Double.equals	OverlapKey for f32/f64 (arrays_overlap.rs), mode before Spark 4.2, the NaN rule in the Parquet variant cast (variant.rs)
compare_floats	SQLOrderingUtil.compareDoubles	spark_double_cmp (percentile.rs), the inline check in position_float (array_position.rs)
float_lt, float_gt	compareDoubles, as single tests for hot loops	the inline checks in float_extrema (array_extrema.rs)
hash_input	Murmur3Hash / XxHash64 input	the six inline -0.0 checks in hash_funcs/utils.rs
normalize_floats	NormalizeNaNAndZero on a flat array	NormalizeNaNAndZero::normalize_array, normalize_floats (hll_plus_plus.rs), canonicalize_float_ordering (max_min_by.rs)
normalize_nested_floats, has_float_leaf, NormalizeNestedFloats	NormalizeNaNAndZero at any depth	moved from array_funcs/nested_float_normalize.rs
NormalizeNaNAndZero::wrap_if_needed		the check in create_normalized_key_expr (planner.rs)
spark_comparator, spark_equality	compareDoubles inside lists and structs	spark_comparator (array_extrema.rs) and nested_equality (nested_comparison.rs)

Notes for review:

• spark_comparator takes a left and a right array, which the nested ordering work (Comparison operators on arrays and structs with floating-point leaves do not match Spark for signed zero #6157, Match Spark ordering and rank semantics for floating values nested in arrays and structs #5507) needs. Nulls sort first at every level, as array_min/array_max require. spark_equality runs the same traversal for nested =/IN, and treats lists of different lengths as unequal without comparing their elements, as the old nested_equality did.
• hash_input keeps today's behavior: it folds -0.0 but does not canonicalize NaN. Its doc comment says so and points at [EPIC] Match Spark's -0.0 and NaN semantics systematically instead of per expression #6385, since the hash fix is the next step and now lands in this one function.
• percentile.rs, array_position.rs and float_extrema were not in the issue's list, but each held a copy of compareDoubles. The Parquet variant cast held a copy of the doubleToLongBits NaN rule.
• Left out because they change behavior: Map support in the nested normalization, and the hash NaN fix.
• float_semantics is a private module. The crate root re-exports normalize_floats and canonicalize_nan for core, plus NormalizeNaNAndZero and NormalizeNestedFloats as before. hash_array_primitive_float! drops its two type parameters, which only fed the inline -0.0 check.
• One difference is unreachable from Spark, which only produces List: the old array_min/array_max comparator fell back to Arrow's float order inside a LargeList or FixedSizeList, and the shared comparator uses Spark's order there too. The type-mismatch internal error, which is also unreachable, now reads "Spark comparison requires matching types". A dictionary array compared with a plain float array passes the logical type check. It now returns an internal error, where the old nested_equality panicked.

How are these changes tested?

The existing tests pass unchanged. These were run on macOS aarch64 with the default Spark 4.1 profile:

• datafusion-comet-spark-expr unit tests: 1022 passed. datafusion-comet unit tests: 552 passed, 5 ignored.
• CometSqlFileTestSuite: 578 passed. This includes the float fixtures for array_min/array_max, arrays_overlap, array_position, mode, max_by/min_by, approx_count_distinct, percentile, hash and nested IN.
• CometArrayExpressionSuite and CometHashExpressionSuite: 109 passed. CometWindowExecSuite and CometNativeShuffleSuite: 124 passed. These cover window partition keys and range partition bounds.

New unit tests:

• Each rule against Spark's order, including sign-bit and payload NaNs.
• The comparator at every nesting depth, with left and right arrays of different lengths and different values hidden under null list slots.
• normalize_floats leaving nested arrays unchanged.
• wrap_if_needed not wrapping a key twice.
• A percentile NaN-ordering test, replacing the test of the removed spark_double_cmp.
• The variant cast canonicalizing NaN and keeping the sign of zero. That path had no test before; the new test also passes on the old code.

I also checked equivalence with a throwaway differential test, not committed, that runs the old helpers verbatim against the new ones:

• The rules on edge values and 2,000 random bit patterns, for f32 and f64.
• The normalizers on random nested arrays: 17 shapes, sliced and unsliced, with byte-identical buffers.
• spark_comparator and spark_equality against the old nested_equality on 2.04M pairs, and spark_comparator against the old array_extrema comparator on 1.10M pairs.

I also planted six mutations in the new code, and the committed tests catch each one.

Performance matches the old code. I timed old and new copies in a standalone release build on an Apple M3 Max, on 8192-row batches:

• float_extrema compiles to the same instructions as before.
• array_position, percentile and nested =/IN are within noise of the old times, including lists of different lengths.
• The hash macros are faster, since each loop now has one hash call site.

[mbutrovich]

Thanks @andygrove. Having one place that names the Spark rule each helper follows will make the follow-ups in #6385 much easier to review.

I checked the performance claims on an Apple M5 Max (aarch64, release profile, rustc 1.98.1) against the merge base 8369bf1. The float compare blocks in float_extrema are the same instructions as before, and the float hash loops now use a branchless fcmp/fcsel into a single inlined hash where they used to branch between two hash sites. The float loop in array_position is now unswitched on whether the search value is NaN. The nested_comparison criterion bench for eq and dynamic at widths 1 and 16 moved between -12% and +6% with no consistent direction, which is within noise at that sample size.

[mbutrovich]

Thanks for the quick revision @andygrove, LGTM!

[andygrove]

Thanks for the review @mbutrovich