diff --git a/src/iceberg/expression/literal.cc b/src/iceberg/expression/literal.cc index d11ab2656..aeb51710b 100644 --- a/src/iceberg/expression/literal.cc +++ b/src/iceberg/expression/literal.cc @@ -430,21 +430,13 @@ Result Literal::CastTo(const std::shared_ptr& target_typ return LiteralCaster::CastTo(*this, target_type); } -// Template function for floating point comparison following Iceberg rules: -// -NaN < NaN, but all NaN values (qNaN, sNaN) are treated as equivalent within their sign +// Template function for floating point comparison following the Iceberg total +// ordering: -NaN < -Infinity < ... < +Infinity < +NaN. std::strong_order +// implements the IEEE 754 totalOrder predicate on IEC 559 types, which matches +// this requirement (and orders -0 below +0). template std::strong_ordering CompareFloat(T lhs, T rhs) { - // If both are NaN, check their signs - bool all_nan = std::isnan(lhs) && std::isnan(rhs); - if (!all_nan) { - // If not both NaN, use strong ordering - return std::strong_order(lhs, rhs); - } - // Same sign NaN values are equivalent (no qNaN vs sNaN distinction), - // and -NAN < NAN. - bool lhs_is_negative = std::signbit(lhs); - bool rhs_is_negative = std::signbit(rhs); - return lhs_is_negative <=> rhs_is_negative; + return std::strong_order(lhs, rhs); } namespace { diff --git a/src/iceberg/test/literal_test.cc b/src/iceberg/test/literal_test.cc index 433c4fbed..712887785 100644 --- a/src/iceberg/test/literal_test.cc +++ b/src/iceberg/test/literal_test.cc @@ -19,6 +19,7 @@ #include "iceberg/expression/literal.h" +#include #include #include #include @@ -211,9 +212,23 @@ TEST(LiteralTest, FloatNaNComparison) { auto nan2 = Literal::Float(std::numeric_limits::quiet_NaN()); auto signaling_nan = Literal::Float(std::numeric_limits::signaling_NaN()); - // NaN should be equal to itself in strong ordering + // Identical NaN bit patterns are equivalent under the total ordering. EXPECT_EQ(nan1 <=> nan2, std::partial_ordering::equivalent); - EXPECT_EQ(nan1 <=> signaling_nan, std::partial_ordering::equivalent); + // Total ordering distinguishes NaNs by bit pattern; a signaling NaN sorts + // below a quiet NaN of the same sign. + EXPECT_EQ(signaling_nan <=> nan1, std::partial_ordering::less); +} + +TEST(LiteralTest, FloatSignedNaNComparison) { + auto neg_nan = + Literal::Float(std::copysign(std::numeric_limits::quiet_NaN(), -1.0f)); + auto pos_nan = + Literal::Float(std::copysign(std::numeric_limits::quiet_NaN(), +1.0f)); + + // Per the total ordering -NaN < ... < +NaN, a negative NaN sorts below a + // positive NaN. + EXPECT_EQ(neg_nan <=> pos_nan, std::partial_ordering::less); + EXPECT_EQ(pos_nan <=> neg_nan, std::partial_ordering::greater); } TEST(LiteralTest, FloatInfinityComparison) { @@ -262,9 +277,23 @@ TEST(LiteralTest, DoubleNaNComparison) { auto nan2 = Literal::Double(std::numeric_limits::quiet_NaN()); auto signaling_nan = Literal::Double(std::numeric_limits::signaling_NaN()); - // NaN should be equal to itself in strong ordering + // Identical NaN bit patterns are equivalent under the total ordering. EXPECT_EQ(nan1 <=> nan2, std::partial_ordering::equivalent); - EXPECT_EQ(nan1 <=> signaling_nan, std::partial_ordering::equivalent); + // Total ordering distinguishes NaNs by bit pattern; a signaling NaN sorts + // below a quiet NaN of the same sign. + EXPECT_EQ(signaling_nan <=> nan1, std::partial_ordering::less); +} + +TEST(LiteralTest, DoubleSignedNaNComparison) { + auto neg_nan = + Literal::Double(std::copysign(std::numeric_limits::quiet_NaN(), -1.0)); + auto pos_nan = + Literal::Double(std::copysign(std::numeric_limits::quiet_NaN(), +1.0)); + + // Per the total ordering -NaN < ... < +NaN, a negative NaN sorts below a + // positive NaN. + EXPECT_EQ(neg_nan <=> pos_nan, std::partial_ordering::less); + EXPECT_EQ(pos_nan <=> neg_nan, std::partial_ordering::greater); } TEST(LiteralTest, DoubleInfinityComparison) {