From 1722193ceb4cf6d3de8cf472a51b6e23007247ff Mon Sep 17 00:00:00 2001 From: "jinli.zjw" Date: Thu, 27 Aug 2026 19:24:45 +0800 Subject: [PATCH 1/6] fix(file-index): canonicalize floating-point NaN values --- include/paimon/predicate/literal.h | 13 +- .../bitmap/bitmap_file_index_meta.cpp | 32 +++ .../bitmap/bitmap_file_index_test.cpp | 253 ++++++++++++++++++ .../file_index/bloomfilter/fast_hash.cpp | 23 +- src/paimon/common/predicate/literal.cpp | 6 +- src/paimon/common/utils/math.h | 36 +++ src/paimon/common/utils/math_test.cpp | 33 +++ .../core/bucket/hive_bucket_function.cpp | 11 +- .../core/bucket/hive_bucket_function_test.cpp | 11 +- 9 files changed, 375 insertions(+), 43 deletions(-) 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/file_index/bitmap/bitmap_file_index_meta.cpp b/src/paimon/common/file_index/bitmap/bitmap_file_index_meta.cpp index 64e05c650..50b88af3c 100644 --- a/src/paimon/common/file_index/bitmap/bitmap_file_index_meta.cpp +++ b/src/paimon/common/file_index/bitmap/bitmap_file_index_meta.cpp @@ -18,11 +18,13 @@ #include "paimon/common/file_index/bitmap/bitmap_file_index_meta.h" +#include #include #include #include "fmt/format.h" #include "paimon/common/utils/field_type_utils.h" +#include "paimon/common/utils/math.h" #include "paimon/defs.h" #include "paimon/io/data_input_stream.h" #include "paimon/memory/bytes.h" @@ -80,6 +82,18 @@ Result> BitmapFileIndexMeta::GetValueWriter( [output_stream](const Literal& literal) -> void { output_stream->WriteValue(literal.GetValue()); }); + case FieldType::FLOAT: + return std::function( + [output_stream](const Literal& literal) -> void { + const float value = CanonicalizeFloatingPoint(literal.GetValue()); + output_stream->WriteValue(value); + }); + case FieldType::DOUBLE: + return std::function( + [output_stream](const Literal& literal) -> void { + const double value = CanonicalizeFloatingPoint(literal.GetValue()); + output_stream->WriteValue(value); + }); case FieldType::STRING: return std::function( [output_stream](const Literal& literal) -> void { @@ -155,6 +169,24 @@ Result()>> BitmapFileIndexMeta::GetValueReader( }; return func; } + case FieldType::FLOAT: { + std::function()> func = [&in, move_body_start, + this]() -> Result { + PAIMON_ASSIGN_OR_RAISE(float value, + ReadAndMoveBodyStart(in, move_body_start)); + return Literal(value); + }; + return func; + } + case FieldType::DOUBLE: { + std::function()> func = [&in, move_body_start, + this]() -> Result { + PAIMON_ASSIGN_OR_RAISE(double value, + ReadAndMoveBodyStart(in, move_body_start)); + return Literal(value); + }; + return func; + } case FieldType::DATE: { std::function()> func = [&in, move_body_start, this]() -> Result { diff --git a/src/paimon/common/file_index/bitmap/bitmap_file_index_test.cpp b/src/paimon/common/file_index/bitmap/bitmap_file_index_test.cpp index 42a888a8b..d0444e4c1 100644 --- a/src/paimon/common/file_index/bitmap/bitmap_file_index_test.cpp +++ b/src/paimon/common/file_index/bitmap/bitmap_file_index_test.cpp @@ -18,6 +18,7 @@ #include "paimon/common/file_index/bitmap/bitmap_file_index.h" +#include #include #include "arrow/api.h" @@ -27,6 +28,7 @@ #include "paimon/common/utils/arrow/status_utils.h" #include "paimon/common/utils/checked_cast.h" #include "paimon/common/utils/date_time_utils.h" +#include "paimon/common/utils/math.h" #include "paimon/data/timestamp.h" #include "paimon/defs.h" #include "paimon/file_index/bitmap_index_result.h" @@ -36,6 +38,23 @@ #include "paimon/memory/memory_pool.h" #include "paimon/testing/utils/testharness.h" namespace paimon::test { +namespace { + +template +FloatingPoint FloatingPointFromBits(UInt bits) { + static_assert(sizeof(FloatingPoint) == sizeof(UInt)); + FloatingPoint value; + std::memcpy(&value, &bits, sizeof(value)); + return value; +} + +template +std::vector JavaBytes(const char (&bytes)[N]) { + return std::vector(bytes, bytes + N - 1); +} + +} // namespace + class BitmapIndexTest : public ::testing::Test { public: void SetUp() override { @@ -78,6 +97,21 @@ class BitmapIndexTest : public ::testing::Test { return writer->SerializedBytes(); } + template + Result> CreateArray(const std::shared_ptr& type, + const std::vector& values) const { + auto value_builder = std::make_shared(); + for (ValueType value : values) { + PAIMON_RETURN_NOT_OK_FROM_ARROW(value_builder->Append(value)); + } + PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(std::shared_ptr value_array, + value_builder->Finish()); + PAIMON_ASSIGN_OR_RAISE_FROM_ARROW( + std::shared_ptr struct_array, + arrow::StructArray::Make({value_array}, {arrow::field("f0", type)})); + return struct_array; + } + private: std::shared_ptr pool_; }; @@ -620,6 +654,225 @@ TEST_F(BitmapIndexTest, TestTimestampType) { } } +TEST_F(BitmapIndexTest, TestFloatAndDoubleTypes) { + const auto check_float = [&](int32_t version) { + const auto type = arrow::float32(); + auto array = + arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_({arrow::field("f0", type)}), + R"([[1.25], [null], [-2.5], [1.25], [3.75]])") + .ValueOrDie(); + ASSERT_OK_AND_ASSIGN(PAIMON_UNIQUE_PTR index_bytes, + WriteIndex(type, version, array)); + auto input_stream = + std::make_shared(index_bytes->data(), index_bytes->size()); + BitmapFileIndex file_index({}); + ASSERT_OK_AND_ASSIGN(auto reader, + file_index.CreateReader(CreateArrowSchema(type).get(), 0, + index_bytes->size(), input_stream, pool_)); + CheckResult(reader->VisitEqual(Literal(1.25f)).value(), {0, 3}); + CheckResult(reader->VisitNotEqual(Literal(1.25f)).value(), {2, 4}); + CheckResult(reader->VisitIsNull().value(), {1}); + }; + + const auto check_double = [&](int32_t version) { + const auto type = arrow::float64(); + auto array = + arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_({arrow::field("f0", type)}), + R"([[1.25], [null], [-2.5], [1.25], [3.75]])") + .ValueOrDie(); + ASSERT_OK_AND_ASSIGN(PAIMON_UNIQUE_PTR index_bytes, + WriteIndex(type, version, array)); + auto input_stream = + std::make_shared(index_bytes->data(), index_bytes->size()); + BitmapFileIndex file_index({}); + ASSERT_OK_AND_ASSIGN(auto reader, + file_index.CreateReader(CreateArrowSchema(type).get(), 0, + index_bytes->size(), input_stream, pool_)); + CheckResult(reader->VisitEqual(Literal(1.25)).value(), {0, 3}); + CheckResult(reader->VisitNotEqual(Literal(1.25)).value(), {2, 4}); + CheckResult(reader->VisitIsNull().value(), {1}); + }; + + for (int32_t version : {1, 2}) { + check_float(version); + check_double(version); + } +} + +TEST_F(BitmapIndexTest, TestFloatingPointJavaCompatibility) { + // Generated by BitmapFloatingPointCompatibilityTest with Apache Paimon Java at + // 0043a70fd88ac75dcb83a8f2da5e72ce91e22b1f. The Java writer receives canonical, + // positive-payload and negative-payload NaNs, followed by both signed zero values. + const std::vector java_float_v1 = JavaBytes( + "\x01\x00\x00\x00\x08\x00\x00\x00\x03\x00\x00\x00\x00\x00\x00\x00" + "\x00\x00\x80\x00\x00\x00\x00\x00\x00\x14\x7f\xc0\x00\x00\x00\x00" + "\x00\x28\x3a\x30\x00\x00\x01\x00\x00\x00\x00\x00\x01\x00\x10\x00" + "\x00\x00\x04\x00\x07\x00\x3a\x30\x00\x00\x01\x00\x00\x00\x00\x00" + "\x01\x00\x10\x00\x00\x00\x03\x00\x06\x00\x3a\x30\x00\x00\x01\x00" + "\x00\x00\x00\x00\x03\x00\x10\x00\x00\x00\x00\x00\x01\x00\x02\x00" + "\x05\x00"); + const std::vector java_float_v2 = JavaBytes( + "\x02\x00\x00\x00\x08\x00\x00\x00\x03\x00\x00\x00\x00\x01\x80\x00" + "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x28\x00\x00\x00\x03\x80\x00" + "\x00\x00\x00\x00\x00\x14\x00\x00\x00\x14\x00\x00\x00\x00\x00\x00" + "\x00\x00\x00\x00\x00\x14\x7f\xc0\x00\x00\x00\x00\x00\x28\x00\x00" + "\x00\x18\x3a\x30\x00\x00\x01\x00\x00\x00\x00\x00\x01\x00\x10\x00" + "\x00\x00\x04\x00\x07\x00\x3a\x30\x00\x00\x01\x00\x00\x00\x00\x00" + "\x01\x00\x10\x00\x00\x00\x03\x00\x06\x00\x3a\x30\x00\x00\x01\x00" + "\x00\x00\x00\x00\x03\x00\x10\x00\x00\x00\x00\x00\x01\x00\x02\x00" + "\x05\x00"); + const std::vector java_double_v1 = JavaBytes( + "\x01\x00\x00\x00\x08\x00\x00\x00\x03\x00\x00\x00\x00\x00\x00\x00" + "\x00\x00\x00\x00\x00\x00\x80\x00\x00\x00\x00\x00\x00\x00\x00\x00" + "\x00\x14\x7f\xf8\x00\x00\x00\x00\x00\x00\x00\x00\x00\x28\x3a\x30" + "\x00\x00\x01\x00\x00\x00\x00\x00\x01\x00\x10\x00\x00\x00\x04\x00" + "\x07\x00\x3a\x30\x00\x00\x01\x00\x00\x00\x00\x00\x01\x00\x10\x00" + "\x00\x00\x03\x00\x06\x00\x3a\x30\x00\x00\x01\x00\x00\x00\x00\x00" + "\x03\x00\x10\x00\x00\x00\x00\x00\x01\x00\x02\x00\x05\x00"); + const std::vector java_double_v2 = JavaBytes( + "\x02\x00\x00\x00\x08\x00\x00\x00\x03\x00\x00\x00\x00\x01\x80\x00" + "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x34\x00\x00" + "\x00\x03\x80\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x14\x00\x00" + "\x00\x14\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" + "\x00\x14\x7f\xf8\x00\x00\x00\x00\x00\x00\x00\x00\x00\x28\x00\x00" + "\x00\x18\x3a\x30\x00\x00\x01\x00\x00\x00\x00\x00\x01\x00\x10\x00" + "\x00\x00\x04\x00\x07\x00\x3a\x30\x00\x00\x01\x00\x00\x00\x00\x00" + "\x01\x00\x10\x00\x00\x00\x03\x00\x06\x00\x3a\x30\x00\x00\x01\x00" + "\x00\x00\x00\x00\x03\x00\x10\x00\x00\x00\x00\x00\x01\x00\x02\x00" + "\x05\x00"); + const std::vector java_float_nan_v1 = JavaBytes( + "\x01\x00\x00\x00\x03\x00\x00\x00\x01\x00\x7f\xc0\x00\x00\x00\x00" + "\x00\x00\x3a\x30\x00\x00\x01\x00\x00\x00\x00\x00\x02\x00\x10\x00" + "\x00\x00\x00\x00\x01\x00\x02\x00"); + const std::vector java_float_nan_v2 = JavaBytes( + "\x02\x00\x00\x00\x03\x00\x00\x00\x01\x00\x00\x00\x00\x01\x7f\xc0" + "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x10\x00\x00\x00\x01\x7f\xc0" + "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x16\x3a\x30\x00\x00\x01\x00" + "\x00\x00\x00\x00\x02\x00\x10\x00\x00\x00\x00\x00\x01\x00\x02\x00"); + const std::vector java_double_nan_v1 = JavaBytes( + "\x01\x00\x00\x00\x03\x00\x00\x00\x01\x00\x7f\xf8\x00\x00\x00\x00" + "\x00\x00\x00\x00\x00\x00\x3a\x30\x00\x00\x01\x00\x00\x00\x00\x00" + "\x02\x00\x10\x00\x00\x00\x00\x00\x01\x00\x02\x00"); + const std::vector java_double_nan_v2 = JavaBytes( + "\x02\x00\x00\x00\x03\x00\x00\x00\x01\x00\x00\x00\x00\x01\x7f\xf8" + "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x14\x00\x00" + "\x00\x01\x7f\xf8\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" + "\x00\x16\x3a\x30\x00\x00\x01\x00\x00\x00\x00\x00\x02\x00\x10\x00" + "\x00\x00\x00\x00\x01\x00\x02\x00"); + + const float float_nan = FloatingPointFromBits(kCanonicalFloatNaNBits); + const float float_positive_payload_nan = FloatingPointFromBits(uint32_t{0x7fc12345}); + const float float_negative_payload_nan = FloatingPointFromBits(uint32_t{0xffc54321}); + const std::vector float_values = {float_nan, + float_positive_payload_nan, + float_negative_payload_nan, + -0.0f, + +0.0f, + float_negative_payload_nan, + -0.0f, + +0.0f}; + ASSERT_OK_AND_ASSIGN(std::shared_ptr float_array, + (CreateArray(arrow::float32(), float_values))); + + const double double_nan = FloatingPointFromBits(kCanonicalDoubleNaNBits); + const double double_positive_payload_nan = + FloatingPointFromBits(uint64_t{0x7ff8123456789abc}); + const double double_negative_payload_nan = + FloatingPointFromBits(uint64_t{0xfff8abcdef012345}); + const std::vector double_values = {double_nan, + double_positive_payload_nan, + double_negative_payload_nan, + -0.0, + +0.0, + double_negative_payload_nan, + -0.0, + +0.0}; + ASSERT_OK_AND_ASSIGN(std::shared_ptr double_array, + (CreateArray(arrow::float64(), double_values))); + + const auto check_reader = [&](const std::shared_ptr& reader, + const std::vector& nan_literals, + const Literal& negative_zero, const Literal& positive_zero) { + for (const Literal& nan_literal : nan_literals) { + CheckResult(reader->VisitEqual(nan_literal).value(), {0, 1, 2, 5}); + } + CheckResult(reader->VisitEqual(negative_zero).value(), {3, 6}); + CheckResult(reader->VisitEqual(positive_zero).value(), {4, 7}); + }; + + const auto check = [&](const std::shared_ptr& type, + const std::shared_ptr& array, + const std::vector& nan_literals, const Literal& negative_zero, + const Literal& positive_zero, int32_t version, + const std::vector& java_bytes) { + auto input_stream = + std::make_shared(java_bytes.data(), java_bytes.size()); + BitmapFileIndex file_index({}); + ASSERT_OK_AND_ASSIGN(std::shared_ptr reader, + file_index.CreateReader(CreateArrowSchema(type).get(), 0, + java_bytes.size(), input_stream, pool_)); + check_reader(reader, nan_literals, negative_zero, positive_zero); + + ASSERT_OK_AND_ASSIGN(PAIMON_UNIQUE_PTR cpp_bytes, WriteIndex(type, version, array)); + auto cpp_input_stream = + std::make_shared(cpp_bytes->data(), cpp_bytes->size()); + ASSERT_OK_AND_ASSIGN(std::shared_ptr cpp_reader, + file_index.CreateReader(CreateArrowSchema(type).get(), 0, + cpp_bytes->size(), cpp_input_stream, pool_)); + check_reader(cpp_reader, nan_literals, negative_zero, positive_zero); + }; + + const auto check_nan_meta = [&](const std::shared_ptr& type, + const std::shared_ptr& array, int32_t version, + size_t key_size, const std::vector& java_bytes) { + ASSERT_OK_AND_ASSIGN(PAIMON_UNIQUE_PTR cpp_bytes, WriteIndex(type, version, array)); + // RoaringBitmap may choose a different, semantically equivalent body encoding. Compare + // the complete V1 meta, and the V2 meta through the entry offset (excluding only the body + // length derived from that encoding). This covers both serialized copies of the V2 key. + const size_t comparable_meta_size = + version == BitmapFileIndex::VERSION_1 ? 14 + key_size : 30 + 2 * key_size; + ASSERT_GE(java_bytes.size(), comparable_meta_size); + ASSERT_GE(cpp_bytes->size(), comparable_meta_size); + ASSERT_EQ(std::vector(java_bytes.begin(), java_bytes.begin() + comparable_meta_size), + std::vector(cpp_bytes->data(), cpp_bytes->data() + comparable_meta_size)); + }; + + check(arrow::float32(), float_array, + {Literal(float_nan), Literal(float_positive_payload_nan), + Literal(float_negative_payload_nan)}, + Literal(-0.0f), Literal(+0.0f), /*version=*/1, java_float_v1); + check(arrow::float32(), float_array, + {Literal(float_nan), Literal(float_positive_payload_nan), + Literal(float_negative_payload_nan)}, + Literal(-0.0f), Literal(+0.0f), /*version=*/2, java_float_v2); + check(arrow::float64(), double_array, + {Literal(double_nan), Literal(double_positive_payload_nan), + Literal(double_negative_payload_nan)}, + Literal(-0.0), Literal(+0.0), /*version=*/1, java_double_v1); + check(arrow::float64(), double_array, + {Literal(double_nan), Literal(double_positive_payload_nan), + Literal(double_negative_payload_nan)}, + Literal(-0.0), Literal(+0.0), /*version=*/2, java_double_v2); + + const std::vector float_nan_values = {float_negative_payload_nan, + float_positive_payload_nan, float_nan}; + ASSERT_OK_AND_ASSIGN(std::shared_ptr float_nan_array, + (CreateArray(arrow::float32(), float_nan_values))); + check_nan_meta(arrow::float32(), float_nan_array, /*version=*/1, sizeof(float), + java_float_nan_v1); + check_nan_meta(arrow::float32(), float_nan_array, /*version=*/2, sizeof(float), + java_float_nan_v2); + + const std::vector double_nan_values = {double_negative_payload_nan, + double_positive_payload_nan, double_nan}; + ASSERT_OK_AND_ASSIGN(std::shared_ptr double_nan_array, + (CreateArray(arrow::float64(), double_nan_values))); + check_nan_meta(arrow::float64(), double_nan_array, /*version=*/1, sizeof(double), + java_double_nan_v1); + check_nan_meta(arrow::float64(), double_nan_array, /*version=*/2, sizeof(double), + java_double_nan_v2); +} + TEST_F(BitmapIndexTest, TestHighCardinalityForCompatibility) { auto type = arrow::utf8(); auto check_result = [&](const std::string& index_file_name) { 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/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..787353fda 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,41 @@ namespace paimon { +inline constexpr uint32_t kCanonicalFloatNaNBits = 0x7fc00000; +inline constexpr uint64_t kCanonicalDoubleNaNBits = 0x7ff8000000000000; + +inline float CanonicalizeFloatingPoint(float value) { + if (std::isnan(value)) { + std::memcpy(&value, &kCanonicalFloatNaNBits, sizeof(value)); + } + return value; +} + +inline double CanonicalizeFloatingPoint(double value) { + if (std::isnan(value)) { + std::memcpy(&value, &kCanonicalDoubleNaNBits, sizeof(value)); + } + 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..343f12831 100644 --- a/src/paimon/common/utils/math_test.cpp +++ b/src/paimon/common/utils/math_test.cpp @@ -28,6 +28,39 @@ namespace paimon::test { +TEST(MathTest, FloatingPointNaNCanonicalization) { + auto float_from_bits = [](uint32_t bits) { + float value; + std::memcpy(&value, &bits, sizeof(value)); + return value; + }; + auto double_from_bits = [](uint64_t bits) { + double value; + std::memcpy(&value, &bits, sizeof(value)); + return value; + }; + + const float float_nan = CanonicalizeFloatingPoint(float_from_bits(0xffc12345)); + 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(float_from_bits(0x7fa12345))); + + const double double_nan = CanonicalizeFloatingPoint(double_from_bits(0xfff8123456789abc)); + 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(double_from_bits(0x7ff123456789abcd))); + + 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..fb3805579 100644 --- a/src/paimon/core/bucket/hive_bucket_function_test.cpp +++ b/src/paimon/core/bucket/hive_bucket_function_test.cpp @@ -24,6 +24,7 @@ #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" @@ -235,11 +236,11 @@ 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(FloatFromBits(kCanonicalFloatNaNBits)), 1000)); } TEST_F(HiveBucketFunctionTest, TestDoubleNaNCanonicalizationCompatibleWithJava) { @@ -248,10 +249,10 @@ 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(DoubleFromBits(kCanonicalDoubleNaNBits)), 1000)); } TEST_F(HiveBucketFunctionTest, TestTinyintNegativeValuesCompatibleWithJava) { From 96582710b046a22813fc35df39400d61dc939b67 Mon Sep 17 00:00:00 2001 From: "jinli.zjw" Date: Thu, 27 Aug 2026 19:35:59 +0800 Subject: [PATCH 2/6] fix(file-index): keep bitmap type support separate --- .../bitmap/bitmap_file_index_meta.cpp | 32 --- .../bitmap/bitmap_file_index_test.cpp | 253 ------------------ 2 files changed, 285 deletions(-) diff --git a/src/paimon/common/file_index/bitmap/bitmap_file_index_meta.cpp b/src/paimon/common/file_index/bitmap/bitmap_file_index_meta.cpp index 50b88af3c..64e05c650 100644 --- a/src/paimon/common/file_index/bitmap/bitmap_file_index_meta.cpp +++ b/src/paimon/common/file_index/bitmap/bitmap_file_index_meta.cpp @@ -18,13 +18,11 @@ #include "paimon/common/file_index/bitmap/bitmap_file_index_meta.h" -#include #include #include #include "fmt/format.h" #include "paimon/common/utils/field_type_utils.h" -#include "paimon/common/utils/math.h" #include "paimon/defs.h" #include "paimon/io/data_input_stream.h" #include "paimon/memory/bytes.h" @@ -82,18 +80,6 @@ Result> BitmapFileIndexMeta::GetValueWriter( [output_stream](const Literal& literal) -> void { output_stream->WriteValue(literal.GetValue()); }); - case FieldType::FLOAT: - return std::function( - [output_stream](const Literal& literal) -> void { - const float value = CanonicalizeFloatingPoint(literal.GetValue()); - output_stream->WriteValue(value); - }); - case FieldType::DOUBLE: - return std::function( - [output_stream](const Literal& literal) -> void { - const double value = CanonicalizeFloatingPoint(literal.GetValue()); - output_stream->WriteValue(value); - }); case FieldType::STRING: return std::function( [output_stream](const Literal& literal) -> void { @@ -169,24 +155,6 @@ Result()>> BitmapFileIndexMeta::GetValueReader( }; return func; } - case FieldType::FLOAT: { - std::function()> func = [&in, move_body_start, - this]() -> Result { - PAIMON_ASSIGN_OR_RAISE(float value, - ReadAndMoveBodyStart(in, move_body_start)); - return Literal(value); - }; - return func; - } - case FieldType::DOUBLE: { - std::function()> func = [&in, move_body_start, - this]() -> Result { - PAIMON_ASSIGN_OR_RAISE(double value, - ReadAndMoveBodyStart(in, move_body_start)); - return Literal(value); - }; - return func; - } case FieldType::DATE: { std::function()> func = [&in, move_body_start, this]() -> Result { diff --git a/src/paimon/common/file_index/bitmap/bitmap_file_index_test.cpp b/src/paimon/common/file_index/bitmap/bitmap_file_index_test.cpp index d0444e4c1..42a888a8b 100644 --- a/src/paimon/common/file_index/bitmap/bitmap_file_index_test.cpp +++ b/src/paimon/common/file_index/bitmap/bitmap_file_index_test.cpp @@ -18,7 +18,6 @@ #include "paimon/common/file_index/bitmap/bitmap_file_index.h" -#include #include #include "arrow/api.h" @@ -28,7 +27,6 @@ #include "paimon/common/utils/arrow/status_utils.h" #include "paimon/common/utils/checked_cast.h" #include "paimon/common/utils/date_time_utils.h" -#include "paimon/common/utils/math.h" #include "paimon/data/timestamp.h" #include "paimon/defs.h" #include "paimon/file_index/bitmap_index_result.h" @@ -38,23 +36,6 @@ #include "paimon/memory/memory_pool.h" #include "paimon/testing/utils/testharness.h" namespace paimon::test { -namespace { - -template -FloatingPoint FloatingPointFromBits(UInt bits) { - static_assert(sizeof(FloatingPoint) == sizeof(UInt)); - FloatingPoint value; - std::memcpy(&value, &bits, sizeof(value)); - return value; -} - -template -std::vector JavaBytes(const char (&bytes)[N]) { - return std::vector(bytes, bytes + N - 1); -} - -} // namespace - class BitmapIndexTest : public ::testing::Test { public: void SetUp() override { @@ -97,21 +78,6 @@ class BitmapIndexTest : public ::testing::Test { return writer->SerializedBytes(); } - template - Result> CreateArray(const std::shared_ptr& type, - const std::vector& values) const { - auto value_builder = std::make_shared(); - for (ValueType value : values) { - PAIMON_RETURN_NOT_OK_FROM_ARROW(value_builder->Append(value)); - } - PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(std::shared_ptr value_array, - value_builder->Finish()); - PAIMON_ASSIGN_OR_RAISE_FROM_ARROW( - std::shared_ptr struct_array, - arrow::StructArray::Make({value_array}, {arrow::field("f0", type)})); - return struct_array; - } - private: std::shared_ptr pool_; }; @@ -654,225 +620,6 @@ TEST_F(BitmapIndexTest, TestTimestampType) { } } -TEST_F(BitmapIndexTest, TestFloatAndDoubleTypes) { - const auto check_float = [&](int32_t version) { - const auto type = arrow::float32(); - auto array = - arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_({arrow::field("f0", type)}), - R"([[1.25], [null], [-2.5], [1.25], [3.75]])") - .ValueOrDie(); - ASSERT_OK_AND_ASSIGN(PAIMON_UNIQUE_PTR index_bytes, - WriteIndex(type, version, array)); - auto input_stream = - std::make_shared(index_bytes->data(), index_bytes->size()); - BitmapFileIndex file_index({}); - ASSERT_OK_AND_ASSIGN(auto reader, - file_index.CreateReader(CreateArrowSchema(type).get(), 0, - index_bytes->size(), input_stream, pool_)); - CheckResult(reader->VisitEqual(Literal(1.25f)).value(), {0, 3}); - CheckResult(reader->VisitNotEqual(Literal(1.25f)).value(), {2, 4}); - CheckResult(reader->VisitIsNull().value(), {1}); - }; - - const auto check_double = [&](int32_t version) { - const auto type = arrow::float64(); - auto array = - arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_({arrow::field("f0", type)}), - R"([[1.25], [null], [-2.5], [1.25], [3.75]])") - .ValueOrDie(); - ASSERT_OK_AND_ASSIGN(PAIMON_UNIQUE_PTR index_bytes, - WriteIndex(type, version, array)); - auto input_stream = - std::make_shared(index_bytes->data(), index_bytes->size()); - BitmapFileIndex file_index({}); - ASSERT_OK_AND_ASSIGN(auto reader, - file_index.CreateReader(CreateArrowSchema(type).get(), 0, - index_bytes->size(), input_stream, pool_)); - CheckResult(reader->VisitEqual(Literal(1.25)).value(), {0, 3}); - CheckResult(reader->VisitNotEqual(Literal(1.25)).value(), {2, 4}); - CheckResult(reader->VisitIsNull().value(), {1}); - }; - - for (int32_t version : {1, 2}) { - check_float(version); - check_double(version); - } -} - -TEST_F(BitmapIndexTest, TestFloatingPointJavaCompatibility) { - // Generated by BitmapFloatingPointCompatibilityTest with Apache Paimon Java at - // 0043a70fd88ac75dcb83a8f2da5e72ce91e22b1f. The Java writer receives canonical, - // positive-payload and negative-payload NaNs, followed by both signed zero values. - const std::vector java_float_v1 = JavaBytes( - "\x01\x00\x00\x00\x08\x00\x00\x00\x03\x00\x00\x00\x00\x00\x00\x00" - "\x00\x00\x80\x00\x00\x00\x00\x00\x00\x14\x7f\xc0\x00\x00\x00\x00" - "\x00\x28\x3a\x30\x00\x00\x01\x00\x00\x00\x00\x00\x01\x00\x10\x00" - "\x00\x00\x04\x00\x07\x00\x3a\x30\x00\x00\x01\x00\x00\x00\x00\x00" - "\x01\x00\x10\x00\x00\x00\x03\x00\x06\x00\x3a\x30\x00\x00\x01\x00" - "\x00\x00\x00\x00\x03\x00\x10\x00\x00\x00\x00\x00\x01\x00\x02\x00" - "\x05\x00"); - const std::vector java_float_v2 = JavaBytes( - "\x02\x00\x00\x00\x08\x00\x00\x00\x03\x00\x00\x00\x00\x01\x80\x00" - "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x28\x00\x00\x00\x03\x80\x00" - "\x00\x00\x00\x00\x00\x14\x00\x00\x00\x14\x00\x00\x00\x00\x00\x00" - "\x00\x00\x00\x00\x00\x14\x7f\xc0\x00\x00\x00\x00\x00\x28\x00\x00" - "\x00\x18\x3a\x30\x00\x00\x01\x00\x00\x00\x00\x00\x01\x00\x10\x00" - "\x00\x00\x04\x00\x07\x00\x3a\x30\x00\x00\x01\x00\x00\x00\x00\x00" - "\x01\x00\x10\x00\x00\x00\x03\x00\x06\x00\x3a\x30\x00\x00\x01\x00" - "\x00\x00\x00\x00\x03\x00\x10\x00\x00\x00\x00\x00\x01\x00\x02\x00" - "\x05\x00"); - const std::vector java_double_v1 = JavaBytes( - "\x01\x00\x00\x00\x08\x00\x00\x00\x03\x00\x00\x00\x00\x00\x00\x00" - "\x00\x00\x00\x00\x00\x00\x80\x00\x00\x00\x00\x00\x00\x00\x00\x00" - "\x00\x14\x7f\xf8\x00\x00\x00\x00\x00\x00\x00\x00\x00\x28\x3a\x30" - "\x00\x00\x01\x00\x00\x00\x00\x00\x01\x00\x10\x00\x00\x00\x04\x00" - "\x07\x00\x3a\x30\x00\x00\x01\x00\x00\x00\x00\x00\x01\x00\x10\x00" - "\x00\x00\x03\x00\x06\x00\x3a\x30\x00\x00\x01\x00\x00\x00\x00\x00" - "\x03\x00\x10\x00\x00\x00\x00\x00\x01\x00\x02\x00\x05\x00"); - const std::vector java_double_v2 = JavaBytes( - "\x02\x00\x00\x00\x08\x00\x00\x00\x03\x00\x00\x00\x00\x01\x80\x00" - "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x34\x00\x00" - "\x00\x03\x80\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x14\x00\x00" - "\x00\x14\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" - "\x00\x14\x7f\xf8\x00\x00\x00\x00\x00\x00\x00\x00\x00\x28\x00\x00" - "\x00\x18\x3a\x30\x00\x00\x01\x00\x00\x00\x00\x00\x01\x00\x10\x00" - "\x00\x00\x04\x00\x07\x00\x3a\x30\x00\x00\x01\x00\x00\x00\x00\x00" - "\x01\x00\x10\x00\x00\x00\x03\x00\x06\x00\x3a\x30\x00\x00\x01\x00" - "\x00\x00\x00\x00\x03\x00\x10\x00\x00\x00\x00\x00\x01\x00\x02\x00" - "\x05\x00"); - const std::vector java_float_nan_v1 = JavaBytes( - "\x01\x00\x00\x00\x03\x00\x00\x00\x01\x00\x7f\xc0\x00\x00\x00\x00" - "\x00\x00\x3a\x30\x00\x00\x01\x00\x00\x00\x00\x00\x02\x00\x10\x00" - "\x00\x00\x00\x00\x01\x00\x02\x00"); - const std::vector java_float_nan_v2 = JavaBytes( - "\x02\x00\x00\x00\x03\x00\x00\x00\x01\x00\x00\x00\x00\x01\x7f\xc0" - "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x10\x00\x00\x00\x01\x7f\xc0" - "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x16\x3a\x30\x00\x00\x01\x00" - "\x00\x00\x00\x00\x02\x00\x10\x00\x00\x00\x00\x00\x01\x00\x02\x00"); - const std::vector java_double_nan_v1 = JavaBytes( - "\x01\x00\x00\x00\x03\x00\x00\x00\x01\x00\x7f\xf8\x00\x00\x00\x00" - "\x00\x00\x00\x00\x00\x00\x3a\x30\x00\x00\x01\x00\x00\x00\x00\x00" - "\x02\x00\x10\x00\x00\x00\x00\x00\x01\x00\x02\x00"); - const std::vector java_double_nan_v2 = JavaBytes( - "\x02\x00\x00\x00\x03\x00\x00\x00\x01\x00\x00\x00\x00\x01\x7f\xf8" - "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x14\x00\x00" - "\x00\x01\x7f\xf8\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" - "\x00\x16\x3a\x30\x00\x00\x01\x00\x00\x00\x00\x00\x02\x00\x10\x00" - "\x00\x00\x00\x00\x01\x00\x02\x00"); - - const float float_nan = FloatingPointFromBits(kCanonicalFloatNaNBits); - const float float_positive_payload_nan = FloatingPointFromBits(uint32_t{0x7fc12345}); - const float float_negative_payload_nan = FloatingPointFromBits(uint32_t{0xffc54321}); - const std::vector float_values = {float_nan, - float_positive_payload_nan, - float_negative_payload_nan, - -0.0f, - +0.0f, - float_negative_payload_nan, - -0.0f, - +0.0f}; - ASSERT_OK_AND_ASSIGN(std::shared_ptr float_array, - (CreateArray(arrow::float32(), float_values))); - - const double double_nan = FloatingPointFromBits(kCanonicalDoubleNaNBits); - const double double_positive_payload_nan = - FloatingPointFromBits(uint64_t{0x7ff8123456789abc}); - const double double_negative_payload_nan = - FloatingPointFromBits(uint64_t{0xfff8abcdef012345}); - const std::vector double_values = {double_nan, - double_positive_payload_nan, - double_negative_payload_nan, - -0.0, - +0.0, - double_negative_payload_nan, - -0.0, - +0.0}; - ASSERT_OK_AND_ASSIGN(std::shared_ptr double_array, - (CreateArray(arrow::float64(), double_values))); - - const auto check_reader = [&](const std::shared_ptr& reader, - const std::vector& nan_literals, - const Literal& negative_zero, const Literal& positive_zero) { - for (const Literal& nan_literal : nan_literals) { - CheckResult(reader->VisitEqual(nan_literal).value(), {0, 1, 2, 5}); - } - CheckResult(reader->VisitEqual(negative_zero).value(), {3, 6}); - CheckResult(reader->VisitEqual(positive_zero).value(), {4, 7}); - }; - - const auto check = [&](const std::shared_ptr& type, - const std::shared_ptr& array, - const std::vector& nan_literals, const Literal& negative_zero, - const Literal& positive_zero, int32_t version, - const std::vector& java_bytes) { - auto input_stream = - std::make_shared(java_bytes.data(), java_bytes.size()); - BitmapFileIndex file_index({}); - ASSERT_OK_AND_ASSIGN(std::shared_ptr reader, - file_index.CreateReader(CreateArrowSchema(type).get(), 0, - java_bytes.size(), input_stream, pool_)); - check_reader(reader, nan_literals, negative_zero, positive_zero); - - ASSERT_OK_AND_ASSIGN(PAIMON_UNIQUE_PTR cpp_bytes, WriteIndex(type, version, array)); - auto cpp_input_stream = - std::make_shared(cpp_bytes->data(), cpp_bytes->size()); - ASSERT_OK_AND_ASSIGN(std::shared_ptr cpp_reader, - file_index.CreateReader(CreateArrowSchema(type).get(), 0, - cpp_bytes->size(), cpp_input_stream, pool_)); - check_reader(cpp_reader, nan_literals, negative_zero, positive_zero); - }; - - const auto check_nan_meta = [&](const std::shared_ptr& type, - const std::shared_ptr& array, int32_t version, - size_t key_size, const std::vector& java_bytes) { - ASSERT_OK_AND_ASSIGN(PAIMON_UNIQUE_PTR cpp_bytes, WriteIndex(type, version, array)); - // RoaringBitmap may choose a different, semantically equivalent body encoding. Compare - // the complete V1 meta, and the V2 meta through the entry offset (excluding only the body - // length derived from that encoding). This covers both serialized copies of the V2 key. - const size_t comparable_meta_size = - version == BitmapFileIndex::VERSION_1 ? 14 + key_size : 30 + 2 * key_size; - ASSERT_GE(java_bytes.size(), comparable_meta_size); - ASSERT_GE(cpp_bytes->size(), comparable_meta_size); - ASSERT_EQ(std::vector(java_bytes.begin(), java_bytes.begin() + comparable_meta_size), - std::vector(cpp_bytes->data(), cpp_bytes->data() + comparable_meta_size)); - }; - - check(arrow::float32(), float_array, - {Literal(float_nan), Literal(float_positive_payload_nan), - Literal(float_negative_payload_nan)}, - Literal(-0.0f), Literal(+0.0f), /*version=*/1, java_float_v1); - check(arrow::float32(), float_array, - {Literal(float_nan), Literal(float_positive_payload_nan), - Literal(float_negative_payload_nan)}, - Literal(-0.0f), Literal(+0.0f), /*version=*/2, java_float_v2); - check(arrow::float64(), double_array, - {Literal(double_nan), Literal(double_positive_payload_nan), - Literal(double_negative_payload_nan)}, - Literal(-0.0), Literal(+0.0), /*version=*/1, java_double_v1); - check(arrow::float64(), double_array, - {Literal(double_nan), Literal(double_positive_payload_nan), - Literal(double_negative_payload_nan)}, - Literal(-0.0), Literal(+0.0), /*version=*/2, java_double_v2); - - const std::vector float_nan_values = {float_negative_payload_nan, - float_positive_payload_nan, float_nan}; - ASSERT_OK_AND_ASSIGN(std::shared_ptr float_nan_array, - (CreateArray(arrow::float32(), float_nan_values))); - check_nan_meta(arrow::float32(), float_nan_array, /*version=*/1, sizeof(float), - java_float_nan_v1); - check_nan_meta(arrow::float32(), float_nan_array, /*version=*/2, sizeof(float), - java_float_nan_v2); - - const std::vector double_nan_values = {double_negative_payload_nan, - double_positive_payload_nan, double_nan}; - ASSERT_OK_AND_ASSIGN(std::shared_ptr double_nan_array, - (CreateArray(arrow::float64(), double_nan_values))); - check_nan_meta(arrow::float64(), double_nan_array, /*version=*/1, sizeof(double), - java_double_nan_v1); - check_nan_meta(arrow::float64(), double_nan_array, /*version=*/2, sizeof(double), - java_double_nan_v2); -} - TEST_F(BitmapIndexTest, TestHighCardinalityForCompatibility) { auto type = arrow::utf8(); auto check_result = [&](const std::string& index_file_name) { From 5b403eb375ef44ded81e1528c223d4bb55928aaa Mon Sep 17 00:00:00 2001 From: "jinli.zjw" Date: Fri, 28 Aug 2026 10:03:23 +0800 Subject: [PATCH 3/6] fix: canonicalize NaN in binary serializers --- .../data/variant/generic_variant_test.cpp | 31 +++++++++++++++ .../common/data/variant/variant_builder.cpp | 7 ++-- .../global_index/btree/key_serializer.cpp | 11 ++---- .../btree/key_serializer_test.cpp | 39 +++++++++++++++++++ .../global_index/global_index_result.cpp | 5 ++- .../global_index/global_index_result_test.cpp | 25 ++++++++++++ .../core/global_index/indexed_split_test.cpp | 23 +++++++++++ src/paimon/core/table/source/split.cpp | 5 ++- 8 files changed, 130 insertions(+), 16 deletions(-) diff --git a/src/paimon/common/data/variant/generic_variant_test.cpp b/src/paimon/common/data/variant/generic_variant_test.cpp index 11386a28a..b0887fa77 100644 --- a/src/paimon/common/data/variant/generic_variant_test.cpp +++ b/src/paimon/common/data/variant/generic_variant_test.cpp @@ -19,6 +19,8 @@ #include "paimon/common/data/variant/generic_variant.h" +#include +#include #include #include #include @@ -31,6 +33,17 @@ #include "paimon/testing/utils/testharness.h" namespace paimon::test { +namespace { + +template +FloatingPoint FloatingPointFromBits(UInt bits) { + static_assert(sizeof(FloatingPoint) == sizeof(UInt)); + FloatingPoint value; + std::memcpy(&value, &bits, sizeof(value)); + return value; +} + +} // namespace class GenericVariantTest : public ::testing::Test { public: @@ -350,6 +363,24 @@ TEST_F(GenericVariantTest, NonFiniteDoubleToJson) { ASSERT_EQ(json, "\"Infinity\""); } +TEST_F(GenericVariantTest, CanonicalizesFloatingPointNaN) { + { + VariantBuilder builder(false); + ASSERT_OK(builder.AppendFloat(FloatingPointFromBits(uint32_t{0xffc12345}))); + 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(uint64_t{0xfff8123456789abc}))); + 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/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..b3d86d3e8 100644 --- a/src/paimon/common/global_index/btree/key_serializer_test.cpp +++ b/src/paimon/common/global_index/btree/key_serializer_test.cpp @@ -19,12 +19,27 @@ #include "paimon/common/global_index/btree/key_serializer.h" +#include +#include +#include + #include "gtest/gtest.h" #include "paimon/data/decimal.h" #include "paimon/data/timestamp.h" #include "paimon/testing/utils/testharness.h" namespace paimon::test { +namespace { + +template +FloatingPoint FloatingPointFromBits(UInt bits) { + static_assert(sizeof(FloatingPoint) == sizeof(UInt)); + FloatingPoint value; + std::memcpy(&value, &bits, sizeof(value)); + return value; +} + +} // namespace class KeySerializerTest : public ::testing::Test { protected: @@ -208,6 +223,30 @@ TEST_F(KeySerializerTest, SerializeAndDeserializeAllTypes) { } } +TEST_F(KeySerializerTest, CanonicalizesFloatingPointNaN) { + const float float_nan = FloatingPointFromBits(uint32_t{0xffc12345}); + const float canonical_float_nan = FloatingPointFromBits(uint32_t{0x7fc00000}); + 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 double double_nan = FloatingPointFromBits(uint64_t{0xfff8123456789abc}); + const double canonical_double_nan = FloatingPointFromBits(uint64_t{0x7ff8000000000000}); + 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..628e8e339 100644 --- a/src/paimon/common/global_index/global_index_result_test.cpp +++ b/src/paimon/common/global_index/global_index_result_test.cpp @@ -19,6 +19,9 @@ #include "paimon/global_index/global_index_result.h" +#include +#include +#include #include #include "gtest/gtest.h" @@ -144,6 +147,28 @@ TEST_F(GlobalIndexResultTest, TestSerializeAndDeserializeWithScore) { serialize_bytes->data() + serialize_bytes->size())); } +TEST_F(GlobalIndexResultTest, TestSerializeCanonicalizesNaNScore) { + auto pool = GetDefaultPool(); + uint32_t payload_bits = 0xffc12345; + float payload_nan; + std::memcpy(&payload_nan, &payload_bits, sizeof(payload_nan)); + auto index_result = std::make_shared( + RoaringBitmap64::From({1}), std::vector{payload_nan}); + + ASSERT_OK_AND_ASSIGN(PAIMON_UNIQUE_PTR serialized, + GlobalIndexResult::Serialize(index_result, pool)); + ASSERT_GE(serialized->size(), sizeof(float)); + ASSERT_EQ(std::string(serialized->data() + serialized->size() - sizeof(float), sizeof(float)), + std::string("\x7f\xc0\x00\x00", sizeof(float))); + + 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/core/global_index/indexed_split_test.cpp b/src/paimon/core/global_index/indexed_split_test.cpp index 7cd121257..a91ef09b4 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 @@ -155,6 +157,27 @@ 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()); + + uint32_t payload_bits = 0xffc12345; + float payload_nan; + std::memcpy(&payload_nan, &payload_bits, sizeof(payload_nan)); + auto indexed_split = std::make_shared( + std::dynamic_pointer_cast(data_split), std::vector{Range(0, 0)}, + std::vector{payload_nan}); + + ASSERT_OK_AND_ASSIGN(std::string serialized, Split::Serialize(indexed_split, pool)); + ASSERT_GE(serialized.size(), sizeof(float)); + ASSERT_EQ(serialized.substr(serialized.size() - sizeof(float)), + std::string("\x7f\xc0\x00\x00", sizeof(float))); +} + 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); From 10531d59a1c505c30d3fba829bcb6c2f9325e1ce Mon Sep 17 00:00:00 2001 From: "jinli.zjw" Date: Fri, 28 Aug 2026 10:39:11 +0800 Subject: [PATCH 4/6] refactor: share floating-point bit conversion --- .../data/variant/generic_variant_test.cpp | 18 +++---------- .../btree/key_serializer_test.cpp | 21 ++++----------- .../global_index/global_index_result_test.cpp | 6 ++--- src/paimon/common/utils/math.h | 14 ++++++++-- src/paimon/common/utils/math_test.cpp | 20 ++++---------- .../core/bucket/hive_bucket_function_test.cpp | 26 +++++++------------ .../core/global_index/indexed_split_test.cpp | 6 ++--- 7 files changed, 38 insertions(+), 73 deletions(-) diff --git a/src/paimon/common/data/variant/generic_variant_test.cpp b/src/paimon/common/data/variant/generic_variant_test.cpp index b0887fa77..bc46a5730 100644 --- a/src/paimon/common/data/variant/generic_variant_test.cpp +++ b/src/paimon/common/data/variant/generic_variant_test.cpp @@ -20,7 +20,6 @@ #include "paimon/common/data/variant/generic_variant.h" #include -#include #include #include #include @@ -29,21 +28,11 @@ #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" namespace paimon::test { -namespace { - -template -FloatingPoint FloatingPointFromBits(UInt bits) { - static_assert(sizeof(FloatingPoint) == sizeof(UInt)); - FloatingPoint value; - std::memcpy(&value, &bits, sizeof(value)); - return value; -} - -} // namespace class GenericVariantTest : public ::testing::Test { public: @@ -366,15 +355,14 @@ TEST_F(GenericVariantTest, NonFiniteDoubleToJson) { TEST_F(GenericVariantTest, CanonicalizesFloatingPointNaN) { { VariantBuilder builder(false); - ASSERT_OK(builder.AppendFloat(FloatingPointFromBits(uint32_t{0xffc12345}))); + 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(uint64_t{0xfff8123456789abc}))); + 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"); 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 b3d86d3e8..cf191c3b1 100644 --- a/src/paimon/common/global_index/btree/key_serializer_test.cpp +++ b/src/paimon/common/global_index/btree/key_serializer_test.cpp @@ -20,26 +20,15 @@ #include "paimon/common/global_index/btree/key_serializer.h" #include -#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" namespace paimon::test { -namespace { - -template -FloatingPoint FloatingPointFromBits(UInt bits) { - static_assert(sizeof(FloatingPoint) == sizeof(UInt)); - FloatingPoint value; - std::memcpy(&value, &bits, sizeof(value)); - return value; -} - -} // namespace class KeySerializerTest : public ::testing::Test { protected: @@ -224,8 +213,8 @@ TEST_F(KeySerializerTest, SerializeAndDeserializeAllTypes) { } TEST_F(KeySerializerTest, CanonicalizesFloatingPointNaN) { - const float float_nan = FloatingPointFromBits(uint32_t{0xffc12345}); - const float canonical_float_nan = FloatingPointFromBits(uint32_t{0x7fc00000}); + const float float_nan = FloatingPointFromBits(0xffc12345U); + const float canonical_float_nan = FloatingPointFromBits(kCanonicalFloatNaNBits); ASSERT_OK_AND_ASSIGN( std::shared_ptr float_bytes, KeySerializer::SerializeKey(Literal(float_nan), arrow::float32(), pool_.get())); @@ -235,8 +224,8 @@ TEST_F(KeySerializerTest, CanonicalizesFloatingPointNaN) { ASSERT_EQ(std::string(float_bytes->data(), float_bytes->size()), std::string(canonical_float_bytes->data(), canonical_float_bytes->size())); - const double double_nan = FloatingPointFromBits(uint64_t{0xfff8123456789abc}); - const double canonical_double_nan = FloatingPointFromBits(uint64_t{0x7ff8000000000000}); + const double double_nan = FloatingPointFromBits(0xfff8123456789abcULL); + const double canonical_double_nan = FloatingPointFromBits(kCanonicalDoubleNaNBits); ASSERT_OK_AND_ASSIGN( std::shared_ptr double_bytes, KeySerializer::SerializeKey(Literal(double_nan), arrow::float64(), pool_.get())); 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 628e8e339..00249a221 100644 --- a/src/paimon/common/global_index/global_index_result_test.cpp +++ b/src/paimon/common/global_index/global_index_result_test.cpp @@ -21,10 +21,10 @@ #include #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" @@ -149,9 +149,7 @@ TEST_F(GlobalIndexResultTest, TestSerializeAndDeserializeWithScore) { TEST_F(GlobalIndexResultTest, TestSerializeCanonicalizesNaNScore) { auto pool = GetDefaultPool(); - uint32_t payload_bits = 0xffc12345; - float payload_nan; - std::memcpy(&payload_nan, &payload_bits, sizeof(payload_nan)); + const float payload_nan = FloatingPointFromBits(0xffc12345U); auto index_result = std::make_shared( RoaringBitmap64::From({1}), std::vector{payload_nan}); diff --git a/src/paimon/common/utils/math.h b/src/paimon/common/utils/math.h index 787353fda..b9e2e3103 100644 --- a/src/paimon/common/utils/math.h +++ b/src/paimon/common/utils/math.h @@ -45,16 +45,26 @@ 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)) { - std::memcpy(&value, &kCanonicalFloatNaNBits, sizeof(value)); + return FloatingPointFromBits(kCanonicalFloatNaNBits); } return value; } inline double CanonicalizeFloatingPoint(double value) { if (std::isnan(value)) { - std::memcpy(&value, &kCanonicalDoubleNaNBits, sizeof(value)); + return FloatingPointFromBits(kCanonicalDoubleNaNBits); } return value; } diff --git a/src/paimon/common/utils/math_test.cpp b/src/paimon/common/utils/math_test.cpp index 343f12831..c8a5c8e05 100644 --- a/src/paimon/common/utils/math_test.cpp +++ b/src/paimon/common/utils/math_test.cpp @@ -29,30 +29,20 @@ namespace paimon::test { TEST(MathTest, FloatingPointNaNCanonicalization) { - auto float_from_bits = [](uint32_t bits) { - float value; - std::memcpy(&value, &bits, sizeof(value)); - return value; - }; - auto double_from_bits = [](uint64_t bits) { - double value; - std::memcpy(&value, &bits, sizeof(value)); - return value; - }; - - const float float_nan = CanonicalizeFloatingPoint(float_from_bits(0xffc12345)); + const float 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(float_from_bits(0x7fa12345))); + CanonicalizeFloatToIntBits(FloatingPointFromBits(0x7fa12345U))); - const double double_nan = CanonicalizeFloatingPoint(double_from_bits(0xfff8123456789abc)); + const double 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(double_from_bits(0x7ff123456789abcd))); + CanonicalizeDoubleToLongBits(FloatingPointFromBits(0x7ff123456789abcdULL))); const float negative_zero = CanonicalizeFloatingPoint(-0.0f); uint32_t negative_zero_bits; diff --git a/src/paimon/core/bucket/hive_bucket_function_test.cpp b/src/paimon/core/bucket/hive_bucket_function_test.cpp index fb3805579..d97a0294b 100644 --- a/src/paimon/core/bucket/hive_bucket_function_test.cpp +++ b/src/paimon/core/bucket/hive_bucket_function_test.cpp @@ -18,7 +18,6 @@ #include "paimon/core/bucket/hive_bucket_function.h" -#include #include #include "gtest/gtest.h" @@ -112,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 @@ -239,8 +226,9 @@ TEST_F(HiveBucketFunctionTest, TestFloatNaNCanonicalizationCompatibleWithJava) { // 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(kCanonicalFloatNaNBits)), 1000)); + ASSERT_EQ(344, func->Bucket(CreateFloatRow(FloatingPointFromBits(0x7FA12345U)), 1000)); + ASSERT_EQ(344, func->Bucket( + CreateFloatRow(FloatingPointFromBits(kCanonicalFloatNaNBits)), 1000)); } TEST_F(HiveBucketFunctionTest, TestDoubleNaNCanonicalizationCompatibleWithJava) { @@ -251,8 +239,12 @@ TEST_F(HiveBucketFunctionTest, TestDoubleNaNCanonicalizationCompatibleWithJava) // Double.NaN, Double.longBitsToDouble(0x7ff123456789abcd), and canonical NaN // 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(kCanonicalDoubleNaNBits)), 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 a91ef09b4..1d1b10ef2 100644 --- a/src/paimon/core/global_index/indexed_split_test.cpp +++ b/src/paimon/core/global_index/indexed_split_test.cpp @@ -18,7 +18,6 @@ */ #include -#include #include #include #include @@ -28,6 +27,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" @@ -165,9 +165,7 @@ TEST(IndexedSplitTest, TestSerializeCanonicalizesNaNScore) { /*data_files=*/{}); ASSERT_OK_AND_ASSIGN(std::shared_ptr data_split, builder.Build()); - uint32_t payload_bits = 0xffc12345; - float payload_nan; - std::memcpy(&payload_nan, &payload_bits, sizeof(payload_nan)); + const float payload_nan = FloatingPointFromBits(0xffc12345U); auto indexed_split = std::make_shared( std::dynamic_pointer_cast(data_split), std::vector{Range(0, 0)}, std::vector{payload_nan}); From 9d8267c93c49cee26ade4bd987ec4720766a466c Mon Sep 17 00:00:00 2001 From: "jinli.zjw" Date: Fri, 28 Aug 2026 11:20:27 +0800 Subject: [PATCH 5/6] test: reuse floating-point bit conversion helper --- .../file_index/bloomfilter/fast_hash_test.cpp | 20 +++++-------------- 1 file changed, 5 insertions(+), 15 deletions(-) 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..b58aed6c0 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 float float_nan = FloatingPointFromBits(0x7fc12345U); + const float 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 double double_nan = FloatingPointFromBits(0x7ff8123456789abcULL); + const double 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())); From 3c39a1ef49e66c7eecc1f4297c75f95362787d1f Mon Sep 17 00:00:00 2001 From: "jinli.zjw" Date: Fri, 28 Aug 2026 14:31:53 +0800 Subject: [PATCH 6/6] test: strengthen NaN serialization coverage --- .../file_index/bloomfilter/fast_hash_test.cpp | 8 +++---- .../btree/key_serializer_test.cpp | 8 +++---- .../global_index/global_index_result_test.cpp | 11 +++++---- src/paimon/common/utils/math_test.cpp | 4 ++-- .../core/global_index/indexed_split_test.cpp | 24 +++++++++++++------ 5 files changed, 34 insertions(+), 21 deletions(-) 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 b58aed6c0..6ffbc7964 100644 --- a/src/paimon/common/file_index/bloomfilter/fast_hash_test.cpp +++ b/src/paimon/common/file_index/bloomfilter/fast_hash_test.cpp @@ -168,16 +168,16 @@ TEST_F(FastHashTest, TestCompatibleWithJava) { } TEST_F(FastHashTest, TestNaNCompatibleWithJava) { - const float float_nan = FloatingPointFromBits(0x7fc12345U); - const float negative_float_nan = FloatingPointFromBits(0xffc54321U); + 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}); - const double double_nan = FloatingPointFromBits(0x7ff8123456789abcULL); - const double negative_double_nan = FloatingPointFromBits(0xfff8abcdef012345ULL); + 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_test.cpp b/src/paimon/common/global_index/btree/key_serializer_test.cpp index cf191c3b1..61322fde4 100644 --- a/src/paimon/common/global_index/btree/key_serializer_test.cpp +++ b/src/paimon/common/global_index/btree/key_serializer_test.cpp @@ -213,8 +213,8 @@ TEST_F(KeySerializerTest, SerializeAndDeserializeAllTypes) { } TEST_F(KeySerializerTest, CanonicalizesFloatingPointNaN) { - const float float_nan = FloatingPointFromBits(0xffc12345U); - const float canonical_float_nan = FloatingPointFromBits(kCanonicalFloatNaNBits); + 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())); @@ -224,8 +224,8 @@ TEST_F(KeySerializerTest, CanonicalizesFloatingPointNaN) { ASSERT_EQ(std::string(float_bytes->data(), float_bytes->size()), std::string(canonical_float_bytes->data(), canonical_float_bytes->size())); - const double double_nan = FloatingPointFromBits(0xfff8123456789abcULL); - const double canonical_double_nan = FloatingPointFromBits(kCanonicalDoubleNaNBits); + 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())); 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 00249a221..3179e6710 100644 --- a/src/paimon/common/global_index/global_index_result_test.cpp +++ b/src/paimon/common/global_index/global_index_result_test.cpp @@ -149,15 +149,18 @@ TEST_F(GlobalIndexResultTest, TestSerializeAndDeserializeWithScore) { TEST_F(GlobalIndexResultTest, TestSerializeCanonicalizesNaNScore) { auto pool = GetDefaultPool(); - const float payload_nan = FloatingPointFromBits(0xffc12345U); + 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_GE(serialized->size(), sizeof(float)); - ASSERT_EQ(std::string(serialized->data() + serialized->size() - sizeof(float), sizeof(float)), - std::string("\x7f\xc0\x00\x00", sizeof(float))); + 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, diff --git a/src/paimon/common/utils/math_test.cpp b/src/paimon/common/utils/math_test.cpp index c8a5c8e05..ce9379f6c 100644 --- a/src/paimon/common/utils/math_test.cpp +++ b/src/paimon/common/utils/math_test.cpp @@ -29,14 +29,14 @@ namespace paimon::test { TEST(MathTest, FloatingPointNaNCanonicalization) { - const float float_nan = CanonicalizeFloatingPoint(FloatingPointFromBits(0xffc12345U)); + 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 double double_nan = + const auto double_nan = CanonicalizeFloatingPoint(FloatingPointFromBits(0xfff8123456789abcULL)); uint64_t double_nan_bits; std::memcpy(&double_nan_bits, &double_nan, sizeof(double_nan_bits)); diff --git a/src/paimon/core/global_index/indexed_split_test.cpp b/src/paimon/core/global_index/indexed_split_test.cpp index 1d1b10ef2..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,7 @@ * under the License. */ +#include #include #include #include @@ -163,17 +164,26 @@ TEST(IndexedSplitTest, TestSerializeCanonicalizesNaNScore) { /*partition=*/BinaryRow::EmptyRow(), /*bucket=*/0, /*bucket_path=*/"bucket-0", /*data_files=*/{}); - ASSERT_OK_AND_ASSIGN(std::shared_ptr data_split, builder.Build()); + ASSERT_OK_AND_ASSIGN(std::shared_ptr data_split, builder.Build()); - const float payload_nan = FloatingPointFromBits(0xffc12345U); + const auto payload_nan = FloatingPointFromBits(0xffc12345U); + const auto canonical_nan = FloatingPointFromBits(kCanonicalFloatNaNBits); auto indexed_split = std::make_shared( - std::dynamic_pointer_cast(data_split), std::vector{Range(0, 0)}, - std::vector{payload_nan}); + 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_GE(serialized.size(), sizeof(float)); - ASSERT_EQ(serialized.substr(serialized.size() - sizeof(float)), - std::string("\x7f\xc0\x00\x00", sizeof(float))); + 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) {