Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions include/paimon/predicate/literal.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,12 @@ class PAIMON_EXPORT Literal {
std::string ToString() const;

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

/// Compares this literal with another literal. The comparison follows SQL semantics for the
Expand Down
19 changes: 19 additions & 0 deletions src/paimon/common/data/variant/generic_variant_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

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

#include <cstdint>
#include <functional>
#include <limits>
#include <string>
Expand All @@ -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"

Expand Down Expand Up @@ -350,6 +352,23 @@ TEST_F(GenericVariantTest, NonFiniteDoubleToJson) {
ASSERT_EQ(json, "\"Infinity\"");
}

TEST_F(GenericVariantTest, CanonicalizesFloatingPointNaN) {
{
VariantBuilder builder(false);
ASSERT_OK(builder.AppendFloat(FloatingPointFromBits<float>(0xffc12345U)));
ASSERT_OK_AND_ASSIGN(std::shared_ptr<GenericVariant> variant, builder.Build(pool_));
ASSERT_OK_AND_ASSIGN(std::string_view value, variant->Value());
ASSERT_EQ(ToHex(value), "380000c07f");
}
{
VariantBuilder builder(false);
ASSERT_OK(builder.AppendDouble(FloatingPointFromBits<double>(0xfff8123456789abcULL)));
ASSERT_OK_AND_ASSIGN(std::shared_ptr<GenericVariant> variant, builder.Build(pool_));
ASSERT_OK_AND_ASSIGN(std::string_view value, variant->Value());
ASSERT_EQ(ToHex(value), "1c000000000000f87f");
}
}

TEST_F(GenericVariantTest, GetTypeInfoReturnsHeaderBits) {
// GetTypeInfo exposes the primitive header's type-info bits; 42 is encoded as an int1.
auto v = FromJson("42");
Expand Down
7 changes: 3 additions & 4 deletions src/paimon/common/data/variant/variant_builder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
23 changes: 3 additions & 20 deletions src/paimon/common/file_index/bloomfilter/fast_hash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#include "paimon/common/file_index/bloomfilter/fast_hash.h"

#include <cassert>
#include <cmath>
#include <cstring>
#include <string>
#include <utility>
Expand All @@ -28,18 +27,14 @@
#include "paimon/common/utils/checked_cast.h"
#include "paimon/common/utils/date_time_utils.h"
#include "paimon/common/utils/field_type_utils.h"
#include "paimon/common/utils/math.h"
#include "paimon/data/timestamp.h"
#include "paimon/defs.h"
#include "paimon/file_index/file_index_result.h"
#include "paimon/status.h"
#include "xxhash.h" // NOLINT(build/include_subdir)

namespace paimon {
namespace {
constexpr int32_t kCanonicalFloatNaNBits = 0x7fc00000;
constexpr int64_t kCanonicalDoubleNaNBits = 0x7ff8000000000000L;
} // namespace

Result<FastHash::HashFunction> FastHash::GetHashFunction(
const std::shared_ptr<arrow::DataType>& arrow_type) {
PAIMON_ASSIGN_OR_RAISE(FieldType field_type,
Expand All @@ -64,23 +59,11 @@ Result<FastHash::HashFunction> FastHash::GetHashFunction(
});
case FieldType::FLOAT:
return HashFunction([](const Literal& literal) -> int64_t {
const auto raw_value = literal.GetValue<float>();
if (std::isnan(raw_value)) {
return GetLongHash(kCanonicalFloatNaNBits);
}
int32_t bits = 0;
std::memcpy(&bits, &raw_value, sizeof(raw_value));
return GetLongHash(bits);
return GetLongHash(CanonicalizeFloatToIntBits(literal.GetValue<float>()));
});
case FieldType::DOUBLE:
return HashFunction([](const Literal& literal) -> int64_t {
const auto raw_value = literal.GetValue<double>();
if (std::isnan(raw_value)) {
return GetLongHash(kCanonicalDoubleNaNBits);
}
int64_t bits;
std::memcpy(&bits, &raw_value, sizeof(raw_value));
return GetLongHash(bits);
return GetLongHash(CanonicalizeDoubleToLongBits(literal.GetValue<double>()));
});
case FieldType::TIMESTAMP: {
auto ts_type = checked_pointer_cast<arrow::TimestampType>(arrow_type);
Expand Down
20 changes: 5 additions & 15 deletions src/paimon/common/file_index/bloomfilter/fast_hash_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@

#include <cmath>
#include <cstdint>
#include <cstring>
#include <limits>
#include <string>
#include <vector>

#include "gtest/gtest.h"
#include "paimon/common/utils/math.h"
#include "paimon/data/timestamp.h"
#include "paimon/defs.h"
#include "paimon/file_index/file_index_result.h"
Expand Down Expand Up @@ -168,26 +168,16 @@ TEST_F(FastHashTest, TestCompatibleWithJava) {
}

TEST_F(FastHashTest, TestNaNCompatibleWithJava) {
auto float_from_bits = [](uint32_t bits) {
float value;
std::memcpy(&value, &bits, sizeof(value));
return value;
};
const float float_nan = float_from_bits(0x7fc12345);
const float negative_float_nan = float_from_bits(0xffc54321);
const auto float_nan = FloatingPointFromBits<float>(0x7fc12345U);
const auto negative_float_nan = FloatingPointFromBits<float>(0xffc54321U);
ASSERT_TRUE(std::isnan(float_nan));
ASSERT_TRUE(std::isnan(negative_float_nan));
ASSERT_OK_AND_ASSIGN(auto float_hash_function, FastHash::GetHashFunction(arrow::float32()));
CheckResult(float_hash_function, {Literal(float_nan), Literal(negative_float_nan)},
{0x67c27c6d9936ae63, 0x67c27c6d9936ae63});

auto double_from_bits = [](uint64_t bits) {
double value;
std::memcpy(&value, &bits, sizeof(value));
return value;
};
const double double_nan = double_from_bits(0x7ff8123456789abc);
const double negative_double_nan = double_from_bits(0xfff8abcdef012345);
const auto double_nan = FloatingPointFromBits<double>(0x7ff8123456789abcULL);
const auto negative_double_nan = FloatingPointFromBits<double>(0xfff8abcdef012345ULL);
ASSERT_TRUE(std::isnan(double_nan));
ASSERT_TRUE(std::isnan(negative_double_nan));
ASSERT_OK_AND_ASSIGN(auto double_hash_function, FastHash::GetHashFunction(arrow::float64()));
Expand Down
11 changes: 3 additions & 8 deletions src/paimon/common/global_index/btree/key_serializer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -164,19 +165,13 @@ Result<std::shared_ptr<Bytes>> KeySerializer::SerializeKey(
case FieldType::FLOAT: {
MemorySliceOutput output(4, pool);
output.Reset();
auto fvalue = literal.GetValue<float>();
int32_t ivalue;
memcpy(&ivalue, &fvalue, sizeof(float));
output.WriteValue<int32_t>(ivalue);
output.WriteValue<int32_t>(CanonicalizeFloatToIntBits(literal.GetValue<float>()));
return output.ToSlice().CopyBytes(pool);
}
case FieldType::DOUBLE: {
MemorySliceOutput output(8, pool);
output.Reset();
auto dvalue = literal.GetValue<double>();
int64_t ivalue;
memcpy(&ivalue, &dvalue, sizeof(double));
output.WriteValue<int64_t>(ivalue);
output.WriteValue<int64_t>(CanonicalizeDoubleToLongBits(literal.GetValue<double>()));
return output.ToSlice().CopyBytes(pool);
}
case FieldType::STRING: {
Expand Down
28 changes: 28 additions & 0 deletions src/paimon/common/global_index/btree/key_serializer_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@

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

#include <cstdint>
#include <string>

#include "gtest/gtest.h"
#include "paimon/common/utils/math.h"
#include "paimon/data/decimal.h"
#include "paimon/data/timestamp.h"
#include "paimon/testing/utils/testharness.h"
Expand Down Expand Up @@ -208,6 +212,30 @@ TEST_F(KeySerializerTest, SerializeAndDeserializeAllTypes) {
}
}

TEST_F(KeySerializerTest, CanonicalizesFloatingPointNaN) {
const auto float_nan = FloatingPointFromBits<float>(0xffc12345U);
const auto canonical_float_nan = FloatingPointFromBits<float>(kCanonicalFloatNaNBits);
ASSERT_OK_AND_ASSIGN(
std::shared_ptr<Bytes> float_bytes,
KeySerializer::SerializeKey(Literal(float_nan), arrow::float32(), pool_.get()));
ASSERT_OK_AND_ASSIGN(
std::shared_ptr<Bytes> canonical_float_bytes,
KeySerializer::SerializeKey(Literal(canonical_float_nan), arrow::float32(), pool_.get()));
ASSERT_EQ(std::string(float_bytes->data(), float_bytes->size()),
std::string(canonical_float_bytes->data(), canonical_float_bytes->size()));

const auto double_nan = FloatingPointFromBits<double>(0xfff8123456789abcULL);
const auto canonical_double_nan = FloatingPointFromBits<double>(kCanonicalDoubleNaNBits);
ASSERT_OK_AND_ASSIGN(
std::shared_ptr<Bytes> double_bytes,
KeySerializer::SerializeKey(Literal(double_nan), arrow::float64(), pool_.get()));
ASSERT_OK_AND_ASSIGN(
std::shared_ptr<Bytes> canonical_double_bytes,
KeySerializer::SerializeKey(Literal(canonical_double_nan), arrow::float64(), pool_.get()));
ASSERT_EQ(std::string(double_bytes->data(), double_bytes->size()),
std::string(canonical_double_bytes->data(), canonical_double_bytes->size()));
}

TEST_F(KeySerializerTest, RejectsMalformedSerializedKeys) {
auto wrap = [this](const std::string& value) {
return MemorySlice::Wrap(std::make_shared<Bytes>(value, pool_.get()));
Expand Down
5 changes: 3 additions & 2 deletions src/paimon/common/global_index/global_index_result.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -37,8 +38,8 @@ void WriteBitmapAndScores(const RoaringBitmap64* bitmap, const std::vector<float
out->WriteBytes(bitmap_bytes);

out->WriteValue<int32_t>(scores.size());
for (auto score : scores) {
out->WriteValue<float>(score);
for (float score : scores) {
out->WriteValue<float>(CanonicalizeFloatingPoint(score));
}
}

Expand Down
26 changes: 26 additions & 0 deletions src/paimon/common/global_index/global_index_result_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,12 @@

#include "paimon/global_index/global_index_result.h"

#include <cmath>
#include <cstdint>
#include <utility>

#include "gtest/gtest.h"
#include "paimon/common/utils/math.h"
#include "paimon/global_index/bitmap_global_index_result.h"
#include "paimon/global_index/bitmap_scored_global_index_result.h"
#include "paimon/testing/utils/testharness.h"
Expand Down Expand Up @@ -144,6 +147,29 @@ TEST_F(GlobalIndexResultTest, TestSerializeAndDeserializeWithScore) {
serialize_bytes->data() + serialize_bytes->size()));
}

TEST_F(GlobalIndexResultTest, TestSerializeCanonicalizesNaNScore) {
auto pool = GetDefaultPool();
const auto payload_nan = FloatingPointFromBits<float>(0xffc12345U);
const auto canonical_nan = FloatingPointFromBits<float>(kCanonicalFloatNaNBits);
auto index_result = std::make_shared<BitmapScoredGlobalIndexResult>(
RoaringBitmap64::From({1}), std::vector<float>{payload_nan});
auto canonical_index_result = std::make_shared<BitmapScoredGlobalIndexResult>(
RoaringBitmap64::From({1}), std::vector<float>{canonical_nan});

ASSERT_OK_AND_ASSIGN(PAIMON_UNIQUE_PTR<Bytes> serialized,
GlobalIndexResult::Serialize(index_result, pool));
ASSERT_OK_AND_ASSIGN(PAIMON_UNIQUE_PTR<Bytes> canonical_serialized,
GlobalIndexResult::Serialize(canonical_index_result, pool));
ASSERT_EQ(*serialized, *canonical_serialized);

ASSERT_OK_AND_ASSIGN(
std::shared_ptr<GlobalIndexResult> deserialized,
GlobalIndexResult::Deserialize(serialized->data(), serialized->size(), pool));
auto scored_result = std::dynamic_pointer_cast<BitmapScoredGlobalIndexResult>(deserialized);
ASSERT_TRUE(scored_result);
ASSERT_TRUE(std::isnan(scored_result->GetScores()[0]));
}

TEST_F(GlobalIndexResultTest, TestInvalidSerialize) {
auto pool = GetDefaultPool();
auto result = std::make_shared<FakeGlobalIndexResult>(std::vector<int64_t>({1, 3, 5, 100}));
Expand Down
6 changes: 3 additions & 3 deletions src/paimon/common/predicate/literal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

#include "paimon/predicate/literal.h"

#include <cmath>
#include <cstring>
#include <functional>
#include <sstream>
Expand All @@ -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"
Expand Down Expand Up @@ -63,9 +63,9 @@ class Literal::Impl {
case FieldType::BIGINT:
return std::hash<int64_t>{}(value_.BigIntVal);
case FieldType::FLOAT:
return std::hash<float>{}(value_.FloatVal);
return std::hash<float>{}(CanonicalizeFloatingPoint(value_.FloatVal));
case FieldType::DOUBLE:
return std::hash<double>{}(value_.DoubleVal);
return std::hash<double>{}(CanonicalizeFloatingPoint(value_.DoubleVal));
case FieldType::STRING:
case FieldType::BINARY:
return std::hash<std::string_view>{}(std::string_view(value_.Buffer, size_));
Expand Down
Loading
Loading