liwuhen commented on code in PR #588:
URL: https://github.com/apache/paimon-rust/pull/588#discussion_r3635207854
##########
crates/paimon/src/table/aggregator/numeric.rs:
##########
@@ -438,12 +438,18 @@ fn agg_minmax(
($acc:expr, $ty:ty) => {{
let v = downcast::<$ty>(array, field_name)?.value(row_idx);
// Match Java `Float.compare` / `Double.compare`, which order NaN
- // greater than any other value (including +Infinity). Using
- // `total_cmp` makes that ordering explicit and deterministic.
+ // greater than any other value (including +Infinity) and compare
+ // all NaN representations as equal. For non-NaN values,
+ // `total_cmp` preserves Java's ordering of -0.0 before +0.0.
*$acc = Some(match *$acc {
None => v,
Some(prev) => {
- let cmp = v.total_cmp(&prev);
+ let cmp = match (v.is_nan(), prev.is_nan()) {
+ (true, true) => std::cmp::Ordering::Equal,
Review Comment:
Addressed in 25b8d33. Min now takes the new input on equal comparison, while
Max retains the existing accumulator, matching Java FieldMinAgg/FieldMaxAgg.
The f32 and f64 NaN bit-pattern tests now verify the distinct tie-breaking
behavior.
--
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]
