Skip to content

Commit 5b403eb

Browse files
committed
fix: canonicalize NaN in binary serializers
1 parent 9658271 commit 5b403eb

8 files changed

Lines changed: 130 additions & 16 deletions

File tree

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
#include "paimon/common/data/variant/generic_variant.h"
2121

22+
#include <cstdint>
23+
#include <cstring>
2224
#include <functional>
2325
#include <limits>
2426
#include <string>
@@ -31,6 +33,17 @@
3133
#include "paimon/testing/utils/testharness.h"
3234

3335
namespace paimon::test {
36+
namespace {
37+
38+
template <typename FloatingPoint, typename UInt>
39+
FloatingPoint FloatingPointFromBits(UInt bits) {
40+
static_assert(sizeof(FloatingPoint) == sizeof(UInt));
41+
FloatingPoint value;
42+
std::memcpy(&value, &bits, sizeof(value));
43+
return value;
44+
}
45+
46+
} // namespace
3447

3548
class GenericVariantTest : public ::testing::Test {
3649
public:
@@ -350,6 +363,24 @@ TEST_F(GenericVariantTest, NonFiniteDoubleToJson) {
350363
ASSERT_EQ(json, "\"Infinity\"");
351364
}
352365

366+
TEST_F(GenericVariantTest, CanonicalizesFloatingPointNaN) {
367+
{
368+
VariantBuilder builder(false);
369+
ASSERT_OK(builder.AppendFloat(FloatingPointFromBits<float>(uint32_t{0xffc12345})));
370+
ASSERT_OK_AND_ASSIGN(std::shared_ptr<GenericVariant> variant, builder.Build(pool_));
371+
ASSERT_OK_AND_ASSIGN(std::string_view value, variant->Value());
372+
ASSERT_EQ(ToHex(value), "380000c07f");
373+
}
374+
{
375+
VariantBuilder builder(false);
376+
ASSERT_OK(
377+
builder.AppendDouble(FloatingPointFromBits<double>(uint64_t{0xfff8123456789abc})));
378+
ASSERT_OK_AND_ASSIGN(std::shared_ptr<GenericVariant> variant, builder.Build(pool_));
379+
ASSERT_OK_AND_ASSIGN(std::string_view value, variant->Value());
380+
ASSERT_EQ(ToHex(value), "1c000000000000f87f");
381+
}
382+
}
383+
353384
TEST_F(GenericVariantTest, GetTypeInfoReturnsHeaderBits) {
354385
// GetTypeInfo exposes the primitive header's type-info bits; 42 is encoded as an int1.
355386
auto v = FromJson("42");

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
#include "fmt/format.h"
3232
#include "paimon/common/data/variant/variant_defs.h"
33+
#include "paimon/common/utils/math.h"
3334
#include "rapidjson/error/en.h"
3435
#include "rapidjson/memorystream.h"
3536
#include "rapidjson/reader.h"
@@ -339,8 +340,7 @@ Status VariantBuilder::AppendLong(int64_t l) {
339340
Status VariantBuilder::AppendDouble(double d) {
340341
PAIMON_RETURN_NOT_OK(CheckCapacity(1 + 8));
341342
write_buffer_[write_pos_++] = VariantBinaryUtil::PrimitiveHeader(VariantDefs::kDouble);
342-
int64_t bits;
343-
memcpy(&bits, &d, sizeof(bits));
343+
const int64_t bits = CanonicalizeDoubleToLongBits(d);
344344
VariantBinaryUtil::WriteLong(bits, 8, write_buffer_.data(), write_pos_);
345345
write_pos_ += 8;
346346
return Status::OK();
@@ -409,8 +409,7 @@ Status VariantBuilder::AppendTimestampNtz(int64_t micros_since_epoch) {
409409
Status VariantBuilder::AppendFloat(float f) {
410410
PAIMON_RETURN_NOT_OK(CheckCapacity(1 + 4));
411411
write_buffer_[write_pos_++] = VariantBinaryUtil::PrimitiveHeader(VariantDefs::kFloat);
412-
int32_t bits;
413-
memcpy(&bits, &f, sizeof(bits));
412+
const int32_t bits = CanonicalizeFloatToIntBits(f);
414413
VariantBinaryUtil::WriteLong(bits, 4, write_buffer_.data(), write_pos_);
415414
write_pos_ += 4;
416415
return Status::OK();

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

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "paimon/common/utils/date_time_utils.h"
2828
#include "paimon/common/utils/field_type_utils.h"
2929
#include "paimon/common/utils/fields_comparator.h"
30+
#include "paimon/common/utils/math.h"
3031
#include "paimon/common/utils/preconditions.h"
3132
#include "paimon/common/utils/var_length_int_utils.h"
3233
#include "paimon/data/decimal.h"
@@ -164,19 +165,13 @@ Result<std::shared_ptr<Bytes>> KeySerializer::SerializeKey(
164165
case FieldType::FLOAT: {
165166
MemorySliceOutput output(4, pool);
166167
output.Reset();
167-
auto fvalue = literal.GetValue<float>();
168-
int32_t ivalue;
169-
memcpy(&ivalue, &fvalue, sizeof(float));
170-
output.WriteValue<int32_t>(ivalue);
168+
output.WriteValue<int32_t>(CanonicalizeFloatToIntBits(literal.GetValue<float>()));
171169
return output.ToSlice().CopyBytes(pool);
172170
}
173171
case FieldType::DOUBLE: {
174172
MemorySliceOutput output(8, pool);
175173
output.Reset();
176-
auto dvalue = literal.GetValue<double>();
177-
int64_t ivalue;
178-
memcpy(&ivalue, &dvalue, sizeof(double));
179-
output.WriteValue<int64_t>(ivalue);
174+
output.WriteValue<int64_t>(CanonicalizeDoubleToLongBits(literal.GetValue<double>()));
180175
return output.ToSlice().CopyBytes(pool);
181176
}
182177
case FieldType::STRING: {

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,27 @@
1919

2020
#include "paimon/common/global_index/btree/key_serializer.h"
2121

22+
#include <cstdint>
23+
#include <cstring>
24+
#include <string>
25+
2226
#include "gtest/gtest.h"
2327
#include "paimon/data/decimal.h"
2428
#include "paimon/data/timestamp.h"
2529
#include "paimon/testing/utils/testharness.h"
2630

2731
namespace paimon::test {
32+
namespace {
33+
34+
template <typename FloatingPoint, typename UInt>
35+
FloatingPoint FloatingPointFromBits(UInt bits) {
36+
static_assert(sizeof(FloatingPoint) == sizeof(UInt));
37+
FloatingPoint value;
38+
std::memcpy(&value, &bits, sizeof(value));
39+
return value;
40+
}
41+
42+
} // namespace
2843

2944
class KeySerializerTest : public ::testing::Test {
3045
protected:
@@ -208,6 +223,30 @@ TEST_F(KeySerializerTest, SerializeAndDeserializeAllTypes) {
208223
}
209224
}
210225

226+
TEST_F(KeySerializerTest, CanonicalizesFloatingPointNaN) {
227+
const float float_nan = FloatingPointFromBits<float>(uint32_t{0xffc12345});
228+
const float canonical_float_nan = FloatingPointFromBits<float>(uint32_t{0x7fc00000});
229+
ASSERT_OK_AND_ASSIGN(
230+
std::shared_ptr<Bytes> float_bytes,
231+
KeySerializer::SerializeKey(Literal(float_nan), arrow::float32(), pool_.get()));
232+
ASSERT_OK_AND_ASSIGN(
233+
std::shared_ptr<Bytes> canonical_float_bytes,
234+
KeySerializer::SerializeKey(Literal(canonical_float_nan), arrow::float32(), pool_.get()));
235+
ASSERT_EQ(std::string(float_bytes->data(), float_bytes->size()),
236+
std::string(canonical_float_bytes->data(), canonical_float_bytes->size()));
237+
238+
const double double_nan = FloatingPointFromBits<double>(uint64_t{0xfff8123456789abc});
239+
const double canonical_double_nan = FloatingPointFromBits<double>(uint64_t{0x7ff8000000000000});
240+
ASSERT_OK_AND_ASSIGN(
241+
std::shared_ptr<Bytes> double_bytes,
242+
KeySerializer::SerializeKey(Literal(double_nan), arrow::float64(), pool_.get()));
243+
ASSERT_OK_AND_ASSIGN(
244+
std::shared_ptr<Bytes> canonical_double_bytes,
245+
KeySerializer::SerializeKey(Literal(canonical_double_nan), arrow::float64(), pool_.get()));
246+
ASSERT_EQ(std::string(double_bytes->data(), double_bytes->size()),
247+
std::string(canonical_double_bytes->data(), canonical_double_bytes->size()));
248+
}
249+
211250
TEST_F(KeySerializerTest, RejectsMalformedSerializedKeys) {
212251
auto wrap = [this](const std::string& value) {
213252
return MemorySlice::Wrap(std::make_shared<Bytes>(value, pool_.get()));

src/paimon/common/global_index/global_index_result.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "fmt/format.h"
2323
#include "paimon/common/io/memory_segment_output_stream.h"
2424
#include "paimon/common/memory/memory_segment_utils.h"
25+
#include "paimon/common/utils/math.h"
2526
#include "paimon/global_index/bitmap_global_index_result.h"
2627
#include "paimon/global_index/bitmap_scored_global_index_result.h"
2728
#include "paimon/io/byte_array_input_stream.h"
@@ -37,8 +38,8 @@ void WriteBitmapAndScores(const RoaringBitmap64* bitmap, const std::vector<float
3738
out->WriteBytes(bitmap_bytes);
3839

3940
out->WriteValue<int32_t>(scores.size());
40-
for (auto score : scores) {
41-
out->WriteValue<float>(score);
41+
for (float score : scores) {
42+
out->WriteValue<float>(CanonicalizeFloatingPoint(score));
4243
}
4344
}
4445

src/paimon/common/global_index/global_index_result_test.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919

2020
#include "paimon/global_index/global_index_result.h"
2121

22+
#include <cmath>
23+
#include <cstdint>
24+
#include <cstring>
2225
#include <utility>
2326

2427
#include "gtest/gtest.h"
@@ -144,6 +147,28 @@ TEST_F(GlobalIndexResultTest, TestSerializeAndDeserializeWithScore) {
144147
serialize_bytes->data() + serialize_bytes->size()));
145148
}
146149

150+
TEST_F(GlobalIndexResultTest, TestSerializeCanonicalizesNaNScore) {
151+
auto pool = GetDefaultPool();
152+
uint32_t payload_bits = 0xffc12345;
153+
float payload_nan;
154+
std::memcpy(&payload_nan, &payload_bits, sizeof(payload_nan));
155+
auto index_result = std::make_shared<BitmapScoredGlobalIndexResult>(
156+
RoaringBitmap64::From({1}), std::vector<float>{payload_nan});
157+
158+
ASSERT_OK_AND_ASSIGN(PAIMON_UNIQUE_PTR<Bytes> serialized,
159+
GlobalIndexResult::Serialize(index_result, pool));
160+
ASSERT_GE(serialized->size(), sizeof(float));
161+
ASSERT_EQ(std::string(serialized->data() + serialized->size() - sizeof(float), sizeof(float)),
162+
std::string("\x7f\xc0\x00\x00", sizeof(float)));
163+
164+
ASSERT_OK_AND_ASSIGN(
165+
std::shared_ptr<GlobalIndexResult> deserialized,
166+
GlobalIndexResult::Deserialize(serialized->data(), serialized->size(), pool));
167+
auto scored_result = std::dynamic_pointer_cast<BitmapScoredGlobalIndexResult>(deserialized);
168+
ASSERT_TRUE(scored_result);
169+
ASSERT_TRUE(std::isnan(scored_result->GetScores()[0]));
170+
}
171+
147172
TEST_F(GlobalIndexResultTest, TestInvalidSerialize) {
148173
auto pool = GetDefaultPool();
149174
auto result = std::make_shared<FakeGlobalIndexResult>(std::vector<int64_t>({1, 3, 5, 100}));

src/paimon/core/global_index/indexed_split_test.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
* under the License.
1818
*/
1919

20+
#include <cstdint>
21+
#include <cstring>
2022
#include <memory>
2123
#include <string>
2224
#include <utility>
@@ -155,6 +157,27 @@ TEST(IndexedSplitTest, TestIndexedSplitWithScore) {
155157
<< roundtrip_indexed_split->ToString();
156158
}
157159

160+
TEST(IndexedSplitTest, TestSerializeCanonicalizesNaNScore) {
161+
auto pool = GetDefaultPool();
162+
DataSplitImpl::Builder builder(
163+
/*partition=*/BinaryRow::EmptyRow(),
164+
/*bucket=*/0, /*bucket_path=*/"bucket-0",
165+
/*data_files=*/{});
166+
ASSERT_OK_AND_ASSIGN(std::shared_ptr<Split> data_split, builder.Build());
167+
168+
uint32_t payload_bits = 0xffc12345;
169+
float payload_nan;
170+
std::memcpy(&payload_nan, &payload_bits, sizeof(payload_nan));
171+
auto indexed_split = std::make_shared<IndexedSplitImpl>(
172+
std::dynamic_pointer_cast<DataSplitImpl>(data_split), std::vector<Range>{Range(0, 0)},
173+
std::vector<float>{payload_nan});
174+
175+
ASSERT_OK_AND_ASSIGN(std::string serialized, Split::Serialize(indexed_split, pool));
176+
ASSERT_GE(serialized.size(), sizeof(float));
177+
ASSERT_EQ(serialized.substr(serialized.size() - sizeof(float)),
178+
std::string("\x7f\xc0\x00\x00", sizeof(float)));
179+
}
180+
158181
TEST(IndexedSplitTest, TestValidate) {
159182
auto meta = std::make_shared<DataFileMeta>(
160183
"file.orc", 1l, 200l, BinaryRow::EmptyRow(), BinaryRow::EmptyRow(),

src/paimon/core/table/source/split.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "paimon/common/data/binary_row.h"
2424
#include "paimon/common/io/memory_segment_output_stream.h"
2525
#include "paimon/common/memory/memory_segment_utils.h"
26+
#include "paimon/common/utils/math.h"
2627
#include "paimon/common/utils/serialization_utils.h"
2728
#include "paimon/core/global_index/indexed_split_impl.h"
2829
#include "paimon/core/io/data_file_meta_serializer.h"
@@ -159,8 +160,8 @@ Result<std::string> Split::Serialize(const std::shared_ptr<Split>& split,
159160
if (!scores.empty()) {
160161
out.WriteValue<bool>(true);
161162
out.WriteValue<int32_t>(scores.size());
162-
for (const auto& score : scores) {
163-
out.WriteValue<float>(score);
163+
for (float score : scores) {
164+
out.WriteValue<float>(CanonicalizeFloatingPoint(score));
164165
}
165166
} else {
166167
out.WriteValue<bool>(false);

0 commit comments

Comments
 (0)