This is an automated email from the ASF dual-hosted git repository.
liurenjie1024 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-rust.git
The following commit(s) were added to refs/heads/main by this push:
new 99ca196a7 fix: follow IEEE 754 totalOrder for `float` and `double`
(#1959)
99ca196a7 is described below
commit 99ca196a7a81343ac35d993d9db2ec59c626f389
Author: Alan Tang <[email protected]>
AuthorDate: Thu Dec 25 09:03:32 2025 +0800
fix: follow IEEE 754 totalOrder for `float` and `double` (#1959)
## Which issue does this PR close?
- Closes #1951.
## What changes are included in this PR?
For `F32` and `F64`, we use the `total_cmp` method for comparison.
## Are these changes tested?
Yes, I added test cases to verify whether the comparison follows the
IEEE 754 rules.
Signed-off-by: StandingMan <[email protected]>
Co-authored-by: Renjie Liu <[email protected]>
---
crates/iceberg/src/spec/values/datum.rs | 36 ++++++++-------------------------
crates/iceberg/src/spec/values/tests.rs | 25 +++++++++++++++++++++++
2 files changed, 33 insertions(+), 28 deletions(-)
diff --git a/crates/iceberg/src/spec/values/datum.rs
b/crates/iceberg/src/spec/values/datum.rs
index cb60fb94e..88209ae95 100644
--- a/crates/iceberg/src/spec/values/datum.rs
+++ b/crates/iceberg/src/spec/values/datum.rs
@@ -166,36 +166,16 @@ impl<'de> Deserialize<'de> for Datum {
// Compare following iceberg float ordering rules:
// -NaN < -Infinity < -value < -0 < 0 < value < Infinity < NaN
-fn iceberg_float_cmp<T: Float>(a: T, b: T) -> Option<Ordering> {
- if a.is_nan() && b.is_nan() {
- return match (a.is_sign_negative(), b.is_sign_negative()) {
- (true, false) => Some(Ordering::Less),
- (false, true) => Some(Ordering::Greater),
- _ => Some(Ordering::Equal),
- };
- }
-
- if a.is_nan() {
- return Some(if a.is_sign_negative() {
- Ordering::Less
- } else {
- Ordering::Greater
- });
- }
-
- if b.is_nan() {
- return Some(if b.is_sign_negative() {
- Ordering::Greater
- } else {
- Ordering::Less
- });
- }
+fn iceberg_float_cmp_f32(a: OrderedFloat<f32>, b: OrderedFloat<f32>) ->
Option<Ordering> {
+ Some(a.total_cmp(&b))
+}
- a.partial_cmp(&b)
+fn iceberg_float_cmp_f64(a: OrderedFloat<f64>, b: OrderedFloat<f64>) ->
Option<Ordering> {
+ Some(a.total_cmp(&b))
}
impl PartialOrd for Datum {
- fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+ fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
match (&self.literal, &other.literal, &self.r#type, &other.r#type) {
// generate the arm with same type and same literal
(
@@ -221,13 +201,13 @@ impl PartialOrd for Datum {
PrimitiveLiteral::Float(other_val),
PrimitiveType::Float,
PrimitiveType::Float,
- ) => iceberg_float_cmp(*val, *other_val),
+ ) => iceberg_float_cmp_f32(*val, *other_val),
(
PrimitiveLiteral::Double(val),
PrimitiveLiteral::Double(other_val),
PrimitiveType::Double,
PrimitiveType::Double,
- ) => iceberg_float_cmp(*val, *other_val),
+ ) => iceberg_float_cmp_f64(*val, *other_val),
(
PrimitiveLiteral::Int(val),
PrimitiveLiteral::Int(other_val),
diff --git a/crates/iceberg/src/spec/values/tests.rs
b/crates/iceberg/src/spec/values/tests.rs
index 73343a9a1..bb10701d8 100644
--- a/crates/iceberg/src/spec/values/tests.rs
+++ b/crates/iceberg/src/spec/values/tests.rs
@@ -1293,6 +1293,31 @@ fn test_iceberg_float_order() {
assert_eq!(double_sorted, double_expected);
}
+#[test]
+fn test_negative_zero_less_than_positive_zero() {
+ {
+ let neg_zero = Datum::float(-0.0);
+ let pos_zero = Datum::float(0.0);
+
+ assert_eq!(
+ neg_zero.partial_cmp(&pos_zero),
+ Some(std::cmp::Ordering::Less),
+ "IEEE 754 totalOrder requires -0.0 < +0.0 on F32"
+ );
+ }
+
+ {
+ let neg_zero = Datum::double(-0.0);
+ let pos_zero = Datum::double(0.0);
+
+ assert_eq!(
+ neg_zero.partial_cmp(&pos_zero),
+ Some(std::cmp::Ordering::Less),
+ "IEEE 754 totalOrder requires -0.0 < +0.0 on F64"
+ );
+ }
+}
+
/// Test Date deserialization from JSON as number (days since epoch).
///
/// This reproduces the scenario from Iceberg Java's TestAddFilesProcedure
where:
