Skip to content

Commit be8fb61

Browse files
zjw1111zjw1111
committed
fix(file-index): address writer compatibility issues
1 parent 275fd27 commit be8fb61

8 files changed

Lines changed: 98 additions & 48 deletions

File tree

‎src/paimon/common/file_index/bitmap/bitmap_file_index.cpp‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -131,15 +131,7 @@ BitmapFileIndexWriter::BitmapFileIndexWriter(int8_t version,
131131
Status BitmapFileIndexWriter::AddBatch(::ArrowArray* batch) {
132132
PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(std::shared_ptr<arrow::Array> arrow_array,
133133
arrow::ImportArray(batch, struct_type_));
134-
if (!arrow_array || arrow_array->type_id() != arrow::Type::STRUCT) {
135-
return Status::Invalid("invalid batch for BitmapFileIndexWriter, expected a struct array");
136-
}
137134
auto struct_array = checked_pointer_cast<arrow::StructArray>(arrow_array);
138-
if (struct_array->num_fields() != 1) {
139-
return Status::Invalid(
140-
"invalid batch for BitmapFileIndexWriter, expected a struct array with exactly one "
141-
"field");
142-
}
143135
PAIMON_ASSIGN_OR_RAISE(
144136
std::vector<Literal> array_values,
145137
LiteralConverter::ConvertLiteralsFromArray(*(struct_array->field(0)), /*own_data=*/true));

‎src/paimon/common/file_index/bloomfilter/bloom_filter_file_index.cpp‎

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
#include <cstddef>
2222
#include <cstdint>
23+
#include <cstring>
2324
#include <functional>
2425
#include <utility>
2526
#include <vector>
@@ -29,6 +30,7 @@
2930
#include "paimon/common/predicate/literal_converter.h"
3031
#include "paimon/common/utils/arrow/status_utils.h"
3132
#include "paimon/common/utils/checked_cast.h"
33+
#include "paimon/common/utils/math.h"
3234
#include "paimon/common/utils/options_utils.h"
3335
#include "paimon/fs/file_system.h"
3436
#include "paimon/memory/bytes.h"
@@ -106,17 +108,8 @@ BloomFilterFileIndexWriter::BloomFilterFileIndexWriter(
106108
Status BloomFilterFileIndexWriter::AddBatch(::ArrowArray* batch) {
107109
PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(std::shared_ptr<arrow::Array> array,
108110
arrow::ImportArray(batch, struct_type_));
109-
if (!array || array->type_id() != arrow::Type::STRUCT) {
110-
return Status::Invalid(
111-
"invalid batch for BloomFilterFileIndexWriter, expected a struct array");
112-
}
113111
std::shared_ptr<arrow::StructArray> struct_array =
114112
checked_pointer_cast<arrow::StructArray>(array);
115-
if (struct_array->num_fields() != 1) {
116-
return Status::Invalid(
117-
"invalid batch for BloomFilterFileIndexWriter, expected a struct array with exactly "
118-
"one field");
119-
}
120113
PAIMON_ASSIGN_OR_RAISE(
121114
std::vector<Literal> values,
122115
LiteralConverter::ConvertLiteralsFromArray(*struct_array->field(0), /*own_data=*/false));
@@ -133,24 +126,19 @@ Result<PAIMON_UNIQUE_PTR<Bytes>> BloomFilterFileIndexWriter::SerializedBytes() c
133126
const int32_t bit_set_length = filter_.GetBitSet().ByteLength();
134127
PAIMON_UNIQUE_PTR<Bytes> bytes =
135128
Bytes::AllocateBytes(kHeaderLength + bit_set_length, pool_.get());
136-
const auto num_hash_functions = static_cast<uint32_t>(filter_.GetNumHashFunctions());
137-
bytes->data()[0] = static_cast<char>((num_hash_functions >> 24) & 0xff);
138-
bytes->data()[1] = static_cast<char>((num_hash_functions >> 16) & 0xff);
139-
bytes->data()[2] = static_cast<char>((num_hash_functions >> 8) & 0xff);
140-
bytes->data()[3] = static_cast<char>(num_hash_functions & 0xff);
129+
const int32_t num_hash_functions = ToBigEndian(filter_.GetNumHashFunctions());
130+
std::memcpy(bytes->data(), &num_hash_functions, sizeof(num_hash_functions));
141131
filter_.GetBitSet().ToByteArray(kHeaderLength, bit_set_length, bytes->data());
142132
return bytes;
143133
}
144134

145135
Result<std::shared_ptr<BloomFilterFileIndexReader>> BloomFilterFileIndexReader::Create(
146136
const std::shared_ptr<arrow::DataType>& arrow_type, const std::shared_ptr<Bytes>& bytes) {
147137
// Compatible with Java's big-endian numHashFunctions header.
148-
const char* data = bytes->data();
149-
auto num_hash_functions =
150-
static_cast<int32_t>((static_cast<uint32_t>(static_cast<uint8_t>(data[0])) << 24) |
151-
(static_cast<uint32_t>(static_cast<uint8_t>(data[1])) << 16) |
152-
(static_cast<uint32_t>(static_cast<uint8_t>(data[2])) << 8) |
153-
static_cast<uint32_t>(static_cast<uint8_t>(data[3])));
138+
int32_t big_endian_num_hash_functions;
139+
std::memcpy(&big_endian_num_hash_functions, bytes->data(),
140+
sizeof(big_endian_num_hash_functions));
141+
const int32_t num_hash_functions = FromBigEndian(big_endian_num_hash_functions);
154142
PAIMON_ASSIGN_OR_RAISE(FastHash::HashFunction hash_function,
155143
FastHash::GetHashFunction(arrow_type));
156144
auto bit_set = std::make_unique<BloomFilter64::BitSet>(bytes, /*offset=*/sizeof(int32_t));

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "paimon/common/file_index/bloomfilter/fast_hash.h"
2020

2121
#include <cassert>
22+
#include <cmath>
2223
#include <cstring>
2324
#include <string>
2425
#include <utility>
@@ -34,6 +35,11 @@
3435
#include "xxhash.h" // NOLINT(build/include_subdir)
3536

3637
namespace paimon {
38+
namespace {
39+
constexpr int32_t kCanonicalFloatNaNBits = 0x7fc00000;
40+
constexpr int64_t kCanonicalDoubleNaNBits = 0x7ff8000000000000L;
41+
} // namespace
42+
3743
Result<FastHash::HashFunction> FastHash::GetHashFunction(
3844
const std::shared_ptr<arrow::DataType>& arrow_type) {
3945
PAIMON_ASSIGN_OR_RAISE(FieldType field_type,
@@ -58,14 +64,20 @@ Result<FastHash::HashFunction> FastHash::GetHashFunction(
5864
});
5965
case FieldType::FLOAT:
6066
return HashFunction([](const Literal& literal) -> int64_t {
61-
auto raw_value = literal.GetValue<float>();
67+
const auto raw_value = literal.GetValue<float>();
68+
if (std::isnan(raw_value)) {
69+
return GetLongHash(kCanonicalFloatNaNBits);
70+
}
6271
int32_t bits = 0;
6372
std::memcpy(&bits, &raw_value, sizeof(raw_value));
6473
return GetLongHash(bits);
6574
});
6675
case FieldType::DOUBLE:
6776
return HashFunction([](const Literal& literal) -> int64_t {
68-
auto raw_value = literal.GetValue<double>();
77+
const auto raw_value = literal.GetValue<double>();
78+
if (std::isnan(raw_value)) {
79+
return GetLongHash(kCanonicalDoubleNaNBits);
80+
}
6981
int64_t bits;
7082
std::memcpy(&bits, &raw_value, sizeof(raw_value));
7183
return GetLongHash(bits);

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@
1818

1919
#include "paimon/common/file_index/bloomfilter/fast_hash.h"
2020

21+
#include <cmath>
22+
#include <cstdint>
23+
#include <cstring>
2124
#include <limits>
2225
#include <string>
2326
#include <vector>
@@ -164,4 +167,32 @@ TEST_F(FastHashTest, TestCompatibleWithJava) {
164167
}
165168
}
166169

170+
TEST_F(FastHashTest, TestNaNCompatibleWithJava) {
171+
auto float_from_bits = [](uint32_t bits) {
172+
float value;
173+
std::memcpy(&value, &bits, sizeof(value));
174+
return value;
175+
};
176+
const float float_nan = float_from_bits(0x7fc12345);
177+
const float negative_float_nan = float_from_bits(0xffc54321);
178+
ASSERT_TRUE(std::isnan(float_nan));
179+
ASSERT_TRUE(std::isnan(negative_float_nan));
180+
ASSERT_OK_AND_ASSIGN(auto float_hash_function, FastHash::GetHashFunction(arrow::float32()));
181+
CheckResult(float_hash_function, {Literal(float_nan), Literal(negative_float_nan)},
182+
{0x67c27c6d9936ae63, 0x67c27c6d9936ae63});
183+
184+
auto double_from_bits = [](uint64_t bits) {
185+
double value;
186+
std::memcpy(&value, &bits, sizeof(value));
187+
return value;
188+
};
189+
const double double_nan = double_from_bits(0x7ff8123456789abc);
190+
const double negative_double_nan = double_from_bits(0xfff8abcdef012345);
191+
ASSERT_TRUE(std::isnan(double_nan));
192+
ASSERT_TRUE(std::isnan(negative_double_nan));
193+
ASSERT_OK_AND_ASSIGN(auto double_hash_function, FastHash::GetHashFunction(arrow::float64()));
194+
CheckResult(double_hash_function, {Literal(double_nan), Literal(negative_double_nan)},
195+
{0x13d2d3f2cc0e846e, 0x13d2d3f2cc0e846e});
196+
}
197+
167198
} // namespace paimon::test

‎src/paimon/common/file_index/bsi/bit_slice_index_bitmap_file_index.cpp‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -89,16 +89,7 @@ BitSliceIndexBitmapFileIndexWriter::BitSliceIndexBitmapFileIndexWriter(
8989
Status BitSliceIndexBitmapFileIndexWriter::AddBatch(::ArrowArray* batch) {
9090
PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(std::shared_ptr<arrow::Array> array,
9191
arrow::ImportArray(batch, struct_type_));
92-
if (!array || array->type_id() != arrow::Type::STRUCT) {
93-
return Status::Invalid(
94-
"invalid batch for BitSliceIndexBitmapFileIndexWriter, expected a struct array");
95-
}
9692
auto struct_array = checked_pointer_cast<arrow::StructArray>(array);
97-
if (struct_array->num_fields() != 1) {
98-
return Status::Invalid(
99-
"invalid batch for BitSliceIndexBitmapFileIndexWriter, expected a struct array with "
100-
"exactly one field");
101-
}
10293
if (struct_array->length() > static_cast<int64_t>(std::numeric_limits<int32_t>::max()) -
10394
static_cast<int64_t>(values_.size())) {
10495
return Status::Invalid("bsi index row count exceeds the supported int32 range");

‎src/paimon/common/file_index/rangebitmap/range_bitmap_file_index.cpp‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -87,16 +87,7 @@ Result<std::shared_ptr<RangeBitmapFileIndexWriter>> RangeBitmapFileIndexWriter::
8787
Status RangeBitmapFileIndexWriter::AddBatch(::ArrowArray* batch) {
8888
PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(std::shared_ptr<arrow::Array> array,
8989
arrow::ImportArray(batch, struct_type_));
90-
if (!array || array->type_id() != arrow::Type::STRUCT) {
91-
return Status::Invalid(
92-
"invalid batch for RangeBitmapFileIndexWriter, expected a struct array");
93-
}
9490
auto struct_array = checked_pointer_cast<arrow::StructArray>(array);
95-
if (struct_array->num_fields() != 1) {
96-
return Status::Invalid(
97-
"invalid batch for RangeBitmapFileIndexWriter, expected a struct array with exactly "
98-
"one field");
99-
}
10091
PAIMON_ASSIGN_OR_RAISE(std::vector<Literal> array_values,
10192
LiteralConverter::ConvertLiteralsFromArray(*(struct_array->field(0)),
10293
/*own_data=*/true));

‎src/paimon/common/utils/math.h‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636

3737
#include "fmt/format.h"
3838
#include "paimon/common/utils/options_utils.h"
39+
#include "paimon/io/byte_order.h"
3940
#include "paimon/status.h"
4041

4142
namespace paimon {
@@ -136,4 +137,30 @@ inline T EndianSwapValue(T v) {
136137
}
137138
}
138139

140+
template <typename T>
141+
inline T ToBigEndian(T value) {
142+
if constexpr (SystemByteOrder() == ByteOrder::PAIMON_LITTLE_ENDIAN) {
143+
return EndianSwapValue(value);
144+
}
145+
return value;
146+
}
147+
148+
template <typename T>
149+
inline T ToLittleEndian(T value) {
150+
if constexpr (SystemByteOrder() == ByteOrder::PAIMON_BIG_ENDIAN) {
151+
return EndianSwapValue(value);
152+
}
153+
return value;
154+
}
155+
156+
template <typename T>
157+
inline T FromBigEndian(T value) {
158+
return ToBigEndian(value);
159+
}
160+
161+
template <typename T>
162+
inline T FromLittleEndian(T value) {
163+
return ToLittleEndian(value);
164+
}
165+
139166
} // namespace paimon

‎src/paimon/common/utils/math_test.cpp‎

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

2020
#include "paimon/common/utils/math.h"
2121

22+
#include <array>
23+
#include <cstring>
2224
#include <limits>
2325

2426
#include "gtest/gtest.h"
@@ -44,6 +46,22 @@ TEST(MathTest, EndianSwapValue) {
4446
ASSERT_EQ(swapped64, 0xF0DEBC9A78563412);
4547
}
4648

49+
TEST(MathTest, ToEndian) {
50+
constexpr uint32_t kValue = 0x12345678;
51+
52+
const uint32_t big_endian = ToBigEndian(kValue);
53+
std::array<uint8_t, sizeof(big_endian)> big_endian_bytes{};
54+
std::memcpy(big_endian_bytes.data(), &big_endian, sizeof(big_endian));
55+
ASSERT_EQ((std::array<uint8_t, 4>{0x12, 0x34, 0x56, 0x78}), big_endian_bytes);
56+
ASSERT_EQ(kValue, FromBigEndian(big_endian));
57+
58+
const uint32_t little_endian = ToLittleEndian(kValue);
59+
std::array<uint8_t, sizeof(little_endian)> little_endian_bytes{};
60+
std::memcpy(little_endian_bytes.data(), &little_endian, sizeof(little_endian));
61+
ASSERT_EQ((std::array<uint8_t, 4>{0x78, 0x56, 0x34, 0x12}), little_endian_bytes);
62+
ASSERT_EQ(kValue, FromLittleEndian(little_endian));
63+
}
64+
4765
TEST(MathTest, InRange) {
4866
// signed -> unsigned: negative values out of range, boundary values in range
4967
ASSERT_TRUE(InRange<uint32_t>(0));

0 commit comments

Comments
 (0)