fix: canonicalize floating-point NaN values (#255)

Author: zjw1111

include/paimon/predicate/literal.h

@@ -91,13 +91,12 @@ class PAIMON_EXPORT Literal {
     std::string ToString() const;
 
     /// Gets the hash code for this literal.
-    /// @note HashCode() hashes the exact bit representation (including Decimal scale), while
-    /// operator== delegates to CompareTo() which uses numeric equality (e.g. decimals with
-    /// different scales can compare equal). This means the hash-equality contract (equal objects
-    /// must have equal hashes) may be violated for Decimal literals with different scales. In
-    /// practice this is safe because all current std::unordered_map<Literal, ...> usages (bitmap
-    /// file index) only store values from the same column, which guarantees a fixed precision and
-    /// scale.
+    /// @note HashCode() canonicalizes all floating-point NaNs so that values considered equal by
+    /// CompareTo() have the same hash. Decimal values include their scale in the hash, while
+    /// CompareTo() uses numeric equality, so Decimal literals with different scales can still
+    /// violate the hash-equality contract. In practice this is safe because all current
+    /// std::unordered_map<Literal, ...> usages only store values from the same column, which has a
+    /// fixed precision and scale.
     size_t HashCode() const;
 
     /// Compares this literal with another literal. The comparison follows SQL semantics for the

src/paimon/common/data/variant/generic_variant_test.cpp

@@ -19,6 +19,7 @@
 
 #include "paimon/common/data/variant/generic_variant.h"
 
+#include <cstdint>
 #include <functional>
 #include <limits>
 #include <string>
@@ -27,6 +28,7 @@
 #include "gtest/gtest.h"
 #include "paimon/common/data/variant/variant_builder.h"
 #include "paimon/common/data/variant/variant_defs.h"
+#include "paimon/common/utils/math.h"
 #include "paimon/memory/memory_pool.h"
 #include "paimon/testing/utils/testharness.h"
 
@@ -350,6 +352,23 @@ TEST_F(GenericVariantTest, NonFiniteDoubleToJson) {
     ASSERT_EQ(json, "\"Infinity\"");
 }
 
+TEST_F(GenericVariantTest, CanonicalizesFloatingPointNaN) {
+    {
+        VariantBuilder builder(false);
+        ASSERT_OK(builder.AppendFloat(FloatingPointFromBits<float>(0xffc12345U)));
+        ASSERT_OK_AND_ASSIGN(std::shared_ptr<GenericVariant> variant, builder.Build(pool_));
+        ASSERT_OK_AND_ASSIGN(std::string_view value, variant->Value());
+        ASSERT_EQ(ToHex(value), "380000c07f");
+    }
+    {
+        VariantBuilder builder(false);
+        ASSERT_OK(builder.AppendDouble(FloatingPointFromBits<double>(0xfff8123456789abcULL)));
+        ASSERT_OK_AND_ASSIGN(std::shared_ptr<GenericVariant> variant, builder.Build(pool_));
+        ASSERT_OK_AND_ASSIGN(std::string_view value, variant->Value());
+        ASSERT_EQ(ToHex(value), "1c000000000000f87f");
+    }
+}
+
 TEST_F(GenericVariantTest, GetTypeInfoReturnsHeaderBits) {
     // GetTypeInfo exposes the primitive header's type-info bits; 42 is encoded as an int1.
     auto v = FromJson("42");

src/paimon/common/data/variant/variant_builder.cpp

@@ -30,6 +30,7 @@
 
 #include "fmt/format.h"
 #include "paimon/common/data/variant/variant_defs.h"
+#include "paimon/common/utils/math.h"
 #include "rapidjson/error/en.h"
 #include "rapidjson/memorystream.h"
 #include "rapidjson/reader.h"
@@ -339,8 +340,7 @@ Status VariantBuilder::AppendLong(int64_t l) {
 Status VariantBuilder::AppendDouble(double d) {
     PAIMON_RETURN_NOT_OK(CheckCapacity(1 + 8));
     write_buffer_[write_pos_++] = VariantBinaryUtil::PrimitiveHeader(VariantDefs::kDouble);
-    int64_t bits;
-    memcpy(&bits, &d, sizeof(bits));
+    const int64_t bits = CanonicalizeDoubleToLongBits(d);
     VariantBinaryUtil::WriteLong(bits, 8, write_buffer_.data(), write_pos_);
     write_pos_ += 8;
     return Status::OK();
@@ -409,8 +409,7 @@ Status VariantBuilder::AppendTimestampNtz(int64_t micros_since_epoch) {
 Status VariantBuilder::AppendFloat(float f) {
     PAIMON_RETURN_NOT_OK(CheckCapacity(1 + 4));
     write_buffer_[write_pos_++] = VariantBinaryUtil::PrimitiveHeader(VariantDefs::kFloat);
-    int32_t bits;
-    memcpy(&bits, &f, sizeof(bits));
+    const int32_t bits = CanonicalizeFloatToIntBits(f);
     VariantBinaryUtil::WriteLong(bits, 4, write_buffer_.data(), write_pos_);
     write_pos_ += 4;
     return Status::OK();

src/paimon/common/file_index/bloomfilter/fast_hash.cpp

@@ -19,7 +19,6 @@
 #include "paimon/common/file_index/bloomfilter/fast_hash.h"
 
 #include <cassert>
-#include <cmath>
 #include <cstring>
 #include <string>
 #include <utility>
@@ -28,18 +27,14 @@
 #include "paimon/common/utils/checked_cast.h"
 #include "paimon/common/utils/date_time_utils.h"
 #include "paimon/common/utils/field_type_utils.h"
+#include "paimon/common/utils/math.h"
 #include "paimon/data/timestamp.h"
 #include "paimon/defs.h"
 #include "paimon/file_index/file_index_result.h"
 #include "paimon/status.h"
 #include "xxhash.h"  // NOLINT(build/include_subdir)
 
 namespace paimon {
-namespace {
-constexpr int32_t kCanonicalFloatNaNBits = 0x7fc00000;
-constexpr int64_t kCanonicalDoubleNaNBits = 0x7ff8000000000000L;
-}  // namespace
-
 Result<FastHash::HashFunction> FastHash::GetHashFunction(
     const std::shared_ptr<arrow::DataType>& arrow_type) {
     PAIMON_ASSIGN_OR_RAISE(FieldType field_type,
@@ -64,23 +59,11 @@ Result<FastHash::HashFunction> FastHash::GetHashFunction(
             });
         case FieldType::FLOAT:
             return HashFunction([](const Literal& literal) -> int64_t {
-                const auto raw_value = literal.GetValue<float>();
-                if (std::isnan(raw_value)) {
-                    return GetLongHash(kCanonicalFloatNaNBits);
-                }
-                int32_t bits = 0;
-                std::memcpy(&bits, &raw_value, sizeof(raw_value));
-                return GetLongHash(bits);
+                return GetLongHash(CanonicalizeFloatToIntBits(literal.GetValue<float>()));
             });
         case FieldType::DOUBLE:
             return HashFunction([](const Literal& literal) -> int64_t {
-                const auto raw_value = literal.GetValue<double>();
-                if (std::isnan(raw_value)) {
-                    return GetLongHash(kCanonicalDoubleNaNBits);
-                }
-                int64_t bits;
-                std::memcpy(&bits, &raw_value, sizeof(raw_value));
-                return GetLongHash(bits);
+                return GetLongHash(CanonicalizeDoubleToLongBits(literal.GetValue<double>()));
             });
         case FieldType::TIMESTAMP: {
             auto ts_type = checked_pointer_cast<arrow::TimestampType>(arrow_type);

src/paimon/common/file_index/bloomfilter/fast_hash_test.cpp

@@ -20,12 +20,12 @@
 
 #include <cmath>
 #include <cstdint>
-#include <cstring>
 #include <limits>
 #include <string>
 #include <vector>
 
 #include "gtest/gtest.h"
+#include "paimon/common/utils/math.h"
 #include "paimon/data/timestamp.h"
 #include "paimon/defs.h"
 #include "paimon/file_index/file_index_result.h"
@@ -168,26 +168,16 @@ TEST_F(FastHashTest, TestCompatibleWithJava) {
 }
 
 TEST_F(FastHashTest, TestNaNCompatibleWithJava) {
-    auto float_from_bits = [](uint32_t bits) {
-        float value;
-        std::memcpy(&value, &bits, sizeof(value));
-        return value;
-    };
-    const float float_nan = float_from_bits(0x7fc12345);
-    const float negative_float_nan = float_from_bits(0xffc54321);
+    const auto float_nan = FloatingPointFromBits<float>(0x7fc12345U);
+    const auto negative_float_nan = FloatingPointFromBits<float>(0xffc54321U);
     ASSERT_TRUE(std::isnan(float_nan));
     ASSERT_TRUE(std::isnan(negative_float_nan));
     ASSERT_OK_AND_ASSIGN(auto float_hash_function, FastHash::GetHashFunction(arrow::float32()));
     CheckResult(float_hash_function, {Literal(float_nan), Literal(negative_float_nan)},
                 {0x67c27c6d9936ae63, 0x67c27c6d9936ae63});
 
-    auto double_from_bits = [](uint64_t bits) {
-        double value;
-        std::memcpy(&value, &bits, sizeof(value));
-        return value;
-    };
-    const double double_nan = double_from_bits(0x7ff8123456789abc);
-    const double negative_double_nan = double_from_bits(0xfff8abcdef012345);
+    const auto double_nan = FloatingPointFromBits<double>(0x7ff8123456789abcULL);
+    const auto negative_double_nan = FloatingPointFromBits<double>(0xfff8abcdef012345ULL);
     ASSERT_TRUE(std::isnan(double_nan));
     ASSERT_TRUE(std::isnan(negative_double_nan));
     ASSERT_OK_AND_ASSIGN(auto double_hash_function, FastHash::GetHashFunction(arrow::float64()));

src/paimon/common/global_index/btree/key_serializer.cpp

@@ -27,6 +27,7 @@
 #include "paimon/common/utils/date_time_utils.h"
 #include "paimon/common/utils/field_type_utils.h"
 #include "paimon/common/utils/fields_comparator.h"
+#include "paimon/common/utils/math.h"
 #include "paimon/common/utils/preconditions.h"
 #include "paimon/common/utils/var_length_int_utils.h"
 #include "paimon/data/decimal.h"
@@ -164,19 +165,13 @@ Result<std::shared_ptr<Bytes>> KeySerializer::SerializeKey(
         case FieldType::FLOAT: {
             MemorySliceOutput output(4, pool);
             output.Reset();
-            auto fvalue = literal.GetValue<float>();
-            int32_t ivalue;
-            memcpy(&ivalue, &fvalue, sizeof(float));
-            output.WriteValue<int32_t>(ivalue);
+            output.WriteValue<int32_t>(CanonicalizeFloatToIntBits(literal.GetValue<float>()));
             return output.ToSlice().CopyBytes(pool);
         }
         case FieldType::DOUBLE: {
             MemorySliceOutput output(8, pool);
             output.Reset();
-            auto dvalue = literal.GetValue<double>();
-            int64_t ivalue;
-            memcpy(&ivalue, &dvalue, sizeof(double));
-            output.WriteValue<int64_t>(ivalue);
+            output.WriteValue<int64_t>(CanonicalizeDoubleToLongBits(literal.GetValue<double>()));
             return output.ToSlice().CopyBytes(pool);
         }
         case FieldType::STRING: {

src/paimon/common/global_index/btree/key_serializer_test.cpp

@@ -19,7 +19,11 @@
 
 #include "paimon/common/global_index/btree/key_serializer.h"
 
+#include <cstdint>
+#include <string>
+
 #include "gtest/gtest.h"
+#include "paimon/common/utils/math.h"
 #include "paimon/data/decimal.h"
 #include "paimon/data/timestamp.h"
 #include "paimon/testing/utils/testharness.h"
@@ -208,6 +212,30 @@ TEST_F(KeySerializerTest, SerializeAndDeserializeAllTypes) {
     }
 }
 
+TEST_F(KeySerializerTest, CanonicalizesFloatingPointNaN) {
+    const auto float_nan = FloatingPointFromBits<float>(0xffc12345U);
+    const auto canonical_float_nan = FloatingPointFromBits<float>(kCanonicalFloatNaNBits);
+    ASSERT_OK_AND_ASSIGN(
+        std::shared_ptr<Bytes> float_bytes,
+        KeySerializer::SerializeKey(Literal(float_nan), arrow::float32(), pool_.get()));
+    ASSERT_OK_AND_ASSIGN(
+        std::shared_ptr<Bytes> canonical_float_bytes,
+        KeySerializer::SerializeKey(Literal(canonical_float_nan), arrow::float32(), pool_.get()));
+    ASSERT_EQ(std::string(float_bytes->data(), float_bytes->size()),
+              std::string(canonical_float_bytes->data(), canonical_float_bytes->size()));
+
+    const auto double_nan = FloatingPointFromBits<double>(0xfff8123456789abcULL);
+    const auto canonical_double_nan = FloatingPointFromBits<double>(kCanonicalDoubleNaNBits);
+    ASSERT_OK_AND_ASSIGN(
+        std::shared_ptr<Bytes> double_bytes,
+        KeySerializer::SerializeKey(Literal(double_nan), arrow::float64(), pool_.get()));
+    ASSERT_OK_AND_ASSIGN(
+        std::shared_ptr<Bytes> canonical_double_bytes,
+        KeySerializer::SerializeKey(Literal(canonical_double_nan), arrow::float64(), pool_.get()));
+    ASSERT_EQ(std::string(double_bytes->data(), double_bytes->size()),
+              std::string(canonical_double_bytes->data(), canonical_double_bytes->size()));
+}
+
 TEST_F(KeySerializerTest, RejectsMalformedSerializedKeys) {
     auto wrap = [this](const std::string& value) {
         return MemorySlice::Wrap(std::make_shared<Bytes>(value, pool_.get()));

src/paimon/common/global_index/global_index_result.cpp

@@ -22,6 +22,7 @@
 #include "fmt/format.h"
 #include "paimon/common/io/memory_segment_output_stream.h"
 #include "paimon/common/memory/memory_segment_utils.h"
+#include "paimon/common/utils/math.h"
 #include "paimon/global_index/bitmap_global_index_result.h"
 #include "paimon/global_index/bitmap_scored_global_index_result.h"
 #include "paimon/io/byte_array_input_stream.h"
@@ -37,8 +38,8 @@ void WriteBitmapAndScores(const RoaringBitmap64* bitmap, const std::vector<float
     out->WriteBytes(bitmap_bytes);
 
     out->WriteValue<int32_t>(scores.size());
-    for (auto score : scores) {
-        out->WriteValue<float>(score);
+    for (float score : scores) {
+        out->WriteValue<float>(CanonicalizeFloatingPoint(score));
     }
 }
 

src/paimon/common/global_index/global_index_result_test.cpp

@@ -19,9 +19,12 @@
 
 #include "paimon/global_index/global_index_result.h"
 
+#include <cmath>
+#include <cstdint>
 #include <utility>
 
 #include "gtest/gtest.h"
+#include "paimon/common/utils/math.h"
 #include "paimon/global_index/bitmap_global_index_result.h"
 #include "paimon/global_index/bitmap_scored_global_index_result.h"
 #include "paimon/testing/utils/testharness.h"
@@ -144,6 +147,29 @@ TEST_F(GlobalIndexResultTest, TestSerializeAndDeserializeWithScore) {
                                                 serialize_bytes->data() + serialize_bytes->size()));
 }
 
+TEST_F(GlobalIndexResultTest, TestSerializeCanonicalizesNaNScore) {
+    auto pool = GetDefaultPool();
+    const auto payload_nan = FloatingPointFromBits<float>(0xffc12345U);
+    const auto canonical_nan = FloatingPointFromBits<float>(kCanonicalFloatNaNBits);
+    auto index_result = std::make_shared<BitmapScoredGlobalIndexResult>(
+        RoaringBitmap64::From({1}), std::vector<float>{payload_nan});
+    auto canonical_index_result = std::make_shared<BitmapScoredGlobalIndexResult>(
+        RoaringBitmap64::From({1}), std::vector<float>{canonical_nan});
+
+    ASSERT_OK_AND_ASSIGN(PAIMON_UNIQUE_PTR<Bytes> serialized,
+                         GlobalIndexResult::Serialize(index_result, pool));
+    ASSERT_OK_AND_ASSIGN(PAIMON_UNIQUE_PTR<Bytes> canonical_serialized,
+                         GlobalIndexResult::Serialize(canonical_index_result, pool));
+    ASSERT_EQ(*serialized, *canonical_serialized);
+
+    ASSERT_OK_AND_ASSIGN(
+        std::shared_ptr<GlobalIndexResult> deserialized,
+        GlobalIndexResult::Deserialize(serialized->data(), serialized->size(), pool));
+    auto scored_result = std::dynamic_pointer_cast<BitmapScoredGlobalIndexResult>(deserialized);
+    ASSERT_TRUE(scored_result);
+    ASSERT_TRUE(std::isnan(scored_result->GetScores()[0]));
+}
+
 TEST_F(GlobalIndexResultTest, TestInvalidSerialize) {
     auto pool = GetDefaultPool();
     auto result = std::make_shared<FakeGlobalIndexResult>(std::vector<int64_t>({1, 3, 5, 100}));

src/paimon/common/predicate/literal.cpp

@@ -18,7 +18,6 @@
 
 #include "paimon/predicate/literal.h"
 
-#include <cmath>
 #include <cstring>
 #include <functional>
 #include <sstream>
@@ -29,6 +28,7 @@
 #include "fmt/format.h"
 #include "paimon/common/utils/field_type_utils.h"
 #include "paimon/common/utils/fields_comparator.h"
+#include "paimon/common/utils/math.h"
 #include "paimon/data/decimal.h"
 #include "paimon/data/timestamp.h"
 #include "paimon/status.h"
@@ -63,9 +63,9 @@ class Literal::Impl {
             case FieldType::BIGINT:
                 return std::hash<int64_t>{}(value_.BigIntVal);
             case FieldType::FLOAT:
-                return std::hash<float>{}(value_.FloatVal);
+                return std::hash<float>{}(CanonicalizeFloatingPoint(value_.FloatVal));
             case FieldType::DOUBLE:
-                return std::hash<double>{}(value_.DoubleVal);
+                return std::hash<double>{}(CanonicalizeFloatingPoint(value_.DoubleVal));
             case FieldType::STRING:
             case FieldType::BINARY:
                 return std::hash<std::string_view>{}(std::string_view(value_.Buffer, size_));