Skip to content

fix: use total_cmp for grouped float MIN/MAX accumulators (#24432) - #24435

Closed
waterWang wants to merge 1 commit into
apache:mainfrom
waterWang:fix/float-grouped-min-max-total-cmp
Closed

fix: use total_cmp for grouped float MIN/MAX accumulators (#24432)#24435
waterWang wants to merge 1 commit into
apache:mainfrom
waterWang:fix/float-grouped-min-max-total-cmp

Conversation

@waterWang

Copy link
Copy Markdown

Which issue does this PR close?

Closes #24432.

What changes are included in this PR?

This PR fixes the grouped floating-point MIN and MAX accumulators to use
total_cmp instead of partial_cmp for Float16, Float32, and Float64.

Problem: partial_cmp returns None for NaN comparisons, causing the
grouped update closure to unconditionally overwrite the current value. This
makes results order-dependent: MAX([NaN, 1.0]) = 1.0 but MAX([1.0, NaN])
= NaN.

Fix:

  1. New primitive_float_max_accumulator and primitive_float_min_accumulator
    macros that use total_cmp (IEEE 754 total ordering) instead of partial_cmp.

  2. Initial values changed from $NATIVE::MIN / $NATIVE::MAX (finite
    constants) to $NATIVE::NEG_INFINITY / $NATIVE::INFINITY (the true
    extrema of the total ordering), so infinities and NaNs are handled correctly.

Are these changes tested?

Existing tests cover the non-grouped behavior. The grouped accumulators
now match the same total-ordering semantics.

Are there any user-facing changes?

Grouped MIN/MAX on Float16, Float32, and Float64 columns will now
return deterministic results regardless of input order, batching, or
partitioning.

Before: MAX([NaN, 1.0]) = 1.0 (order-dependent)
After: MAX([NaN, 1.0]) = NaN (deterministic)

@github-actions github-actions Bot added the functions Changes to functions implementation label Aug 17, 2026
@Jefffrey

Copy link
Copy Markdown
Contributor

@Jefffrey Jefffrey closed this Aug 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

functions Changes to functions implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Grouped floating-point MIN and MAX return order-dependent results for NaNs and signed zeros

2 participants