zhangfengcdt opened a new issue, #3353: URL: https://github.com/apache/iceberg-rust/issues/3353
### Apache Iceberg Rust version main (ecba0b5) ### Describe the bug `Datum` equality and ordering disagree on NaN for `float` and `double`. `PartialEq` is derived from `PrimitiveLiteral`, which treats every NaN as the same value. `PartialOrd` compares floats with `total_cmp` (`iceberg_float_cmp_f32` / `iceberg_float_cmp_f64` in `datum.rs`), which orders NaNs by sign and payload. So two NaN datums can be `==` while `partial_cmp` returns `Some(Greater)` or `Some(Less)`. This breaks the `PartialEq` / `PartialOrd` consistency contract (`a == b` iff `partial_cmp == Some(Equal)`). Code that mixes `==` with `<`, `<=` or `partial_cmp` on `Datum` can get inconsistent answers for NaN. This predates #3327: `OrderedFloat` already made all NaNs equal. #3327 lines equality up with ordering for signed zero only. ### To Reproduce ```rust let a = Datum::float(f32::NAN); let b = Datum::float(-f32::NAN); let c = Datum::float(f32::from_bits(0x7fc0_0001)); // NaN with a different payload assert!(a == b); assert_eq!(a.partial_cmp(&b), Some(Ordering::Greater)); assert!(a == c); assert_eq!(a.partial_cmp(&c), Some(Ordering::Less)); let d = Datum::double(f64::NAN); assert!(d == Datum::double(-f64::NAN)); assert_eq!(d.partial_cmp(&Datum::double(-f64::NAN)), Some(Ordering::Greater)); ``` ### Expected behavior `==` and `partial_cmp` on `Datum` should agree for NaN. There are two orderings in play, so this needs a decision: - iceberg-java's literal comparator (`Comparators.forType`) uses `Float.compare` / `Double.compare`, where all NaNs are equal and greater than everything else. The spec's partition equality rule also normalizes NaNs. Following this, `Datum` ordering would treat all NaNs as one value, matching current equality. - The comment on `iceberg_float_cmp_f32` cites the sort order rule `-NaN < -Infinity < ... < Infinity < NaN`. If `Datum` ordering must keep that, equality would need to distinguish NaN sign instead. ### Willingness to contribute I can contribute a fix for this bug independently -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
