diff --git a/include/paimon/predicate/literal.h b/include/paimon/predicate/literal.h index ef5864573..168d6a451 100644 --- a/include/paimon/predicate/literal.h +++ b/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 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 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 diff --git a/src/paimon/common/data/variant/generic_variant_test.cpp b/src/paimon/common/data/variant/generic_variant_test.cpp index 11386a28a..bc46a5730 100644 --- a/src/paimon/common/data/variant/generic_variant_test.cpp +++ b/src/paimon/common/data/variant/generic_variant_test.cpp @@ -19,6 +19,7 @@ #include "paimon/common/data/variant/generic_variant.h" +#include #include #include #include @@ -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(0xffc12345U))); + ASSERT_OK_AND_ASSIGN(std::shared_ptr 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(0xfff8123456789abcULL))); + ASSERT_OK_AND_ASSIGN(std::shared_ptr 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"); diff --git a/src/paimon/common/data/variant/variant_builder.cpp b/src/paimon/common/data/variant/variant_builder.cpp index b3cf7ee5d..51f704175 100644 --- a/src/paimon/common/data/variant/variant_builder.cpp +++ b/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(); diff --git a/src/paimon/common/file_index/bloomfilter/fast_hash.cpp b/src/paimon/common/file_index/bloomfilter/fast_hash.cpp index b1d8784fc..ef63f0fc1 100644 --- a/src/paimon/common/file_index/bloomfilter/fast_hash.cpp +++ b/src/paimon/common/file_index/bloomfilter/fast_hash.cpp @@ -19,7 +19,6 @@ #include "paimon/common/file_index/bloomfilter/fast_hash.h" #include -#include #include #include #include @@ -28,6 +27,7 @@ #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" @@ -35,11 +35,6 @@ #include "xxhash.h" // NOLINT(build/include_subdir) namespace paimon { -namespace { -constexpr int32_t kCanonicalFloatNaNBits = 0x7fc00000; -constexpr int64_t kCanonicalDoubleNaNBits = 0x7ff8000000000000L; -} // namespace - Result FastHash::GetHashFunction( const std::shared_ptr& arrow_type) { PAIMON_ASSIGN_OR_RAISE(FieldType field_type, @@ -64,23 +59,11 @@ Result FastHash::GetHashFunction( }); case FieldType::FLOAT: return HashFunction([](const Literal& literal) -> int64_t { - const auto raw_value = literal.GetValue(); - 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())); }); case FieldType::DOUBLE: return HashFunction([](const Literal& literal) -> int64_t { - const auto raw_value = literal.GetValue(); - 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())); }); case FieldType::TIMESTAMP: { auto ts_type = checked_pointer_cast(arrow_type); diff --git a/src/paimon/common/file_index/bloomfilter/fast_hash_test.cpp b/src/paimon/common/file_index/bloomfilter/fast_hash_test.cpp index 8a528e443..6ffbc7964 100644 --- a/src/paimon/common/file_index/bloomfilter/fast_hash_test.cpp +++ b/src/paimon/common/file_index/bloomfilter/fast_hash_test.cpp @@ -20,12 +20,12 @@ #include #include -#include #include #include #include #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(0x7fc12345U); + const auto negative_float_nan = FloatingPointFromBits(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(0x7ff8123456789abcULL); + const auto negative_double_nan = FloatingPointFromBits(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())); diff --git a/src/paimon/common/global_index/btree/key_serializer.cpp b/src/paimon/common/global_index/btree/key_serializer.cpp index 464f37ea4..4b66d1d1f 100644 --- a/src/paimon/common/global_index/btree/key_serializer.cpp +++ b/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> KeySerializer::SerializeKey( case FieldType::FLOAT: { MemorySliceOutput output(4, pool); output.Reset(); - auto fvalue = literal.GetValue(); - int32_t ivalue; - memcpy(&ivalue, &fvalue, sizeof(float)); - output.WriteValue(ivalue); + output.WriteValue(CanonicalizeFloatToIntBits(literal.GetValue())); return output.ToSlice().CopyBytes(pool); } case FieldType::DOUBLE: { MemorySliceOutput output(8, pool); output.Reset(); - auto dvalue = literal.GetValue(); - int64_t ivalue; - memcpy(&ivalue, &dvalue, sizeof(double)); - output.WriteValue(ivalue); + output.WriteValue(CanonicalizeDoubleToLongBits(literal.GetValue())); return output.ToSlice().CopyBytes(pool); } case FieldType::STRING: { diff --git a/src/paimon/common/global_index/btree/key_serializer_test.cpp b/src/paimon/common/global_index/btree/key_serializer_test.cpp index e36e72ed6..61322fde4 100644 --- a/src/paimon/common/global_index/btree/key_serializer_test.cpp +++ b/src/paimon/common/global_index/btree/key_serializer_test.cpp @@ -19,7 +19,11 @@ #include "paimon/common/global_index/btree/key_serializer.h" +#include +#include + #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(0xffc12345U); + const auto canonical_float_nan = FloatingPointFromBits(kCanonicalFloatNaNBits); + ASSERT_OK_AND_ASSIGN( + std::shared_ptr float_bytes, + KeySerializer::SerializeKey(Literal(float_nan), arrow::float32(), pool_.get())); + ASSERT_OK_AND_ASSIGN( + std::shared_ptr 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(0xfff8123456789abcULL); + const auto canonical_double_nan = FloatingPointFromBits(kCanonicalDoubleNaNBits); + ASSERT_OK_AND_ASSIGN( + std::shared_ptr double_bytes, + KeySerializer::SerializeKey(Literal(double_nan), arrow::float64(), pool_.get())); + ASSERT_OK_AND_ASSIGN( + std::shared_ptr 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(value, pool_.get())); diff --git a/src/paimon/common/global_index/global_index_result.cpp b/src/paimon/common/global_index/global_index_result.cpp index f329b0361..d2b94f216 100644 --- a/src/paimon/common/global_index/global_index_result.cpp +++ b/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::vectorWriteBytes(bitmap_bytes); out->WriteValue(scores.size()); - for (auto score : scores) { - out->WriteValue(score); + for (float score : scores) { + out->WriteValue(CanonicalizeFloatingPoint(score)); } } diff --git a/src/paimon/common/global_index/global_index_result_test.cpp b/src/paimon/common/global_index/global_index_result_test.cpp index 73c6d05ed..3179e6710 100644 --- a/src/paimon/common/global_index/global_index_result_test.cpp +++ b/src/paimon/common/global_index/global_index_result_test.cpp @@ -19,9 +19,12 @@ #include "paimon/global_index/global_index_result.h" +#include +#include #include #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(0xffc12345U); + const auto canonical_nan = FloatingPointFromBits(kCanonicalFloatNaNBits); + auto index_result = std::make_shared( + RoaringBitmap64::From({1}), std::vector{payload_nan}); + auto canonical_index_result = std::make_shared( + RoaringBitmap64::From({1}), std::vector{canonical_nan}); + + ASSERT_OK_AND_ASSIGN(PAIMON_UNIQUE_PTR serialized, + GlobalIndexResult::Serialize(index_result, pool)); + ASSERT_OK_AND_ASSIGN(PAIMON_UNIQUE_PTR canonical_serialized, + GlobalIndexResult::Serialize(canonical_index_result, pool)); + ASSERT_EQ(*serialized, *canonical_serialized); + + ASSERT_OK_AND_ASSIGN( + std::shared_ptr deserialized, + GlobalIndexResult::Deserialize(serialized->data(), serialized->size(), pool)); + auto scored_result = std::dynamic_pointer_cast(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(std::vector({1, 3, 5, 100})); diff --git a/src/paimon/common/predicate/literal.cpp b/src/paimon/common/predicate/literal.cpp index 3b2bcc0e6..d679c2ccf 100644 --- a/src/paimon/common/predicate/literal.cpp +++ b/src/paimon/common/predicate/literal.cpp @@ -18,7 +18,6 @@ #include "paimon/predicate/literal.h" -#include #include #include #include @@ -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{}(value_.BigIntVal); case FieldType::FLOAT: - return std::hash{}(value_.FloatVal); + return std::hash{}(CanonicalizeFloatingPoint(value_.FloatVal)); case FieldType::DOUBLE: - return std::hash{}(value_.DoubleVal); + return std::hash{}(CanonicalizeFloatingPoint(value_.DoubleVal)); case FieldType::STRING: case FieldType::BINARY: return std::hash{}(std::string_view(value_.Buffer, size_)); diff --git a/src/paimon/common/utils/math.h b/src/paimon/common/utils/math.h index 6aba6523f..b9e2e3103 100644 --- a/src/paimon/common/utils/math.h +++ b/src/paimon/common/utils/math.h @@ -28,6 +28,7 @@ #pragma once #include +#include #include #include #include @@ -41,6 +42,51 @@ namespace paimon { +inline constexpr uint32_t kCanonicalFloatNaNBits = 0x7fc00000; +inline constexpr uint64_t kCanonicalDoubleNaNBits = 0x7ff8000000000000; + +template +inline FloatingPoint FloatingPointFromBits(Bits bits) { + static_assert(std::is_floating_point_v); + static_assert(std::is_integral_v); + static_assert(sizeof(FloatingPoint) == sizeof(Bits)); + FloatingPoint value; + std::memcpy(&value, &bits, sizeof(value)); + return value; +} + +inline float CanonicalizeFloatingPoint(float value) { + if (std::isnan(value)) { + return FloatingPointFromBits(kCanonicalFloatNaNBits); + } + return value; +} + +inline double CanonicalizeFloatingPoint(double value) { + if (std::isnan(value)) { + return FloatingPointFromBits(kCanonicalDoubleNaNBits); + } + return value; +} + +inline int32_t CanonicalizeFloatToIntBits(float value) { + if (std::isnan(value)) { + return static_cast(kCanonicalFloatNaNBits); + } + int32_t bits; + std::memcpy(&bits, &value, sizeof(bits)); + return bits; +} + +inline int64_t CanonicalizeDoubleToLongBits(double value) { + if (std::isnan(value)) { + return static_cast(kCanonicalDoubleNaNBits); + } + int64_t bits; + std::memcpy(&bits, &value, sizeof(bits)); + return bits; +} + template constexpr bool InRange(From value) { static_assert(std::is_integral_v && std::is_integral_v, diff --git a/src/paimon/common/utils/math_test.cpp b/src/paimon/common/utils/math_test.cpp index 49d31d472..ce9379f6c 100644 --- a/src/paimon/common/utils/math_test.cpp +++ b/src/paimon/common/utils/math_test.cpp @@ -28,6 +28,29 @@ namespace paimon::test { +TEST(MathTest, FloatingPointNaNCanonicalization) { + const auto float_nan = CanonicalizeFloatingPoint(FloatingPointFromBits(0xffc12345U)); + uint32_t float_nan_bits; + std::memcpy(&float_nan_bits, &float_nan, sizeof(float_nan_bits)); + ASSERT_EQ(kCanonicalFloatNaNBits, float_nan_bits); + ASSERT_EQ(static_cast(kCanonicalFloatNaNBits), + CanonicalizeFloatToIntBits(FloatingPointFromBits(0x7fa12345U))); + + const auto double_nan = + CanonicalizeFloatingPoint(FloatingPointFromBits(0xfff8123456789abcULL)); + uint64_t double_nan_bits; + std::memcpy(&double_nan_bits, &double_nan, sizeof(double_nan_bits)); + ASSERT_EQ(kCanonicalDoubleNaNBits, double_nan_bits); + ASSERT_EQ(static_cast(kCanonicalDoubleNaNBits), + CanonicalizeDoubleToLongBits(FloatingPointFromBits(0x7ff123456789abcdULL))); + + const float negative_zero = CanonicalizeFloatingPoint(-0.0f); + uint32_t negative_zero_bits; + std::memcpy(&negative_zero_bits, &negative_zero, sizeof(negative_zero_bits)); + ASSERT_EQ(0x80000000U, negative_zero_bits); + ASSERT_EQ(0x3ff0000000000000, CanonicalizeDoubleToLongBits(1.0)); +} + // Test case: Test EndianSwapValue for different integral types TEST(MathTest, EndianSwapValue) { // Test 16-bit value diff --git a/src/paimon/core/bucket/hive_bucket_function.cpp b/src/paimon/core/bucket/hive_bucket_function.cpp index 913053c1f..e87c292d9 100644 --- a/src/paimon/core/bucket/hive_bucket_function.cpp +++ b/src/paimon/core/bucket/hive_bucket_function.cpp @@ -19,13 +19,12 @@ #include "paimon/core/bucket/hive_bucket_function.h" #include -#include -#include #include #include "fmt/format.h" #include "paimon/common/data/binary_row.h" #include "paimon/common/utils/field_type_utils.h" +#include "paimon/common/utils/math.h" #include "paimon/core/bucket/hive_hasher.h" #include "paimon/status.h" @@ -105,10 +104,8 @@ uint32_t HiveBucketFunction::ComputeHash(const BinaryRow& row, int32_t field_ind uint32_t bits; if (float_value == -0.0f) { bits = 0; - } else if (std::isnan(float_value)) { - bits = 0x7FC00000U; } else { - std::memcpy(&bits, &float_value, sizeof(bits)); + bits = static_cast(CanonicalizeFloatToIntBits(float_value)); } return HiveHasher::HashInt(bits); } @@ -117,10 +114,8 @@ uint32_t HiveBucketFunction::ComputeHash(const BinaryRow& row, int32_t field_ind uint64_t bits; if (double_value == -0.0) { bits = 0; - } else if (std::isnan(double_value)) { - bits = 0x7FF8000000000000ULL; } else { - std::memcpy(&bits, &double_value, sizeof(bits)); + bits = static_cast(CanonicalizeDoubleToLongBits(double_value)); } return HiveHasher::HashLong(bits); } diff --git a/src/paimon/core/bucket/hive_bucket_function_test.cpp b/src/paimon/core/bucket/hive_bucket_function_test.cpp index 21f2a9843..d97a0294b 100644 --- a/src/paimon/core/bucket/hive_bucket_function_test.cpp +++ b/src/paimon/core/bucket/hive_bucket_function_test.cpp @@ -18,12 +18,12 @@ #include "paimon/core/bucket/hive_bucket_function.h" -#include #include #include "gtest/gtest.h" #include "paimon/common/data/binary_row.h" #include "paimon/common/data/binary_row_writer.h" +#include "paimon/common/utils/math.h" #include "paimon/core/bucket/hive_hasher.h" #include "paimon/memory/memory_pool.h" #include "paimon/testing/utils/binary_row_generator.h" @@ -111,18 +111,6 @@ class HiveBucketFunctionTest : public ::testing::Test { auto pool = GetDefaultPool(); return BinaryRowGenerator::GenerateRow({value}, pool.get()); } - - float FloatFromBits(uint32_t bits) { - float value; - std::memcpy(&value, &bits, sizeof(value)); - return value; - } - - double DoubleFromBits(uint64_t bits) { - double value; - std::memcpy(&value, &bits, sizeof(value)); - return value; - } }; /// Test matching Java: testHiveBucketFunction @@ -235,11 +223,12 @@ TEST_F(HiveBucketFunctionTest, TestFloatNaNCanonicalizationCompatibleWithJava) { ASSERT_OK_AND_ASSIGN(auto func, HiveBucketFunction::Create(field_types)); // Verified with Java HiveBucketFunction: - // Float.NaN, Float.intBitsToFloat(0x7fa12345), and Float.intBitsToFloat(0x7fc00000) - // all hash through Float.floatToIntBits(...) = 0x7fc00000. + // Float.NaN, a payload NaN, and the canonical NaN all hash through + // Float.floatToIntBits(...) to kCanonicalFloatNaNBits. ASSERT_EQ(344, func->Bucket(CreateFloatRow(std::numeric_limits::quiet_NaN()), 1000)); - ASSERT_EQ(344, func->Bucket(CreateFloatRow(FloatFromBits(0x7FA12345U)), 1000)); - ASSERT_EQ(344, func->Bucket(CreateFloatRow(FloatFromBits(0x7FC00000U)), 1000)); + ASSERT_EQ(344, func->Bucket(CreateFloatRow(FloatingPointFromBits(0x7FA12345U)), 1000)); + ASSERT_EQ(344, func->Bucket( + CreateFloatRow(FloatingPointFromBits(kCanonicalFloatNaNBits)), 1000)); } TEST_F(HiveBucketFunctionTest, TestDoubleNaNCanonicalizationCompatibleWithJava) { @@ -248,10 +237,14 @@ TEST_F(HiveBucketFunctionTest, TestDoubleNaNCanonicalizationCompatibleWithJava) // Verified with Java HiveBucketFunction: // Double.NaN, Double.longBitsToDouble(0x7ff123456789abcd), and canonical NaN - // all hash through Double.doubleToLongBits(...) = 0x7ff8000000000000. + // All NaNs hash through Double.doubleToLongBits(...) to kCanonicalDoubleNaNBits. ASSERT_EQ(360, func->Bucket(CreateDoubleRow(std::numeric_limits::quiet_NaN()), 1000)); - ASSERT_EQ(360, func->Bucket(CreateDoubleRow(DoubleFromBits(0x7FF123456789ABCDULL)), 1000)); - ASSERT_EQ(360, func->Bucket(CreateDoubleRow(DoubleFromBits(0x7FF8000000000000ULL)), 1000)); + ASSERT_EQ( + 360, + func->Bucket(CreateDoubleRow(FloatingPointFromBits(0x7FF123456789ABCDULL)), 1000)); + ASSERT_EQ(360, + func->Bucket(CreateDoubleRow(FloatingPointFromBits(kCanonicalDoubleNaNBits)), + 1000)); } TEST_F(HiveBucketFunctionTest, TestTinyintNegativeValuesCompatibleWithJava) { diff --git a/src/paimon/core/global_index/indexed_split_test.cpp b/src/paimon/core/global_index/indexed_split_test.cpp index 7cd121257..03cd2976d 100644 --- a/src/paimon/core/global_index/indexed_split_test.cpp +++ b/src/paimon/core/global_index/indexed_split_test.cpp @@ -17,6 +17,8 @@ * under the License. */ +#include +#include #include #include #include @@ -26,6 +28,7 @@ #include "gtest/gtest.h" #include "paimon/common/data/binary_row.h" #include "paimon/common/data/data_define.h" +#include "paimon/common/utils/math.h" #include "paimon/core/global_index/indexed_split_impl.h" #include "paimon/core/table/source/data_split_impl.h" #include "paimon/fs/local/local_file_system.h" @@ -155,6 +158,34 @@ TEST(IndexedSplitTest, TestIndexedSplitWithScore) { << roundtrip_indexed_split->ToString(); } +TEST(IndexedSplitTest, TestSerializeCanonicalizesNaNScore) { + auto pool = GetDefaultPool(); + DataSplitImpl::Builder builder( + /*partition=*/BinaryRow::EmptyRow(), + /*bucket=*/0, /*bucket_path=*/"bucket-0", + /*data_files=*/{}); + ASSERT_OK_AND_ASSIGN(std::shared_ptr data_split, builder.Build()); + + const auto payload_nan = FloatingPointFromBits(0xffc12345U); + const auto canonical_nan = FloatingPointFromBits(kCanonicalFloatNaNBits); + auto indexed_split = std::make_shared( + data_split, std::vector{Range(0, 0)}, std::vector{payload_nan}); + auto canonical_indexed_split = std::make_shared( + data_split, std::vector{Range(0, 0)}, std::vector{canonical_nan}); + + ASSERT_OK_AND_ASSIGN(std::string serialized, Split::Serialize(indexed_split, pool)); + ASSERT_OK_AND_ASSIGN(std::string canonical_serialized, + Split::Serialize(canonical_indexed_split, pool)); + ASSERT_EQ(serialized, canonical_serialized); + + ASSERT_OK_AND_ASSIGN(std::shared_ptr roundtrip, + Split::Deserialize(serialized.data(), serialized.size(), pool)); + auto roundtrip_indexed_split = std::dynamic_pointer_cast(roundtrip); + ASSERT_TRUE(roundtrip_indexed_split); + ASSERT_EQ(roundtrip_indexed_split->Scores().size(), 1); + ASSERT_TRUE(std::isnan(roundtrip_indexed_split->Scores()[0])); +} + TEST(IndexedSplitTest, TestValidate) { auto meta = std::make_shared( "file.orc", 1l, 200l, BinaryRow::EmptyRow(), BinaryRow::EmptyRow(), diff --git a/src/paimon/core/table/source/split.cpp b/src/paimon/core/table/source/split.cpp index 007df3cb1..d6117a899 100644 --- a/src/paimon/core/table/source/split.cpp +++ b/src/paimon/core/table/source/split.cpp @@ -23,6 +23,7 @@ #include "paimon/common/data/binary_row.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/common/utils/serialization_utils.h" #include "paimon/core/global_index/indexed_split_impl.h" #include "paimon/core/io/data_file_meta_serializer.h" @@ -159,8 +160,8 @@ Result Split::Serialize(const std::shared_ptr& split, if (!scores.empty()) { out.WriteValue(true); out.WriteValue(scores.size()); - for (const auto& score : scores) { - out.WriteValue(score); + for (float score : scores) { + out.WriteValue(CanonicalizeFloatingPoint(score)); } } else { out.WriteValue(false);