diff --git a/src/iceberg/expression/literal.cc b/src/iceberg/expression/literal.cc index d11ab2656..8d0e723c2 100644 --- a/src/iceberg/expression/literal.cc +++ b/src/iceberg/expression/literal.cc @@ -444,7 +444,9 @@ std::strong_ordering CompareFloat(T lhs, T rhs) { // and -NAN < NAN. bool lhs_is_negative = std::signbit(lhs); bool rhs_is_negative = std::signbit(rhs); - return lhs_is_negative <=> rhs_is_negative; + // A negative sign bit sorts below a positive one (-NaN < +NaN), so a + // negative operand must compare as less. + return rhs_is_negative <=> lhs_is_negative; } namespace { diff --git a/src/iceberg/test/literal_test.cc b/src/iceberg/test/literal_test.cc index 433c4fbed..bb0914b8d 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 @@ -216,6 +217,18 @@ TEST(LiteralTest, FloatNaNComparison) { EXPECT_EQ(nan1 <=> signaling_nan, std::partial_ordering::equivalent); } +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) { auto neg_inf = Literal::Float(-std::numeric_limits::infinity()); auto pos_inf = Literal::Float(std::numeric_limits::infinity()); @@ -267,6 +280,18 @@ TEST(LiteralTest, DoubleNaNComparison) { EXPECT_EQ(nan1 <=> signaling_nan, std::partial_ordering::equivalent); } +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) { auto neg_inf = Literal::Double(-std::numeric_limits::infinity()); auto pos_inf = Literal::Double(std::numeric_limits::infinity());