liwuhen opened a new pull request, #588:
URL: https://github.com/apache/paimon-rust/pull/588
### Purpose
The `min` and `max` aggregators currently use Rust's `total_cmp` directly for
floating-point values.
Although `total_cmp` preserves the desired ordering of `-0.0` before `+0.0`,
it orders negative NaNs before numeric values. This differs from Java
`Float.compare` and `Double.compare`, where every NaN sorts after all numeric
values regardless of its sign bit or payload.
As a result, aggregating a negative NaN together with a finite value can
return
the wrong result:
- `min(negative_nan, 1.0)` can incorrectly return NaN.
- `max(negative_nan, 1.0)` can incorrectly return `1.0`.
- Two NaNs can be ordered by sign or payload and unnecessarily replace the
existing accumulator value.
This change handles NaN ordering locally in the floating-point `min` / `max`
comparison:
- Every NaN sorts after every non-NaN value.
- Two NaNs compare as equal, independent of sign or payload.
- Non-NaN values continue to use `total_cmp`, preserving Java's ordering of
`-0.0` before `+0.0`.
The comparison affects both `FLOAT` (`f32`) and `DOUBLE` (`f64`) aggregation
without canonicalizing or rewriting the stored NaN value.
### Brief change log
- Update the local floating-point comparison in `update_float!` to:
- Order positive and negative NaNs after all numeric values.
- Treat all NaN representations as equal for comparison.
- Continue using `total_cmp` for non-NaN values.
- Apply the behavior consistently to both `MinAgg` and `MaxAgg`.
- Add regression coverage for both `f32` and `f64`, including:
- Negative NaN and finite values.
- Positive NaN and finite values.
- A NaN already stored in the accumulator before a finite value arrives.
- `-0.0` and `+0.0`, asserted by bit pattern.
- Ordinary finite-value ordering.
- NaNs with different signs and payloads without accumulator replacement.
### Tests
- Unit test `test_float32_min_max_matches_java_ordering`
(`numeric.rs`): verifies Java-compatible ordering for `f32`, including
negative NaNs, positive NaNs, signed zero, finite values, and distinct NaN
representations. **Fails on the pre-fix code.**
- Unit test `test_float64_min_max_matches_java_ordering`
(`numeric.rs`): verifies the same behavior for `f64`.
**Fails on the pre-fix code.**
- Existing numeric aggregation tests verify that normal values and null
aggregation behavior remain unchanged.
- Commands run locally:
- `cargo test -p paimon --lib table::aggregator::numeric`
- `cargo test -p paimon --lib`
- `cargo fmt --all --check`
- `cargo check -p paimon --all-targets`
- `cargo clippy -p paimon --all-targets -- -D warnings`
- `git diff --check`
### API and Format
No public API or persisted data format changes.
The behavior changes only for floating-point `min` and `max` comparisons
involving NaN values. NaN values are not canonicalized or rewritten; only
their
comparison ordering is corrected to match Java `Float.compare` and
`Double.compare`.
Sum, product, decimal, integer, and PK Vector comparison behavior are
unchanged.
### Documentation
No documentation changes required.
--
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]
