From ac965bd0e622c7f47591e25ef9a95d38dda699b6 Mon Sep 17 00:00:00 2001 From: "jinli.zjw" Date: Tue, 1 Sep 2026 14:42:10 +0800 Subject: [PATCH 1/4] feat(file-index): support additional data types --- src/paimon/CMakeLists.txt | 1 + src/paimon/common/file_index/CMakeLists.txt | 4 +- .../bitmap/bitmap_file_index_meta.cpp | 32 +++ .../bitmap/bitmap_file_index_test.cpp | 248 ++++++++++++++++++ .../bsi/bit_slice_index_bitmap_file_index.cpp | 21 +- ...bit_slice_index_bitmap_file_index_test.cpp | 22 ++ .../dictionary/chunked_dictionary.cpp | 6 +- .../dictionary/chunked_dictionary.h | 1 - .../dictionary/chunked_dictionary_test.cpp | 22 +- .../rangebitmap/dictionary/key_factory.cpp | 26 +- .../dictionary/variable_length_chunk.cpp | 171 ++++++++++++ .../dictionary/variable_length_chunk.h | 103 ++++++++ .../rangebitmap/range_bitmap_file_index.cpp | 89 ++++--- .../rangebitmap/range_bitmap_file_index.h | 20 +- .../range_bitmap_file_index_test.cpp | 182 ++++++++++++- .../rangebitmap/range_bitmap_type_adapter.cpp | 113 ++++++++ .../rangebitmap/range_bitmap_type_adapter.h | 56 ++++ .../range_bitmap_type_adapter_test.cpp | 109 ++++++++ 18 files changed, 1168 insertions(+), 58 deletions(-) create mode 100644 src/paimon/common/file_index/rangebitmap/dictionary/variable_length_chunk.cpp create mode 100644 src/paimon/common/file_index/rangebitmap/dictionary/variable_length_chunk.h create mode 100644 src/paimon/common/file_index/rangebitmap/range_bitmap_type_adapter.cpp create mode 100644 src/paimon/common/file_index/rangebitmap/range_bitmap_type_adapter.h create mode 100644 src/paimon/common/file_index/rangebitmap/range_bitmap_type_adapter_test.cpp diff --git a/src/paimon/CMakeLists.txt b/src/paimon/CMakeLists.txt index 051eba324..8011574fb 100644 --- a/src/paimon/CMakeLists.txt +++ b/src/paimon/CMakeLists.txt @@ -572,6 +572,7 @@ if(PAIMON_BUILD_TESTS) common/file_index/rangebitmap/dictionary/chunked_dictionary_test.cpp common/file_index/rangebitmap/range_bitmap_file_index_test.cpp common/file_index/rangebitmap/range_bitmap_io_test.cpp + common/file_index/rangebitmap/range_bitmap_type_adapter_test.cpp common/file_index/bloomfilter/bloom_filter_file_index_test.cpp common/file_index/bloomfilter/fast_hash_test.cpp common/global_index/complete_index_score_batch_reader_test.cpp diff --git a/src/paimon/common/file_index/CMakeLists.txt b/src/paimon/common/file_index/CMakeLists.txt index 7ab5a0696..1e083c515 100644 --- a/src/paimon/common/file_index/CMakeLists.txt +++ b/src/paimon/common/file_index/CMakeLists.txt @@ -29,11 +29,13 @@ set(PAIMON_FILE_INDEX_SRC rangebitmap/dictionary/chunked_dictionary.cpp rangebitmap/dictionary/fixed_length_chunk.cpp rangebitmap/dictionary/key_factory.cpp + rangebitmap/dictionary/variable_length_chunk.cpp rangebitmap/utils/literal_serialization_utils.cpp rangebitmap/bit_slice_index_bitmap.cpp rangebitmap/range_bitmap.cpp rangebitmap/range_bitmap_file_index.cpp - rangebitmap/range_bitmap_file_index_factory.cpp) + rangebitmap/range_bitmap_file_index_factory.cpp + rangebitmap/range_bitmap_type_adapter.cpp) add_paimon_lib(paimon_file_index SOURCES 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..060d812a1 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 @@ -27,6 +27,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 +37,15 @@ #include "paimon/memory/memory_pool.h" #include "paimon/testing/utils/testharness.h" namespace paimon::test { +namespace { + +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 +88,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 +645,229 @@ 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 auto float_nan = FloatingPointFromBits(kCanonicalFloatNaNBits); + const auto float_positive_payload_nan = FloatingPointFromBits(uint32_t{0x7fc12345}); + const auto 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 auto double_nan = FloatingPointFromBits(kCanonicalDoubleNaNBits); + const auto double_positive_payload_nan = + FloatingPointFromBits(uint64_t{0x7ff8123456789abc}); + const auto 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 + // only the prefix which is independent of that encoding. For V1 it contains version, + // row count, entry count, has-null flag, key and offset: 1 + 4 + 4 + 1 + key_size + 4. + // For V2 it additionally contains one secondary-index entry, bitmap-body offset, and one + // index-block entry through entry.offset, but excludes entry.length and the bitmap body: + // 1 + 4 + 4 + 1 + 4 + (key_size + 4) + 4 + 4 + (key_size + 4). + const size_t comparable_prefix_size = + version == BitmapFileIndex::VERSION_1 ? 14 + key_size : 30 + 2 * key_size; + ASSERT_GE(java_bytes.size(), comparable_prefix_size); + ASSERT_GE(cpp_bytes->size(), comparable_prefix_size); + ASSERT_EQ( + std::vector(java_bytes.begin(), java_bytes.begin() + comparable_prefix_size), + std::vector(cpp_bytes->data(), cpp_bytes->data() + comparable_prefix_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/bsi/bit_slice_index_bitmap_file_index.cpp b/src/paimon/common/file_index/bsi/bit_slice_index_bitmap_file_index.cpp index 4cb98cdca..558dec502 100644 --- a/src/paimon/common/file_index/bsi/bit_slice_index_bitmap_file_index.cpp +++ b/src/paimon/common/file_index/bsi/bit_slice_index_bitmap_file_index.cpp @@ -38,6 +38,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/data/decimal.h" #include "paimon/data/timestamp.h" #include "paimon/defs.h" #include "paimon/file_index/bitmap_index_result.h" @@ -252,10 +253,26 @@ Result BitSliceIndexBitmapFileInd return literal.GetValue().ToMicrosecond(); }); } + case FieldType::DECIMAL: + return BitSliceIndexBitmapFileIndex::ValueMapperType( + [](const Literal& literal) -> Result { + if (literal.IsNull()) { + return Status::Invalid( + "literal cannot be null when GetValue in BitSliceIndexBitmapFileIndex"); + } + const auto value = literal.GetValue(); + if (value.Value() < std::numeric_limits::min() || + value.Value() > std::numeric_limits::max()) { + return Status::Invalid(fmt::format( + "decimal unscaled value {} does not fit in int64 for bsi index", + value.ToString())); + } + return value.ToUnscaledLong(); + }); default: - // TODO(xinyu.lxy): support decimal return Status::Invalid( - "BitSliceIndexBitmapFileIndex only support TINYINT/SMALLINT/INT/BIGINT/DATE"); + "BitSliceIndexBitmapFileIndex only support " + "TINYINT/SMALLINT/INT/BIGINT/DATE/TIMESTAMP/DECIMAL"); } } diff --git a/src/paimon/common/file_index/bsi/bit_slice_index_bitmap_file_index_test.cpp b/src/paimon/common/file_index/bsi/bit_slice_index_bitmap_file_index_test.cpp index 760434e19..2130164d8 100644 --- a/src/paimon/common/file_index/bsi/bit_slice_index_bitmap_file_index_test.cpp +++ b/src/paimon/common/file_index/bsi/bit_slice_index_bitmap_file_index_test.cpp @@ -27,6 +27,7 @@ #include "gtest/gtest.h" #include "paimon/common/utils/arrow/status_utils.h" #include "paimon/common/utils/field_type_utils.h" +#include "paimon/data/decimal.h" #include "paimon/data/timestamp.h" #include "paimon/defs.h" #include "paimon/file_index/bitmap_index_result.h" @@ -420,6 +421,27 @@ TEST_F(BitSliceIndexBitmapIndexReaderTest, TestTimestampType) { "literal cannot be null when GetValue in BitSliceIndexBitmapFileIndex"); } +TEST_F(BitSliceIndexBitmapIndexReaderTest, TestDecimalType) { + const auto type = arrow::decimal128(10, 2); + ASSERT_OK_AND_ASSIGN(PAIMON_UNIQUE_PTR index_bytes, + WriteIndex(type, R"([["1.00"], ["2.50"], [null], ["-1.25"], ["2.50"]])")); + auto input_stream = + std::make_shared(index_bytes->data(), index_bytes->size()); + BitSliceIndexBitmapFileIndex 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(Decimal(10, 2, 250))).value(), {1, 4}); + CheckResult(reader->VisitGreaterThan(Literal(Decimal(10, 2, 100))).value(), {1, 4}); + CheckResult(reader->VisitLessThan(Literal(Decimal(10, 2, 0))).value(), {3}); + CheckResult(reader->VisitIsNull().value(), {2}); + + // test invalid case for decimal128(20, 0) which exceeds int64 range + ASSERT_NOK_WITH_MSG(WriteIndex(arrow::decimal128(20, 0), R"([["9223372036854775808"]])"), + "does not fit in int64 for bsi index"); +} + TEST_F(BitSliceIndexBitmapIndexReaderTest, TestUnInvalidType) { std::vector index_bytes = { 1, 0, 0, 0, 5, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 58, diff --git a/src/paimon/common/file_index/rangebitmap/dictionary/chunked_dictionary.cpp b/src/paimon/common/file_index/rangebitmap/dictionary/chunked_dictionary.cpp index 3011afea7..cc96e48f5 100644 --- a/src/paimon/common/file_index/rangebitmap/dictionary/chunked_dictionary.cpp +++ b/src/paimon/common/file_index/rangebitmap/dictionary/chunked_dictionary.cpp @@ -89,14 +89,11 @@ Result> ChunkedDictionary::GetChunk(int32_t index) { if (index < 0 || index >= size_) { return Status::Invalid(fmt::format("Invalid chunk index: {}", index)); } - if (offsets_bytes_ == nullptr || chunks_bytes_ == nullptr) { + if (offsets_bytes_ == nullptr) { PAIMON_RETURN_NOT_OK(input_stream_->Seek(body_offset_, FS_SEEK_SET)); auto offsets = Bytes::AllocateBytes(offsets_length_, pool_.get()); PAIMON_RETURN_NOT_OK(input_stream_->Read(offsets->data(), offsets_length_)); offsets_bytes_ = std::move(offsets); - auto chunks = Bytes::AllocateBytes(chunks_length_, pool_.get()); - PAIMON_RETURN_NOT_OK(input_stream_->Read(chunks->data(), chunks_length_)); - chunks_bytes_ = std::move(chunks); } if (chunks_cache_[index]) { return chunks_cache_[index]; @@ -244,6 +241,5 @@ ChunkedDictionary::ChunkedDictionary(const std::shared_ptr& input_s chunks_length_(chunks_length), body_offset_(body_offset), offsets_bytes_(nullptr), - chunks_bytes_(nullptr), chunks_cache_(std::vector>(size)) {} } // namespace paimon diff --git a/src/paimon/common/file_index/rangebitmap/dictionary/chunked_dictionary.h b/src/paimon/common/file_index/rangebitmap/dictionary/chunked_dictionary.h index 7cb4ba4f9..a1a6edcfd 100644 --- a/src/paimon/common/file_index/rangebitmap/dictionary/chunked_dictionary.h +++ b/src/paimon/common/file_index/rangebitmap/dictionary/chunked_dictionary.h @@ -96,7 +96,6 @@ class ChunkedDictionary final : public Dictionary { // for lazy loading PAIMON_UNIQUE_PTR offsets_bytes_; - PAIMON_UNIQUE_PTR chunks_bytes_; // mmap chunks cache std::vector> chunks_cache_; diff --git a/src/paimon/common/file_index/rangebitmap/dictionary/chunked_dictionary_test.cpp b/src/paimon/common/file_index/rangebitmap/dictionary/chunked_dictionary_test.cpp index 93c198548..671d891c1 100644 --- a/src/paimon/common/file_index/rangebitmap/dictionary/chunked_dictionary_test.cpp +++ b/src/paimon/common/file_index/rangebitmap/dictionary/chunked_dictionary_test.cpp @@ -456,9 +456,25 @@ TEST_F(ChunkedDictionaryTest, TestKeyFactoryUnsupportedType) { "Unsupported field type for KeyFactory: BINARY"); } -TEST_F(ChunkedDictionaryTest, TestStringKeyFactoryNotImplemented) { - ASSERT_NOK_WITH_MSG(KeyFactory::Create(FieldType::STRING), - "Unsupported field type for KeyFactory: STRING"); +TEST_F(ChunkedDictionaryTest, TestStringKeyFactory) { + ASSERT_OK_AND_ASSIGN(auto key_factory, KeyFactory::Create(FieldType::STRING)); + ASSERT_OK_AND_ASSIGN(auto appender, + ChunkedDictionary::Appender::Create(key_factory, 12, pool_)); + ASSERT_OK(appender->AppendSorted(Literal(FieldType::STRING, "apple", 5), 0)); + ASSERT_OK(appender->AppendSorted(Literal(FieldType::STRING, "banana", 6), 1)); + ASSERT_OK(appender->AppendSorted(Literal(FieldType::STRING, "pear", 4), 2)); + ASSERT_OK_AND_ASSIGN(auto bytes, appender->Serialize()); + auto input_stream = std::make_shared(bytes->data(), bytes->size()); + ASSERT_OK_AND_ASSIGN(auto dict, + ChunkedDictionary::Create(FieldType::STRING, input_stream, 0, pool_)); + + ASSERT_OK_AND_ASSIGN(int32_t banana_code, dict->Find(Literal(FieldType::STRING, "banana", 6))); + ASSERT_EQ(banana_code, 1); + ASSERT_OK_AND_ASSIGN(Literal pear, dict->Find(2)); + ASSERT_EQ(pear.GetValue(), "pear"); + ASSERT_OK_AND_ASSIGN(int32_t between_code, + dict->Find(Literal(FieldType::STRING, "blueberry", 9))); + ASSERT_EQ(between_code, -3); } TEST_F(ChunkedDictionaryTest, TestFindByCodeInvalidNegative) { diff --git a/src/paimon/common/file_index/rangebitmap/dictionary/key_factory.cpp b/src/paimon/common/file_index/rangebitmap/dictionary/key_factory.cpp index 37f80c312..a3fafd599 100644 --- a/src/paimon/common/file_index/rangebitmap/dictionary/key_factory.cpp +++ b/src/paimon/common/file_index/rangebitmap/dictionary/key_factory.cpp @@ -23,6 +23,7 @@ #include "fmt/format.h" #include "paimon/common/file_index/rangebitmap/dictionary/chunked_dictionary.h" #include "paimon/common/file_index/rangebitmap/dictionary/fixed_length_chunk.h" +#include "paimon/common/file_index/rangebitmap/dictionary/variable_length_chunk.h" #include "paimon/common/file_index/rangebitmap/utils/literal_serialization_utils.h" #include "paimon/common/utils/field_type_utils.h" #include "paimon/common/utils/fields_comparator.h" @@ -48,6 +49,8 @@ Result> KeyFactory::Create(FieldType field_type) { return std::make_shared(); case FieldType::DOUBLE: return std::make_shared(); + case FieldType::STRING: + return std::make_shared(); default: return Status::Invalid(fmt::format("Unsupported field type for KeyFactory: {}", FieldTypeUtils::FieldTypeToString(field_type))); @@ -91,12 +94,31 @@ Result> FixedLengthKeyFactory::MmapChunk( Result> VariableLengthKeyFactory::CreateChunk( const Literal& key, int32_t code, int32_t keys_length_limit, const std::shared_ptr& pool) { - return Status::NotImplemented("VariableLengthKeyFactory::CreateChunk not implemented"); + PAIMON_ASSIGN_OR_RAISE(LiteralSerDeUtils::Serializer serializer, + LiteralSerDeUtils::CreateValueWriter(GetFieldType())); + return std::make_unique(key, code, keys_length_limit, shared_from_this(), + serializer, pool); } Result> VariableLengthKeyFactory::MmapChunk( const std::shared_ptr& input_stream, int32_t chunk_offset, int32_t keys_base_offset, const std::shared_ptr& pool) { - return Status::NotImplemented("VariableLengthKeyFactory::MmapChunk not implemented"); + PAIMON_RETURN_NOT_OK(input_stream->Seek(chunk_offset, FS_SEEK_SET)); + const auto data_in = std::make_shared(input_stream); + PAIMON_ASSIGN_OR_RAISE(int8_t version, data_in->ReadValue()); + if (version != VariableLengthChunk::kCurrentVersion) { + return Status::Invalid(fmt::format("Unsupported version for KeyFactory: {}", version)); + } + PAIMON_ASSIGN_OR_RAISE(LiteralSerDeUtils::Deserializer deserializer, + LiteralSerDeUtils::CreateValueReader(GetFieldType())); + PAIMON_ASSIGN_OR_RAISE(Literal key_literal, deserializer(data_in, pool.get())); + PAIMON_ASSIGN_OR_RAISE(int32_t code, data_in->ReadValue()); + PAIMON_ASSIGN_OR_RAISE(int32_t offset, data_in->ReadValue()); + PAIMON_ASSIGN_OR_RAISE(int32_t size, data_in->ReadValue()); + PAIMON_ASSIGN_OR_RAISE(int32_t offsets_length, data_in->ReadValue()); + PAIMON_ASSIGN_OR_RAISE(int32_t keys_length, data_in->ReadValue()); + return std::make_unique(key_literal, code, offset, size, + shared_from_this(), input_stream, keys_base_offset, + offsets_length, keys_length, pool); } /// Java-compatible ordering for floats diff --git a/src/paimon/common/file_index/rangebitmap/dictionary/variable_length_chunk.cpp b/src/paimon/common/file_index/rangebitmap/dictionary/variable_length_chunk.cpp new file mode 100644 index 000000000..5bb6dda4b --- /dev/null +++ b/src/paimon/common/file_index/rangebitmap/dictionary/variable_length_chunk.cpp @@ -0,0 +1,171 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "paimon/common/file_index/rangebitmap/dictionary/variable_length_chunk.h" + +#include +#include + +#include "fmt/format.h" +#include "paimon/common/file_index/rangebitmap/dictionary/key_factory.h" +#include "paimon/common/io/memory_segment_output_stream.h" +#include "paimon/common/memory/memory_segment_utils.h" +#include "paimon/io/byte_array_input_stream.h" +#include "paimon/io/data_input_stream.h" +#include "paimon/memory/bytes.h" + +namespace paimon { + +Result VariableLengthChunk::TryAdd(const Literal& key) { + PAIMON_ASSIGN_OR_RAISE(int32_t key_length, LiteralSerDeUtils::GetSerializedSizeInBytes(key)); + if (key_length > remaining_keys_size_ || + static_cast(sizeof(int32_t)) > remaining_offsets_size_) { + return false; + } + offsets_stream_out_->WriteValue(static_cast(keys_stream_out_->CurrentSize())); + PAIMON_RETURN_NOT_OK(serializer_(keys_stream_out_, key)); + remaining_offsets_size_ -= sizeof(int32_t); + remaining_keys_size_ -= key_length; + ++size_; + return true; +} + +Result VariableLengthChunk::CompareKey(const Literal& lhs, const Literal& rhs) { + return factory_->CompareLiteral(lhs, rhs); +} + +Status VariableLengthChunk::LoadKeys() { + if (offsets_ != nullptr && keys_ != nullptr) { + return Status::OK(); + } + if (offsets_length_ < 0 || keys_length_ < 0 || + offsets_length_ > std::numeric_limits::max() - keys_length_) { + return Status::Invalid("Invalid variable length chunk payload length"); + } + PAIMON_RETURN_NOT_OK(input_stream_->Seek(keys_base_offset_ + offset_, FS_SEEK_SET)); + offsets_ = Bytes::AllocateBytes(offsets_length_, pool_.get()); + PAIMON_ASSIGN_OR_RAISE(int64_t offsets_read, + input_stream_->Read(offsets_->data(), offsets_length_)); + if (offsets_read != offsets_length_) { + return Status::Invalid(fmt::format( + "Failed to read variable length chunk offsets, expected {} bytes but got {}", + offsets_length_, offsets_read)); + } + keys_ = Bytes::AllocateBytes(keys_length_, pool_.get()); + PAIMON_ASSIGN_OR_RAISE(int64_t keys_read, input_stream_->Read(keys_->data(), keys_length_)); + if (keys_read != keys_length_) { + return Status::Invalid( + fmt::format("Failed to read variable length chunk keys, expected {} bytes but got {}", + keys_length_, keys_read)); + } + PAIMON_ASSIGN_OR_RAISE(deserializer_, + LiteralSerDeUtils::CreateValueReader(factory_->GetFieldType())); + return Status::OK(); +} + +Result VariableLengthChunk::GetKey(int32_t index) { + if (index < 0 || index >= size_) { + return Status::Invalid("Index out of bounds"); + } + PAIMON_RETURN_NOT_OK(LoadKeys()); + auto offsets_in = std::make_shared( + std::make_shared(offsets_->data(), offsets_->size())); + PAIMON_RETURN_NOT_OK(offsets_in->Seek(static_cast(index) * sizeof(int32_t))); + PAIMON_ASSIGN_OR_RAISE(int32_t key_offset, offsets_in->ReadValue()); + if (key_offset < 0 || key_offset >= keys_length_) { + return Status::Invalid("Invalid key offset in variable length chunk"); + } + auto keys_in = std::make_shared( + std::make_shared(keys_->data(), keys_->size())); + PAIMON_RETURN_NOT_OK(keys_in->Seek(key_offset)); + return deserializer_(keys_in, pool_.get()); +} + +Result> VariableLengthChunk::SerializeChunk() const { + const auto data_out = std::make_shared( + MemorySegmentOutputStream::DEFAULT_SEGMENT_SIZE, pool_); + data_out->WriteValue(kCurrentVersion); + PAIMON_RETURN_NOT_OK(serializer_(data_out, key_)); + data_out->WriteValue(code_); + data_out->WriteValue(offset_); + data_out->WriteValue(size_); + data_out->WriteValue(static_cast(offsets_stream_out_->CurrentSize())); + data_out->WriteValue(static_cast(keys_stream_out_->CurrentSize())); + return MemorySegmentUtils::CopyToBytes( + data_out->Segments(), 0, static_cast(data_out->CurrentSize()), pool_.get()); +} + +Result> VariableLengthChunk::SerializeKeys() const { + const auto data_out = std::make_shared( + MemorySegmentOutputStream::DEFAULT_SEGMENT_SIZE, pool_); + PAIMON_RETURN_NOT_OK(MemorySegmentUtils::CopyToStream( + offsets_stream_out_->Segments(), 0, + static_cast(offsets_stream_out_->CurrentSize()), data_out.get())); + PAIMON_RETURN_NOT_OK(MemorySegmentUtils::CopyToStream( + keys_stream_out_->Segments(), 0, static_cast(keys_stream_out_->CurrentSize()), + data_out.get())); + return MemorySegmentUtils::CopyToBytes( + data_out->Segments(), 0, static_cast(data_out->CurrentSize()), pool_.get()); +} + +/// Read path +VariableLengthChunk::VariableLengthChunk(Literal key, int32_t code, int32_t offset, int32_t size, + const std::shared_ptr& factory, + const std::shared_ptr& input_stream, + int32_t keys_base_offset, int32_t offsets_length, + int32_t keys_length, + const std::shared_ptr& pool) + : pool_(pool), + key_(std::move(key)), + code_(code), + offset_(offset), + size_(size), + factory_(factory), + input_stream_(input_stream), + keys_base_offset_(keys_base_offset), + offsets_length_(offsets_length), + keys_length_(keys_length), + deserializer_({}), + serializer_({}), + remaining_offsets_size_(0), + remaining_keys_size_(0) {} + +/// Write path +VariableLengthChunk::VariableLengthChunk(Literal key, int32_t code, int32_t keys_length_limit, + const std::shared_ptr& factory, + const LiteralSerDeUtils::Serializer& serializer, + const std::shared_ptr& pool) + : pool_(pool), + key_(std::move(key)), + code_(code), + offset_(0), + size_(0), + factory_(factory), + keys_base_offset_(0), + offsets_length_(0), + keys_length_(0), + deserializer_({}), + serializer_(serializer), + offsets_stream_out_(std::make_shared( + MemorySegmentOutputStream::DEFAULT_SEGMENT_SIZE, pool)), + keys_stream_out_(std::make_shared( + MemorySegmentOutputStream::DEFAULT_SEGMENT_SIZE, pool)), + remaining_offsets_size_(keys_length_limit), + remaining_keys_size_(keys_length_limit) {} + +} // namespace paimon diff --git a/src/paimon/common/file_index/rangebitmap/dictionary/variable_length_chunk.h b/src/paimon/common/file_index/rangebitmap/dictionary/variable_length_chunk.h new file mode 100644 index 000000000..0c1733279 --- /dev/null +++ b/src/paimon/common/file_index/rangebitmap/dictionary/variable_length_chunk.h @@ -0,0 +1,103 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include + +#include "paimon/common/file_index/rangebitmap/dictionary/chunk.h" +#include "paimon/common/file_index/rangebitmap/utils/literal_serialization_utils.h" +#include "paimon/fs/file_system.h" + +namespace paimon { + +class DataInputStream; +class InputStream; +class KeyFactory; +class MemoryPool; +class MemorySegmentOutputStream; + +class VariableLengthChunk final : public Chunk { + public: + Result TryAdd(const Literal& key) override; + Result CompareKey(const Literal& lhs, const Literal& rhs) override; + Result GetKey(int32_t index) override; + + const Literal& Key() const override { + return key_; + } + int32_t Code() const override { + return code_; + } + int32_t Offset() const override { + return offset_; + } + void SetOffset(int32_t offset) override { + offset_ = offset; + } + int32_t Size() const override { + return size_; + } + + Result> SerializeChunk() const override; + Result> SerializeKeys() const override; + + // For Read Path + VariableLengthChunk(Literal key, int32_t code, int32_t offset, int32_t size, + const std::shared_ptr& factory, + const std::shared_ptr& input_stream, int32_t keys_base_offset, + int32_t offsets_length, int32_t keys_length, + const std::shared_ptr& pool); + + // For Write Path + VariableLengthChunk(Literal key, int32_t code, int32_t keys_length_limit, + const std::shared_ptr& factory, + const LiteralSerDeUtils::Serializer& serializer, + const std::shared_ptr& pool); + + public: + static constexpr int8_t kCurrentVersion = 1; + + private: + Status LoadKeys(); + + std::shared_ptr pool_; + Literal key_; // representative key for binary search + int32_t code_; // first code in this chunk + int32_t offset_; // offset of this chunk + int32_t size_; // number of keys in this chunk + std::shared_ptr factory_; + + // For read path lazy keys loading + std::shared_ptr input_stream_; + int32_t keys_base_offset_; + int32_t offsets_length_; + int32_t keys_length_; + PAIMON_UNIQUE_PTR offsets_; + PAIMON_UNIQUE_PTR keys_; + LiteralSerDeUtils::Deserializer deserializer_; + + // For write path + LiteralSerDeUtils::Serializer serializer_; + std::shared_ptr offsets_stream_out_; + std::shared_ptr keys_stream_out_; + int32_t remaining_offsets_size_; + int32_t remaining_keys_size_; +}; + +} // namespace paimon diff --git a/src/paimon/common/file_index/rangebitmap/range_bitmap_file_index.cpp b/src/paimon/common/file_index/rangebitmap/range_bitmap_file_index.cpp index 88e081bd1..8384e7df0 100644 --- a/src/paimon/common/file_index/rangebitmap/range_bitmap_file_index.cpp +++ b/src/paimon/common/file_index/rangebitmap/range_bitmap_file_index.cpp @@ -22,12 +22,12 @@ #include #include "paimon/common/file_index/rangebitmap/range_bitmap.h" +#include "paimon/common/file_index/rangebitmap/range_bitmap_type_adapter.h" #include "paimon/common/io/offset_input_stream.h" #include "paimon/common/options/memory_size.h" #include "paimon/common/predicate/literal_converter.h" #include "paimon/common/utils/arrow/status_utils.h" #include "paimon/common/utils/checked_cast.h" -#include "paimon/common/utils/field_type_utils.h" #include "paimon/file_index/bitmap_index_result.h" #include "paimon/predicate/literal.h" #include "paimon/result.h" @@ -66,10 +66,10 @@ Result> RangeBitmapFileIndex::CreateWriter( Result> RangeBitmapFileIndexWriter::Create( const std::shared_ptr& field, const std::map& options, const std::shared_ptr& pool) { - PAIMON_ASSIGN_OR_RAISE(FieldType field_type, - FieldTypeUtils::ConvertToFieldType(field->type()->id())); + PAIMON_ASSIGN_OR_RAISE(std::unique_ptr type_adapter, + RangeBitmapTypeAdapter::Create(field->type())); PAIMON_ASSIGN_OR_RAISE(std::shared_ptr shared_key_factory, - KeyFactory::Create(field_type)); + KeyFactory::Create(type_adapter->GetStorageType())); PAIMON_ASSIGN_OR_RAISE(int64_t parsed_chunk_size, MemorySize::ParseBytes(KeyFactory::kDefaultChunkSize)); if (const auto chunk_size_it = options.find(RangeBitmapFileIndex::kChunkSize); @@ -80,10 +80,12 @@ Result> RangeBitmapFileIndexWriter:: PAIMON_ASSIGN_OR_RAISE( std::unique_ptr appender_ptr, RangeBitmap::Appender::Create(shared_key_factory, parsed_chunk_size, pool)); - return std::make_shared(struct_type, pool, shared_key_factory, - std::move(appender_ptr)); + return std::shared_ptr(new RangeBitmapFileIndexWriter( + struct_type, std::move(type_adapter), std::move(appender_ptr))); } +RangeBitmapFileIndexWriter::~RangeBitmapFileIndexWriter() = default; + Status RangeBitmapFileIndexWriter::AddBatch(::ArrowArray* batch) { PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(std::shared_ptr array, arrow::ImportArray(batch, struct_type_)); @@ -91,8 +93,9 @@ Status RangeBitmapFileIndexWriter::AddBatch(::ArrowArray* batch) { PAIMON_ASSIGN_OR_RAISE(std::vector array_values, LiteralConverter::ConvertLiteralsFromArray(*(struct_array->field(0)), /*own_data=*/true)); - for (const auto& literal : array_values) { - appender_->Append(literal); + for (const Literal& literal : array_values) { + PAIMON_ASSIGN_OR_RAISE(Literal converted_literal, type_adapter_->ToStorageLiteral(literal)); + appender_->Append(converted_literal); } return Status::OK(); } @@ -102,58 +105,70 @@ Result> RangeBitmapFileIndexWriter::SerializedBytes() c } RangeBitmapFileIndexWriter::RangeBitmapFileIndexWriter( - const std::shared_ptr& struct_type, const std::shared_ptr& pool, - const std::shared_ptr& key_factory, std::unique_ptr appender) + const std::shared_ptr& struct_type, + std::unique_ptr type_adapter, + std::unique_ptr appender) : struct_type_(struct_type), - pool_(pool), - key_factory_(key_factory), + type_adapter_(std::move(type_adapter)), appender_(std::move(appender)) {} Result> RangeBitmapFileIndexReader::Create( const std::shared_ptr& arrow_type, const int32_t start, const int32_t length, const std::shared_ptr& input_stream, const std::shared_ptr& pool) { - PAIMON_ASSIGN_OR_RAISE(FieldType field_type, - FieldTypeUtils::ConvertToFieldType(arrow_type->id())); + PAIMON_ASSIGN_OR_RAISE(std::unique_ptr type_adapter, + RangeBitmapTypeAdapter::Create(arrow_type)); PAIMON_ASSIGN_OR_RAISE(std::shared_ptr bounded_stream, OffsetInputStream::Create(input_stream, length, start)); - PAIMON_ASSIGN_OR_RAISE(std::unique_ptr range_bitmap, - RangeBitmap::Create(bounded_stream, 0, field_type, pool)); + PAIMON_ASSIGN_OR_RAISE( + std::unique_ptr range_bitmap, + RangeBitmap::Create(bounded_stream, 0, type_adapter->GetStorageType(), pool)); return std::shared_ptr( - new RangeBitmapFileIndexReader(std::move(range_bitmap))); + new RangeBitmapFileIndexReader(std::move(type_adapter), std::move(range_bitmap))); } -RangeBitmapFileIndexReader::RangeBitmapFileIndexReader(std::unique_ptr range_bitmap) - : range_bitmap_(std::move(range_bitmap)) {} +RangeBitmapFileIndexReader::~RangeBitmapFileIndexReader() = default; + +RangeBitmapFileIndexReader::RangeBitmapFileIndexReader( + std::unique_ptr type_adapter, std::unique_ptr range_bitmap) + : type_adapter_(std::move(type_adapter)), range_bitmap_(std::move(range_bitmap)) {} Result> RangeBitmapFileIndexReader::VisitEqual( const Literal& literal) { + PAIMON_ASSIGN_OR_RAISE(Literal converted_literal, type_adapter_->ToStorageLiteral(literal)); return std::make_shared( - [self = shared_from_this(), literal]() -> Result { - return self->range_bitmap_->Eq(literal); + [self = shared_from_this(), converted_literal]() -> Result { + return self->range_bitmap_->Eq(converted_literal); }); } Result> RangeBitmapFileIndexReader::VisitNotEqual( const Literal& literal) { + PAIMON_ASSIGN_OR_RAISE(Literal converted_literal, type_adapter_->ToStorageLiteral(literal)); return std::make_shared( - [self = shared_from_this(), literal]() -> Result { - return self->range_bitmap_->Neq(literal); + [self = shared_from_this(), converted_literal]() -> Result { + return self->range_bitmap_->Neq(converted_literal); }); } Result> RangeBitmapFileIndexReader::VisitIn( const std::vector& literals) { + PAIMON_ASSIGN_OR_RAISE(std::vector converted_literals, + type_adapter_->ToStorageLiterals(literals)); return std::make_shared( - [self = shared_from_this(), literals]() -> Result { - return self->range_bitmap_->In(literals); + [self = shared_from_this(), + converted_literals = std::move(converted_literals)]() -> Result { + return self->range_bitmap_->In(converted_literals); }); } Result> RangeBitmapFileIndexReader::VisitNotIn( const std::vector& literals) { + PAIMON_ASSIGN_OR_RAISE(std::vector converted_literals, + type_adapter_->ToStorageLiterals(literals)); return std::make_shared( - [self = shared_from_this(), literals]() -> Result { - return self->range_bitmap_->NotIn(literals); + [self = shared_from_this(), + converted_literals = std::move(converted_literals)]() -> Result { + return self->range_bitmap_->NotIn(converted_literals); }); } @@ -173,33 +188,37 @@ Result> RangeBitmapFileIndexReader::VisitIsNotN Result> RangeBitmapFileIndexReader::VisitGreaterThan( const Literal& literal) { + PAIMON_ASSIGN_OR_RAISE(Literal converted_literal, type_adapter_->ToStorageLiteral(literal)); return std::make_shared( - [self = shared_from_this(), literal]() -> Result { - return self->range_bitmap_->Gt(literal); + [self = shared_from_this(), converted_literal]() -> Result { + return self->range_bitmap_->Gt(converted_literal); }); } Result> RangeBitmapFileIndexReader::VisitLessThan( const Literal& literal) { + PAIMON_ASSIGN_OR_RAISE(Literal converted_literal, type_adapter_->ToStorageLiteral(literal)); return std::make_shared( - [self = shared_from_this(), literal]() -> Result { - return self->range_bitmap_->Lt(literal); + [self = shared_from_this(), converted_literal]() -> Result { + return self->range_bitmap_->Lt(converted_literal); }); } Result> RangeBitmapFileIndexReader::VisitGreaterOrEqual( const Literal& literal) { + PAIMON_ASSIGN_OR_RAISE(Literal converted_literal, type_adapter_->ToStorageLiteral(literal)); return std::make_shared( - [self = shared_from_this(), literal]() -> Result { - return self->range_bitmap_->Gte(literal); + [self = shared_from_this(), converted_literal]() -> Result { + return self->range_bitmap_->Gte(converted_literal); }); } Result> RangeBitmapFileIndexReader::VisitLessOrEqual( const Literal& literal) { + PAIMON_ASSIGN_OR_RAISE(Literal converted_literal, type_adapter_->ToStorageLiteral(literal)); return std::make_shared( - [self = shared_from_this(), literal]() -> Result { - return self->range_bitmap_->Lte(literal); + [self = shared_from_this(), converted_literal]() -> Result { + return self->range_bitmap_->Lte(converted_literal); }); } diff --git a/src/paimon/common/file_index/rangebitmap/range_bitmap_file_index.h b/src/paimon/common/file_index/rangebitmap/range_bitmap_file_index.h index 64289a536..3e7d511e0 100644 --- a/src/paimon/common/file_index/rangebitmap/range_bitmap_file_index.h +++ b/src/paimon/common/file_index/rangebitmap/range_bitmap_file_index.h @@ -36,6 +36,7 @@ namespace paimon { class RangeBitmapFileIndexWriter; class RangeBitmapFileIndexReader; +class RangeBitmapTypeAdapter; class PAIMON_EXPORT RangeBitmapFileIndex final : public FileIndexer { public: @@ -64,20 +65,19 @@ class RangeBitmapFileIndexWriter final : public FileIndexWriter { const std::shared_ptr& field, const std::map& options, const std::shared_ptr& pool); + ~RangeBitmapFileIndexWriter() override; + Status AddBatch(::ArrowArray* batch) override; Result> SerializedBytes() const override; + private: RangeBitmapFileIndexWriter(const std::shared_ptr& struct_type, - const std::shared_ptr& pool, - const std::shared_ptr& key_factory, + std::unique_ptr type_adapter, std::unique_ptr appender); - private: - /// @note struct_type_ contains only one field with arrow_type_, used for import from C - /// interface. + /// @note struct_type_ contains only the indexed field and is used to import Arrow C data. std::shared_ptr struct_type_; - std::shared_ptr pool_; - std::shared_ptr key_factory_; + std::unique_ptr type_adapter_; std::unique_ptr appender_; }; @@ -89,8 +89,11 @@ class RangeBitmapFileIndexReader final const std::shared_ptr& arrow_type, int32_t start, int32_t length, const std::shared_ptr& input_stream, const std::shared_ptr& pool); + ~RangeBitmapFileIndexReader() override; + private: - explicit RangeBitmapFileIndexReader(std::unique_ptr range_bitmap); + RangeBitmapFileIndexReader(std::unique_ptr type_adapter, + std::unique_ptr range_bitmap); Result> VisitEqual(const Literal& literal) override; Result> VisitNotEqual(const Literal& literal) override; @@ -104,6 +107,7 @@ class RangeBitmapFileIndexReader final Result> VisitGreaterOrEqual(const Literal& literal) override; Result> VisitLessOrEqual(const Literal& literal) override; + std::unique_ptr type_adapter_; std::unique_ptr range_bitmap_; }; diff --git a/src/paimon/common/file_index/rangebitmap/range_bitmap_file_index_test.cpp b/src/paimon/common/file_index/rangebitmap/range_bitmap_file_index_test.cpp index 6157ee020..be3eb0fe8 100644 --- a/src/paimon/common/file_index/rangebitmap/range_bitmap_file_index_test.cpp +++ b/src/paimon/common/file_index/rangebitmap/range_bitmap_file_index_test.cpp @@ -20,13 +20,18 @@ #include +#include #include #include #include #include "arrow/api.h" #include "arrow/c/bridge.h" +#include "arrow/ipc/json_simple.h" #include "paimon/common/utils/arrow/status_utils.h" +#include "paimon/common/utils/math.h" +#include "paimon/data/decimal.h" +#include "paimon/data/timestamp.h" #include "paimon/file_index/bitmap_index_result.h" #include "paimon/file_index/file_index_format.h" #include "paimon/file_index/file_indexer_factory.h" @@ -98,6 +103,17 @@ class RangeBitmapFileIndexTest : public ::testing::Test { const std::set& null_indices, const std::map& options, PAIMON_UNIQUE_PTR* serialized_bytes_out); + Result> CreateReaderFromJson( + const std::shared_ptr& arrow_type, const std::string& json, + const std::map& options, + PAIMON_UNIQUE_PTR* serialized_bytes_out); + + Result> CreateReaderFromArray( + const std::shared_ptr& arrow_type, + const std::shared_ptr& array, + const std::map& options, + PAIMON_UNIQUE_PTR* serialized_bytes_out); + protected: std::shared_ptr pool_; @@ -122,11 +138,18 @@ Result> RangeBitmapFileIndexTest::Cr } std::shared_ptr arrow_array; PAIMON_RETURN_NOT_OK_FROM_ARROW(builder->Finish(&arrow_array)); + return CreateReaderFromArray(arrow_type, arrow_array, options, serialized_bytes_out); +} + +Result> RangeBitmapFileIndexTest::CreateReaderFromArray( + const std::shared_ptr& arrow_type, const std::shared_ptr& array, + const std::map& options, + PAIMON_UNIQUE_PTR* serialized_bytes_out) { // Wrap in StructArray (single field) as required by RangeBitmapFileIndexWriter auto field = arrow::field("test_field", arrow_type); arrow::FieldVector fields = {field}; PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(std::shared_ptr struct_array, - arrow::StructArray::Make({arrow_array}, fields)); + arrow::StructArray::Make({array}, fields)); auto c_array = std::make_unique<::ArrowArray>(); PAIMON_RETURN_NOT_OK_FROM_ARROW(arrow::ExportArray(*struct_array, c_array.get())); // Create writer @@ -149,6 +172,18 @@ Result> RangeBitmapFileIndexTest::Cr return reader; } +Result> RangeBitmapFileIndexTest::CreateReaderFromJson( + const std::shared_ptr& arrow_type, const std::string& json, + const std::map& options, + PAIMON_UNIQUE_PTR* serialized_bytes_out) { + const auto field = arrow::field("test_field", arrow_type); + PAIMON_ASSIGN_OR_RAISE_FROM_ARROW( + std::shared_ptr array, + arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_({field}), json)); + const auto struct_array = checked_pointer_cast(array); + return CreateReaderFromArray(arrow_type, struct_array->field(0), options, serialized_bytes_out); +} + // Test with all NULL values TEST_F(RangeBitmapFileIndexTest, TestAllNullValues) { constexpr int32_t num_rows = 10; @@ -532,6 +567,106 @@ TEST_F(RangeBitmapFileIndexTest, TestWriteAndReadRangeBitmapIndexDouble) { CheckResult(is_not_null_result, all_positions); } +TEST_F(RangeBitmapFileIndexTest, TestFloatingPointSpecialValues) { + const std::vector nan_positions = {0, 1, 2, 5}; + const std::vector negative_zero_positions = {3, 6}; + const std::vector positive_zero_positions = {4, 7}; + const std::vector non_nan_positions = {3, 4, 6, 7, 8, 9, 10, 11}; + const std::vector less_than_positive_zero_positions = {3, 6, 8, 10}; + const std::vector nan_and_negative_zero_positions = {0, 1, 2, 3, 5, 6}; + const std::vector non_nan_and_non_negative_zero_positions = {4, 7, 8, 9, 10, 11}; + + const auto check_reader = [&](const std::shared_ptr& reader, + const std::vector& nan_literals, + const Literal& negative_zero, const Literal& positive_zero, + const Literal& positive_infinity) { + for (const Literal& nan_literal : nan_literals) { + ASSERT_OK_AND_ASSIGN(std::shared_ptr result, + reader->VisitEqual(nan_literal)); + CheckResult(result, nan_positions); + } + + ASSERT_OK_AND_ASSIGN(std::shared_ptr negative_zero_result, + reader->VisitEqual(negative_zero)); + CheckResult(negative_zero_result, negative_zero_positions); + ASSERT_OK_AND_ASSIGN(std::shared_ptr positive_zero_result, + reader->VisitEqual(positive_zero)); + CheckResult(positive_zero_result, positive_zero_positions); + + ASSERT_OK_AND_ASSIGN(std::shared_ptr less_than_nan_result, + reader->VisitLessThan(nan_literals.front())); + CheckResult(less_than_nan_result, non_nan_positions); + ASSERT_OK_AND_ASSIGN(std::shared_ptr greater_than_infinity_result, + reader->VisitGreaterThan(positive_infinity)); + CheckResult(greater_than_infinity_result, nan_positions); + ASSERT_OK_AND_ASSIGN(std::shared_ptr less_than_positive_zero_result, + reader->VisitLessThan(positive_zero)); + CheckResult(less_than_positive_zero_result, less_than_positive_zero_positions); + + const std::vector nan_and_negative_zero = {nan_literals[1], negative_zero}; + ASSERT_OK_AND_ASSIGN(std::shared_ptr in_result, + reader->VisitIn(nan_and_negative_zero)); + CheckResult(in_result, nan_and_negative_zero_positions); + ASSERT_OK_AND_ASSIGN(std::shared_ptr not_in_result, + reader->VisitNotIn(nan_and_negative_zero)); + CheckResult(not_in_result, non_nan_and_non_negative_zero_positions); + }; + + const auto float_nan = FloatingPointFromBits(kCanonicalFloatNaNBits); + const auto float_positive_payload_nan = FloatingPointFromBits(uint32_t{0x7fc12345}); + const auto 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, + -std::numeric_limits::infinity(), + std::numeric_limits::infinity(), + -1.0f, + +1.0f, + }; + PAIMON_UNIQUE_PTR float_serialized_bytes; + ASSERT_OK_AND_ASSIGN(std::shared_ptr float_reader, + (CreateReaderForTest( + arrow::float32(), float_values, &float_serialized_bytes))); + check_reader(float_reader, + {Literal(float_nan), Literal(float_positive_payload_nan), + Literal(float_negative_payload_nan)}, + Literal(-0.0f), Literal(+0.0f), Literal(std::numeric_limits::infinity())); + + const auto double_nan = FloatingPointFromBits(kCanonicalDoubleNaNBits); + const auto double_positive_payload_nan = + FloatingPointFromBits(uint64_t{0x7ff8123456789abc}); + const auto 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, + -std::numeric_limits::infinity(), + std::numeric_limits::infinity(), + -1.0, + +1.0, + }; + PAIMON_UNIQUE_PTR double_serialized_bytes; + ASSERT_OK_AND_ASSIGN(std::shared_ptr double_reader, + (CreateReaderForTest( + arrow::float64(), double_values, &double_serialized_bytes))); + check_reader(double_reader, + {Literal(double_nan), Literal(double_positive_payload_nan), + Literal(double_negative_payload_nan)}, + Literal(-0.0), Literal(+0.0), Literal(std::numeric_limits::infinity())); +} + TEST_F(RangeBitmapFileIndexTest, TestWriteAndReadRangeBitmapIndexDate) { std::vector test_data = {42432, 24649, 42432, 38001, 24649, 50000, 12000}; const auto& arrow_type = arrow::date32(); @@ -580,6 +715,51 @@ TEST_F(RangeBitmapFileIndexTest, TestWriteAndReadRangeBitmapIndexDate) { CheckResult(is_not_null_result, all_positions); } +TEST_F(RangeBitmapFileIndexTest, TestWriteAndReadStringDecimalAndTimestamp) { + { + const auto type = arrow::utf8(); + PAIMON_UNIQUE_PTR serialized_bytes; + ASSERT_OK_AND_ASSIGN( + auto reader, + CreateReaderFromJson(type, R"([["pear"], ["apple"], [null], ["banana"], ["apple"]])", + {{"chunk-size", "12b"}}, &serialized_bytes)); + const Literal apple(FieldType::STRING, "apple", 5); + const Literal banana(FieldType::STRING, "banana", 6); + CheckResult(reader->VisitEqual(apple).value(), {1, 4}); + CheckResult(reader->VisitGreaterOrEqual(banana).value(), {0, 3}); + CheckResult(reader->VisitIsNull().value(), {2}); + } + { + const auto type = arrow::decimal128(10, 2); + PAIMON_UNIQUE_PTR serialized_bytes; + ASSERT_OK_AND_ASSIGN( + auto reader, + CreateReaderFromJson(type, R"([["1.00"], ["2.50"], [null], ["-1.25"], ["2.50"]])", {}, + &serialized_bytes)); + CheckResult(reader->VisitEqual(Literal(Decimal(10, 2, 250))).value(), {1, 4}); + CheckResult(reader->VisitLessThan(Literal(Decimal(10, 2, 0))).value(), {3}); + CheckResult(reader->VisitIsNull().value(), {2}); + } + { + const auto type = arrow::timestamp(arrow::TimeUnit::MICRO); + PAIMON_UNIQUE_PTR serialized_bytes; + ASSERT_OK_AND_ASSIGN( + auto reader, + CreateReaderFromJson(type, R"([[1000001], [2000002], [null], [-1000001], [2000002]])", + {}, &serialized_bytes)); + CheckResult(reader->VisitEqual(Literal(Timestamp(2000, 2000))).value(), {1, 4}); + CheckResult(reader->VisitLessThan(Literal(Timestamp(0, 0))).value(), {3}); + CheckResult(reader->VisitIsNull().value(), {2}); + } + + ASSERT_NOK_WITH_MSG( + RangeBitmapFileIndexWriter::Create(arrow::field("f0", arrow::decimal128(19, 2)), {}, pool_), + "DECIMAL with precision in [1, 18]"); + ASSERT_NOK_WITH_MSG(RangeBitmapFileIndexWriter::Create( + arrow::field("f0", arrow::timestamp(arrow::TimeUnit::NANO)), {}, pool_), + "TIMESTAMP with precision in [0, 6]"); +} + TEST_F(RangeBitmapFileIndexTest, TestRangeBitmapEdgeCases) { // Scope 1: All values identical { diff --git a/src/paimon/common/file_index/rangebitmap/range_bitmap_type_adapter.cpp b/src/paimon/common/file_index/rangebitmap/range_bitmap_type_adapter.cpp new file mode 100644 index 000000000..e9157ec16 --- /dev/null +++ b/src/paimon/common/file_index/rangebitmap/range_bitmap_type_adapter.cpp @@ -0,0 +1,113 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "paimon/common/file_index/rangebitmap/range_bitmap_type_adapter.h" + +#include "arrow/type.h" +#include "fmt/format.h" +#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/data/decimal.h" +#include "paimon/data/timestamp.h" +#include "paimon/status.h" + +namespace paimon { + +Result> RangeBitmapTypeAdapter::Create( + const std::shared_ptr& arrow_type) { + PAIMON_ASSIGN_OR_RAISE(FieldType field_type, + FieldTypeUtils::ConvertToFieldType(arrow_type->id())); + if (field_type == FieldType::DECIMAL) { + const auto decimal_type = checked_pointer_cast(arrow_type); + if (decimal_type->precision() > 18) { + return Status::Invalid(fmt::format( + "range-bitmap index only supports DECIMAL with precision in [1, 18], got {}", + decimal_type->precision())); + } + return std::unique_ptr( + new RangeBitmapTypeAdapter(field_type, FieldType::BIGINT, std::nullopt)); + } + if (field_type == FieldType::TIMESTAMP) { + const auto timestamp_type = checked_pointer_cast(arrow_type); + const int32_t precision = DateTimeUtils::GetPrecisionFromType(timestamp_type); + if (precision > 6) { + return Status::Invalid(fmt::format( + "range-bitmap index only supports TIMESTAMP with precision in [0, 6], got {}", + precision)); + } + return std::unique_ptr( + new RangeBitmapTypeAdapter(field_type, FieldType::BIGINT, precision)); + } + return std::unique_ptr( + new RangeBitmapTypeAdapter(field_type, field_type, std::nullopt)); +} + +FieldType RangeBitmapTypeAdapter::GetStorageType() const { + return storage_type_; +} + +Result RangeBitmapTypeAdapter::ToStorageLiteral(const Literal& literal) const { + if (literal.IsNull()) { + return Literal(storage_type_); + } + if (logical_type_ == FieldType::DECIMAL) { + if (literal.GetType() != FieldType::DECIMAL) { + return Status::Invalid("range-bitmap DECIMAL field requires a DECIMAL literal"); + } + return Literal(literal.GetValue().ToUnscaledLong()); + } + if (logical_type_ == FieldType::TIMESTAMP) { + if (literal.GetType() != FieldType::TIMESTAMP) { + return Status::Invalid("range-bitmap TIMESTAMP field requires a TIMESTAMP literal"); + } + if (!timestamp_precision_.has_value()) { + return Status::Invalid("range-bitmap TIMESTAMP adapter is missing precision"); + } + const auto value = literal.GetValue(); + return Literal(*timestamp_precision_ <= Timestamp::MILLIS_PRECISION + ? value.GetMillisecond() + : value.ToMicrosecond()); + } + if (literal.GetType() != storage_type_) { + return Status::Invalid( + fmt::format("range-bitmap literal type {} does not match field type {}", + FieldTypeUtils::FieldTypeToString(literal.GetType()), + FieldTypeUtils::FieldTypeToString(storage_type_))); + } + return literal; +} + +Result> RangeBitmapTypeAdapter::ToStorageLiterals( + const std::vector& literals) const { + std::vector converted_literals; + converted_literals.reserve(literals.size()); + for (const Literal& literal : literals) { + PAIMON_ASSIGN_OR_RAISE(Literal converted_literal, ToStorageLiteral(literal)); + converted_literals.emplace_back(std::move(converted_literal)); + } + return converted_literals; +} + +RangeBitmapTypeAdapter::RangeBitmapTypeAdapter(FieldType logical_type, FieldType storage_type, + std::optional timestamp_precision) + : logical_type_(logical_type), + storage_type_(storage_type), + timestamp_precision_(timestamp_precision) {} + +} // namespace paimon diff --git a/src/paimon/common/file_index/rangebitmap/range_bitmap_type_adapter.h b/src/paimon/common/file_index/rangebitmap/range_bitmap_type_adapter.h new file mode 100644 index 000000000..bd9cd5bbf --- /dev/null +++ b/src/paimon/common/file_index/rangebitmap/range_bitmap_type_adapter.h @@ -0,0 +1,56 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include +#include + +#include "paimon/defs.h" +#include "paimon/predicate/literal.h" +#include "paimon/result.h" + +namespace arrow { +class DataType; +} // namespace arrow + +namespace paimon { + +/// Adapts logical field values to the physical key type stored by range-bitmap indexes. +class RangeBitmapTypeAdapter { + public: + static Result> Create( + const std::shared_ptr& arrow_type); + + FieldType GetStorageType() const; + + Result ToStorageLiteral(const Literal& literal) const; + + Result> ToStorageLiterals(const std::vector& literals) const; + + private: + RangeBitmapTypeAdapter(FieldType logical_type, FieldType storage_type, + std::optional timestamp_precision); + + FieldType logical_type_; + FieldType storage_type_; + std::optional timestamp_precision_; +}; + +} // namespace paimon diff --git a/src/paimon/common/file_index/rangebitmap/range_bitmap_type_adapter_test.cpp b/src/paimon/common/file_index/rangebitmap/range_bitmap_type_adapter_test.cpp new file mode 100644 index 000000000..90f7dfbbc --- /dev/null +++ b/src/paimon/common/file_index/rangebitmap/range_bitmap_type_adapter_test.cpp @@ -0,0 +1,109 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "paimon/common/file_index/rangebitmap/range_bitmap_type_adapter.h" + +#include + +#include "arrow/api.h" +#include "paimon/data/decimal.h" +#include "paimon/data/timestamp.h" +#include "paimon/testing/utils/testharness.h" + +namespace paimon::test { + +TEST(RangeBitmapTypeAdapterTest, TestStorageType) { + ASSERT_OK_AND_ASSIGN(std::unique_ptr int_adapter, + RangeBitmapTypeAdapter::Create(arrow::int32())); + ASSERT_EQ(FieldType::INT, int_adapter->GetStorageType()); + + ASSERT_OK_AND_ASSIGN(std::unique_ptr string_adapter, + RangeBitmapTypeAdapter::Create(arrow::utf8())); + ASSERT_EQ(FieldType::STRING, string_adapter->GetStorageType()); + + ASSERT_OK_AND_ASSIGN(std::unique_ptr decimal_adapter, + RangeBitmapTypeAdapter::Create(arrow::decimal128(18, 2))); + ASSERT_EQ(FieldType::BIGINT, decimal_adapter->GetStorageType()); + + ASSERT_OK_AND_ASSIGN(std::unique_ptr timestamp_adapter, + RangeBitmapTypeAdapter::Create(arrow::timestamp(arrow::TimeUnit::MICRO))); + ASSERT_EQ(FieldType::BIGINT, timestamp_adapter->GetStorageType()); + + ASSERT_NOK_WITH_MSG(RangeBitmapTypeAdapter::Create(arrow::decimal128(19, 2)), + "DECIMAL with precision in [1, 18]"); + ASSERT_NOK_WITH_MSG(RangeBitmapTypeAdapter::Create(arrow::timestamp(arrow::TimeUnit::NANO)), + "TIMESTAMP with precision in [0, 6]"); +} + +TEST(RangeBitmapTypeAdapterTest, TestDecimalLiteralConversion) { + ASSERT_OK_AND_ASSIGN(std::unique_ptr adapter, + RangeBitmapTypeAdapter::Create(arrow::decimal128(10, 2))); + + ASSERT_OK_AND_ASSIGN(Literal converted, + adapter->ToStorageLiteral(Literal(Decimal(10, 2, 12345)))); + ASSERT_EQ(FieldType::BIGINT, converted.GetType()); + ASSERT_EQ(12345, converted.GetValue()); + + ASSERT_OK_AND_ASSIGN(Literal converted_null, + adapter->ToStorageLiteral(Literal(FieldType::DECIMAL))); + ASSERT_EQ(FieldType::BIGINT, converted_null.GetType()); + ASSERT_TRUE(converted_null.IsNull()); + + ASSERT_NOK_WITH_MSG(adapter->ToStorageLiteral(Literal(int64_t{12345})), + "DECIMAL field requires a DECIMAL literal"); +} + +TEST(RangeBitmapTypeAdapterTest, TestTimestampLiteralConversion) { + const Timestamp timestamp(1234, 567000); + ASSERT_OK_AND_ASSIGN(std::unique_ptr millis_adapter, + RangeBitmapTypeAdapter::Create(arrow::timestamp(arrow::TimeUnit::MILLI))); + ASSERT_OK_AND_ASSIGN(Literal millis, millis_adapter->ToStorageLiteral(Literal(timestamp))); + ASSERT_EQ(FieldType::BIGINT, millis.GetType()); + ASSERT_EQ(1234, millis.GetValue()); + + ASSERT_OK_AND_ASSIGN(std::unique_ptr micros_adapter, + RangeBitmapTypeAdapter::Create(arrow::timestamp(arrow::TimeUnit::MICRO))); + ASSERT_OK_AND_ASSIGN(Literal micros, micros_adapter->ToStorageLiteral(Literal(timestamp))); + ASSERT_EQ(FieldType::BIGINT, micros.GetType()); + ASSERT_EQ(1234567, micros.GetValue()); + + ASSERT_OK_AND_ASSIGN(Literal converted_null, + micros_adapter->ToStorageLiteral(Literal(FieldType::TIMESTAMP))); + ASSERT_EQ(FieldType::BIGINT, converted_null.GetType()); + ASSERT_TRUE(converted_null.IsNull()); + + ASSERT_NOK_WITH_MSG(micros_adapter->ToStorageLiteral(Literal(int64_t{1234567})), + "TIMESTAMP field requires a TIMESTAMP literal"); +} + +TEST(RangeBitmapTypeAdapterTest, TestLiteralBatchConversion) { + ASSERT_OK_AND_ASSIGN(std::unique_ptr adapter, + RangeBitmapTypeAdapter::Create(arrow::int32())); + const std::vector literals = {Literal(int32_t{1}), Literal(FieldType::INT), + Literal(int32_t{3})}; + ASSERT_OK_AND_ASSIGN(std::vector converted, adapter->ToStorageLiterals(literals)); + ASSERT_EQ(3, converted.size()); + ASSERT_EQ(1, converted[0].GetValue()); + ASSERT_TRUE(converted[1].IsNull()); + ASSERT_EQ(3, converted[2].GetValue()); + + ASSERT_NOK_WITH_MSG(adapter->ToStorageLiterals({Literal(int32_t{1}), Literal(int64_t{2})}), + "literal type BIGINT does not match field type INT"); +} + +} // namespace paimon::test From f30dd14d0905bebdff54e057b3b3d20c77bc938b Mon Sep 17 00:00:00 2001 From: "jinli.zjw" Date: Wed, 2 Sep 2026 18:49:43 +0800 Subject: [PATCH 2/4] fix(predicate): validate decimal literal scale --- include/paimon/file_index/file_index_reader.h | 2 + .../paimon/global_index/global_index_reader.h | 2 + ...bit_slice_index_bitmap_file_index_test.cpp | 4 ++ .../range_bitmap_file_index_test.cpp | 5 ++ .../btree_global_index_integration_test.cpp | 8 +++ .../common/predicate/predicate_validator.h | 33 ++++++++- .../predicate/predicate_validator_test.cpp | 68 +++++++++++++++++-- .../core/operation/internal_read_context.cpp | 4 +- src/paimon/core/table/source/table_scan.cpp | 4 +- test/inte/global_index_test.cpp | 8 +++ 10 files changed, 129 insertions(+), 9 deletions(-) diff --git a/include/paimon/file_index/file_index_reader.h b/include/paimon/file_index/file_index_reader.h index ff1d31d16..2da36fd2e 100644 --- a/include/paimon/file_index/file_index_reader.h +++ b/include/paimon/file_index/file_index_reader.h @@ -34,6 +34,8 @@ namespace paimon { /// `std::shared_ptr` objects. It reads pre-built file-level index data /// (e.g., bitmap, bsi or bloom filters) from index file and evaluates /// whether a given data file may contain rows matching a specific predicate. +/// @note Callers of `Visit*` for DECIMAL fields must ensure each literal's scale matches the scale +/// of the indexed data; otherwise, index filtering results may be incorrect. class PAIMON_EXPORT FileIndexReader : public FunctionVisitor> { public: Result> VisitIsNotNull() override; diff --git a/include/paimon/global_index/global_index_reader.h b/include/paimon/global_index/global_index_reader.h index ca3f4c9b9..257924652 100644 --- a/include/paimon/global_index/global_index_reader.h +++ b/include/paimon/global_index/global_index_reader.h @@ -35,6 +35,8 @@ namespace paimon { /// Derived classes are expected to implement the visitor methods (e.g., `VisitEqual`, /// `VisitIsNull`, etc.) to return index-based results that indicate which /// rows satisfy the given predicate. +/// @note Callers of `Visit*` for DECIMAL fields must ensure each literal's scale matches the scale +/// of the indexed data; otherwise, index filtering results may be incorrect. class PAIMON_EXPORT GlobalIndexReader : public FunctionVisitor> { public: /// VisitVectorSearch performs approximate vector similarity search. diff --git a/src/paimon/common/file_index/bsi/bit_slice_index_bitmap_file_index_test.cpp b/src/paimon/common/file_index/bsi/bit_slice_index_bitmap_file_index_test.cpp index 2130164d8..8da7ace29 100644 --- a/src/paimon/common/file_index/bsi/bit_slice_index_bitmap_file_index_test.cpp +++ b/src/paimon/common/file_index/bsi/bit_slice_index_bitmap_file_index_test.cpp @@ -437,6 +437,10 @@ TEST_F(BitSliceIndexBitmapIndexReaderTest, TestDecimalType) { CheckResult(reader->VisitLessThan(Literal(Decimal(10, 2, 0))).value(), {3}); CheckResult(reader->VisitIsNull().value(), {2}); + // BSI does not rescale Decimal literals. A mathematically equivalent literal with a + // different scale produces an incorrect empty result, so callers must use the field's scale. + CheckResult(reader->VisitEqual(Literal(Decimal(10, 3, 2500))).value(), {}); + // test invalid case for decimal128(20, 0) which exceeds int64 range ASSERT_NOK_WITH_MSG(WriteIndex(arrow::decimal128(20, 0), R"([["9223372036854775808"]])"), "does not fit in int64 for bsi index"); diff --git a/src/paimon/common/file_index/rangebitmap/range_bitmap_file_index_test.cpp b/src/paimon/common/file_index/rangebitmap/range_bitmap_file_index_test.cpp index be3eb0fe8..794f25c5b 100644 --- a/src/paimon/common/file_index/rangebitmap/range_bitmap_file_index_test.cpp +++ b/src/paimon/common/file_index/rangebitmap/range_bitmap_file_index_test.cpp @@ -739,6 +739,11 @@ TEST_F(RangeBitmapFileIndexTest, TestWriteAndReadStringDecimalAndTimestamp) { CheckResult(reader->VisitEqual(Literal(Decimal(10, 2, 250))).value(), {1, 4}); CheckResult(reader->VisitLessThan(Literal(Decimal(10, 2, 0))).value(), {3}); CheckResult(reader->VisitIsNull().value(), {2}); + + // Range Bitmap does not rescale Decimal literals. A mathematically equivalent literal + // with a different scale produces an incorrect empty result, so callers must use the + // field's scale. + CheckResult(reader->VisitEqual(Literal(Decimal(10, 3, 2500))).value(), {}); } { const auto type = arrow::timestamp(arrow::TimeUnit::MICRO); diff --git a/src/paimon/common/global_index/btree/btree_global_index_integration_test.cpp b/src/paimon/common/global_index/btree/btree_global_index_integration_test.cpp index 653b617d3..ce25b9b4e 100644 --- a/src/paimon/common/global_index/btree/btree_global_index_integration_test.cpp +++ b/src/paimon/common/global_index/btree/btree_global_index_integration_test.cpp @@ -1330,6 +1330,14 @@ TEST_P(BTreeGlobalIndexIntegrationTest, WriteAndReadDecimalCompactData) { ASSERT_OK_AND_ASSIGN(auto result, reader->VisitEqual(lit_250)); CheckResult(result, {2, 3}); } + { + // BTree does not rescale Decimal literals. A mathematically equivalent literal with a + // different scale produces an incorrect empty result, so callers must use the field's + // scale. + Literal lit_2500(Decimal::FromUnscaledLong(2500, 10, 3)); + ASSERT_OK_AND_ASSIGN(auto result, reader->VisitEqual(lit_2500)); + CheckResult(result, {}); + } { Literal lit_250(Decimal::FromUnscaledLong(250, 10, 2)); ASSERT_OK_AND_ASSIGN(auto result, reader->VisitNotEqual(lit_250)); diff --git a/src/paimon/common/predicate/predicate_validator.h b/src/paimon/common/predicate/predicate_validator.h index 3242d4bc1..96bc33b7a 100644 --- a/src/paimon/common/predicate/predicate_validator.h +++ b/src/paimon/common/predicate/predicate_validator.h @@ -24,8 +24,11 @@ #include #include "arrow/type.h" +#include "arrow/util/decimal.h" #include "fmt/format.h" +#include "paimon/common/utils/checked_cast.h" #include "paimon/common/utils/field_type_utils.h" +#include "paimon/data/decimal.h" #include "paimon/defs.h" #include "paimon/predicate/compound_predicate.h" #include "paimon/predicate/leaf_predicate.h" @@ -86,8 +89,14 @@ class PredicateValidator { field_name, schema_field_idx, leaf_predicate->FieldIndex())); } // check field type (schema vs. predicate) + const std::shared_ptr& schema_type = + schema.field(schema_field_idx)->type(); PAIMON_RETURN_NOT_OK(ValidateDataTypeWithSchemaAndPredicate( - *schema.field(schema_field_idx)->type(), leaf_predicate->GetFieldType())); + *schema_type, leaf_predicate->GetFieldType())); + if (schema_type->id() == arrow::Type::DECIMAL128) { + PAIMON_RETURN_NOT_OK(ValidateDecimalLiterals( + *checked_pointer_cast(schema_type), *leaf_predicate)); + } } else if (auto compound_predicate = std::dynamic_pointer_cast(predicate)) { const auto& children = compound_predicate->Children(); @@ -100,6 +109,28 @@ class PredicateValidator { } private: + static Status ValidateDecimalLiterals(const arrow::Decimal128Type& field_type, + const LeafPredicate& predicate) { + const std::string& field_name = predicate.FieldName(); + for (const Literal& literal : predicate.Literals()) { + const auto decimal = literal.GetValue(); + if (decimal.Scale() != field_type.scale()) { + return Status::Invalid(fmt::format( + "decimal literal for field {} has scale {}, expected {}; rescale the literal " + "before building the predicate", + field_name, decimal.Scale(), field_type.scale())); + } + + arrow::Decimal128 unscaled_value(decimal.HighBits(), decimal.LowBits()); + if (!unscaled_value.FitsInPrecision(field_type.precision())) { + return Status::Invalid(fmt::format( + "decimal literal {} for field {} does not fit field type DECIMAL({}, {})", + decimal.ToString(), field_name, field_type.precision(), field_type.scale())); + } + } + return Status::OK(); + } + static Status ValidateDataTypeWithSchemaAndPredicate(const arrow::DataType& schema_type, const FieldType& field_type) { const auto kind = schema_type.id(); diff --git a/src/paimon/common/predicate/predicate_validator_test.cpp b/src/paimon/common/predicate/predicate_validator_test.cpp index d83420758..c0dba4dcd 100644 --- a/src/paimon/common/predicate/predicate_validator_test.cpp +++ b/src/paimon/common/predicate/predicate_validator_test.cpp @@ -158,8 +158,7 @@ TEST(PredicateValidatorTest, TestValidateSchema) { /*validate_field_idx=*/true)); } { - // f2 schema type is DECIMAL(23,5), predicate type can be different precision and scale, - // such as DECIMAL(22,4) + // f2 schema type is DECIMAL(23,5), but the literal scale is 4. std::shared_ptr schema = arrow::schema(arrow::FieldVector({ arrow::field("f0", arrow::int16()), arrow::field("f1", arrow::float32()), @@ -180,8 +179,11 @@ TEST(PredicateValidatorTest, TestValidateSchema) { Literal(true)), })); ASSERT_OK(PredicateValidator::ValidatePredicateWithLiterals(predicate)); - ASSERT_OK(PredicateValidator::ValidatePredicateWithSchema(*schema, predicate, - /*validate_field_idx=*/true)); + ASSERT_NOK_WITH_MSG( + PredicateValidator::ValidatePredicateWithSchema(*schema, predicate, + /*validate_field_idx=*/true), + "decimal literal for field f2 has scale 4, expected 5; rescale the literal before " + "building the predicate"); } { // predicate field idx mismatch @@ -341,4 +343,62 @@ TEST(PredicateValidatorTest, TestValidateSchema) { "field f2 does not exist in schema"); } } + +TEST(PredicateValidatorTest, TestValidateDecimalLiteral) { + std::shared_ptr schema = + arrow::schema({arrow::field("amount", arrow::decimal128(10, 2))}); + + { + auto predicate = + PredicateBuilder::Equal(/*field_index=*/0, /*field_name=*/"amount", FieldType::DECIMAL, + Literal(Decimal(10, 2, 12345))); + ASSERT_OK(PredicateValidator::ValidatePredicateWithSchema(*schema, predicate, + /*validate_field_idx=*/true)); + } + { + // Literal precision metadata may be smaller than the field precision. + auto predicate = PredicateBuilder::Equal(/*field_index=*/0, /*field_name=*/"amount", + FieldType::DECIMAL, Literal(Decimal(9, 2, 12345))); + ASSERT_OK(PredicateValidator::ValidatePredicateWithSchema(*schema, predicate, + /*validate_field_idx=*/true)); + } + { + // Literal precision metadata may be larger than the field precision if the value fits. + auto predicate = + PredicateBuilder::Equal(/*field_index=*/0, /*field_name=*/"amount", FieldType::DECIMAL, + Literal(Decimal(12, 2, 12345))); + ASSERT_OK(PredicateValidator::ValidatePredicateWithSchema(*schema, predicate, + /*validate_field_idx=*/true)); + } + { + auto predicate = + PredicateBuilder::Equal(/*field_index=*/0, /*field_name=*/"amount", FieldType::DECIMAL, + Literal(Decimal(10, 1, 12345))); + ASSERT_NOK_WITH_MSG( + PredicateValidator::ValidatePredicateWithSchema(*schema, predicate, + /*validate_field_idx=*/true), + "decimal literal for field amount has scale 1, expected 2; rescale the literal before " + "building the predicate"); + } + { + auto predicate = PredicateBuilder::In( + /*field_index=*/0, /*field_name=*/"amount", FieldType::DECIMAL, + {Literal(Decimal(10, 2, 12345)), Literal(Decimal(10, 3, 123450))}); + ASSERT_NOK_WITH_MSG( + PredicateValidator::ValidatePredicateWithSchema(*schema, predicate, + /*validate_field_idx=*/true), + "decimal literal for field amount has scale 3, expected 2; rescale the literal before " + "building the predicate"); + } + { + auto predicate = + PredicateBuilder::Equal(/*field_index=*/0, /*field_name=*/"amount", FieldType::DECIMAL, + Literal(Decimal(10, 2, 10000000000LL))); + ASSERT_NOK_WITH_MSG( + PredicateValidator::ValidatePredicateWithSchema(*schema, predicate, + /*validate_field_idx=*/true), + "decimal literal 100000000.00 for field amount does not fit field type DECIMAL(10, " + "2)"); + } +} } // namespace paimon::test diff --git a/src/paimon/core/operation/internal_read_context.cpp b/src/paimon/core/operation/internal_read_context.cpp index dd60ce404..28247824a 100644 --- a/src/paimon/core/operation/internal_read_context.cpp +++ b/src/paimon/core/operation/internal_read_context.cpp @@ -276,10 +276,10 @@ Result> InternalReadContext::Create( } // validate predicate if (context->GetPredicate()) { - PAIMON_RETURN_NOT_OK(PredicateValidator::ValidatePredicateWithSchema( - *read_schema, context->GetPredicate(), /*validate_field_idx=*/true)); PAIMON_RETURN_NOT_OK( PredicateValidator::ValidatePredicateWithLiterals(context->GetPredicate())); + PAIMON_RETURN_NOT_OK(PredicateValidator::ValidatePredicateWithSchema( + *read_schema, context->GetPredicate(), /*validate_field_idx=*/true)); } if (!context->GetMemoryPool()) { diff --git a/src/paimon/core/table/source/table_scan.cpp b/src/paimon/core/table/source/table_scan.cpp index 95af2a23b..64baf9028 100644 --- a/src/paimon/core/table/source/table_scan.cpp +++ b/src/paimon/core/table/source/table_scan.cpp @@ -309,11 +309,11 @@ Result> NewDataTableScan(const std::shared_ptrFields()); if (context->GetScanFilters() && context->GetScanFilters()->GetPredicate()) { + PAIMON_RETURN_NOT_OK(PredicateValidator::ValidatePredicateWithLiterals( + context->GetScanFilters()->GetPredicate())); PAIMON_RETURN_NOT_OK(PredicateValidator::ValidatePredicateWithSchema( *arrow_schema, context->GetScanFilters()->GetPredicate(), /*validate_field_idx=*/false)); - PAIMON_RETURN_NOT_OK(PredicateValidator::ValidatePredicateWithLiterals( - context->GetScanFilters()->GetPredicate())); } PAIMON_ASSIGN_OR_RAISE(std::vector external_paths, core_options.CreateExternalPaths()); diff --git a/test/inte/global_index_test.cpp b/test/inte/global_index_test.cpp index b4ac8b83a..1ba231247 100644 --- a/test/inte/global_index_test.cpp +++ b/test/inte/global_index_test.cpp @@ -3290,6 +3290,14 @@ TEST_P(GlobalIndexTest, TestBTreeScanWithPartitionWithMultiMeta) { auto gt_mid, reader->VisitGreaterThan(Literal(Decimal::FromUnscaledLong(10 * 123456L, 18, 6)))); ASSERT_EQ(count_rows(gt_mid), 18); + + // Global index readers do not rescale Decimal literals. A mathematically equivalent + // literal with a different scale produces an incorrect empty result, so callers must use + // the field's scale. + ASSERT_OK_AND_ASSIGN(auto eq_same_value_different_scale, + reader->VisitEqual(Literal(Decimal::FromUnscaledLong( + 5 * 1234560L, /*precision=*/18, /*scale=*/7)))); + ASSERT_EQ(count_rows(eq_same_value_different_scale), 0); } // ---- col_string (values are "str_00000" .. "str_00019") ---- From ed65b7e7132e5be189587ccec1d4cce5e192e251 Mon Sep 17 00:00:00 2001 From: "jinli.zjw" Date: Wed, 2 Sep 2026 21:31:38 +0800 Subject: [PATCH 3/4] test(predicate): align literals with validation --- test/inte/blob_table_inte_test.cpp | 2 +- test/inte/read_inte_test.cpp | 2 +- test/inte/scan_inte_test.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/inte/blob_table_inte_test.cpp b/test/inte/blob_table_inte_test.cpp index b26c278e4..b19be8db3 100644 --- a/test/inte/blob_table_inte_test.cpp +++ b/test/inte/blob_table_inte_test.cpp @@ -2192,7 +2192,7 @@ TEST_P(BlobTableInteTest, TestPartitionWithPredicate) { // set partition predicate and data field predicate, blob type not support predicate auto equal = PredicateBuilder::Equal(/*field_index=*/1, /*field_name=*/"f1", FieldType::STRING, - Literal(FieldType::BLOB, "2024", 4)); + Literal(FieldType::STRING, "2024", 4)); auto greater_than = PredicateBuilder::GreaterThan(/*field_index=*/0, /*field_name=*/"f0", FieldType::INT, Literal(100)); ASSERT_OK_AND_ASSIGN(auto predicate, PredicateBuilder::And({equal, greater_than})); diff --git a/test/inte/read_inte_test.cpp b/test/inte/read_inte_test.cpp index a22e3a104..652df664e 100644 --- a/test/inte/read_inte_test.cpp +++ b/test/inte/read_inte_test.cpp @@ -2081,7 +2081,7 @@ TEST_P(ReadInteTest, TestAppendReadWithComplexTypePredicate) { PredicateBuilder::And( {PredicateBuilder::Or( {PredicateBuilder::GreaterThan(/*field_index=*/4, /*field_name=*/"f5", - FieldType::DECIMAL, Literal(Decimal(5, 2, 0))), + FieldType::DECIMAL, Literal(Decimal(23, 5, 0))), PredicateBuilder::LessThan(/*field_index=*/2, /*field_name=*/"f4", FieldType::TIMESTAMP, Literal(Timestamp(-2240521239999l, 1002))), diff --git a/test/inte/scan_inte_test.cpp b/test/inte/scan_inte_test.cpp index f626dba6b..3f949484d 100644 --- a/test/inte/scan_inte_test.cpp +++ b/test/inte/scan_inte_test.cpp @@ -1452,7 +1452,7 @@ TEST_P(ScanInteTest, TestScanAppendComplexDataWithSnapshot4WithPredicateFilter) Literal(paimon::Timestamp(1735344000, 0))); auto predicate2 = PredicateBuilder::GreaterThan( /*field_index=*/4, /*field_name=*/"f5", FieldType::DECIMAL, - Literal(paimon::Decimal(5, 2, 0))); + Literal(paimon::Decimal(23, 5, 0))); ASSERT_OK_AND_ASSIGN(auto predicate, PredicateBuilder::And({predicate1, predicate2})); ScanContextBuilder context_builder(table_path); From 83ec2264a4dd50a163ce507e1fc0fd50679d3a58 Mon Sep 17 00:00:00 2001 From: "jinli.zjw" Date: Wed, 2 Sep 2026 21:47:16 +0800 Subject: [PATCH 4/4] test(predicate): use blob type in predicate test --- test/inte/blob_table_inte_test.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/inte/blob_table_inte_test.cpp b/test/inte/blob_table_inte_test.cpp index b19be8db3..974b0779d 100644 --- a/test/inte/blob_table_inte_test.cpp +++ b/test/inte/blob_table_inte_test.cpp @@ -2190,9 +2190,8 @@ TEST_P(BlobTableInteTest, TestPartitionWithPredicate) { } { // set partition predicate and data field predicate, blob type not support predicate - auto equal = - PredicateBuilder::Equal(/*field_index=*/1, /*field_name=*/"f1", FieldType::STRING, - Literal(FieldType::STRING, "2024", 4)); + auto equal = PredicateBuilder::Equal(/*field_index=*/1, /*field_name=*/"f1", + FieldType::BLOB, Literal(FieldType::BLOB, "2024", 4)); auto greater_than = PredicateBuilder::GreaterThan(/*field_index=*/0, /*field_name=*/"f0", FieldType::INT, Literal(100)); ASSERT_OK_AND_ASSIGN(auto predicate, PredicateBuilder::And({equal, greater_than}));