Skip to content

Commit 5ebbaf3

Browse files
author
wangyong.alen
committed
fix(scan): preserve decimal and NaN bucket matches
1 parent f40a71e commit 5ebbaf3

4 files changed

Lines changed: 121 additions & 2 deletions

File tree

docs/source/api/scan.rst

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,14 @@ all bucket keys with equality, and bucket-unaware tables, keep the existing scan
3232
behavior. Inferred pruning applies only to files matching the scan schema and
3333
bucket count; older layouts retain the existing filtering behavior.
3434

35+
This inference prunes data files at the manifest-entry level. It does not enable
36+
manifest min/max-bucket skipping or the bucket-specific live-manifest-entry cache,
37+
which require an explicit bucket filter.
38+
39+
Decimal literals are rescaled to the bucket field's type only when the conversion
40+
is exact. NaN literals and decimals that cannot be represented exactly disable
41+
inferred bucket pruning.
42+
3543
Interface
3644
=========
3745

src/paimon/core/bucket/bucket_select_converter.cpp

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "paimon/core/bucket/bucket_select_converter.h"
2020

2121
#include <cassert>
22+
#include <cmath>
2223
#include <set>
2324
#include <string>
2425
#include <utility>
@@ -34,6 +35,7 @@
3435
#include "paimon/core/bucket/default_bucket_function.h"
3536
#include "paimon/core/bucket/hive_bucket_function.h"
3637
#include "paimon/core/bucket/mod_bucket_function.h"
38+
#include "paimon/core/casting/decimal_to_decimal_cast_executor.h"
3739
#include "paimon/data/timestamp.h"
3840
#include "paimon/memory/memory_pool.h"
3941
#include "paimon/predicate/leaf_predicate.h"
@@ -79,7 +81,21 @@ Result<std::optional<int32_t>> BucketSelectConverter::Convert(
7981

8082
for (int32_t i = 0; i < num_fields; i++) {
8183
const auto& field_name = bucket_key_names[i];
82-
const auto& literal = literals_map.at(field_name);
84+
Literal literal = literals_map.at(field_name);
85+
// Equal NaNs can have different stored bit patterns and therefore different buckets.
86+
if ((bucket_key_types[i] == FieldType::FLOAT && std::isnan(literal.GetValue<float>())) ||
87+
(bucket_key_types[i] == FieldType::DOUBLE && std::isnan(literal.GetValue<double>()))) {
88+
return std::optional<int32_t>(std::nullopt);
89+
}
90+
if (bucket_key_types[i] == FieldType::DECIMAL) {
91+
PAIMON_ASSIGN_OR_RAISE(Literal scaled, DecimalToDecimalCastExecutor().Cast(
92+
literal, bucket_key_arrow_types[i]));
93+
// Hash the field's representation only when rescaling preserves the exact value.
94+
if (scaled.IsNull() || !(scaled.GetValue<Decimal>() == literal.GetValue<Decimal>())) {
95+
return std::optional<int32_t>(std::nullopt);
96+
}
97+
literal = std::move(scaled);
98+
}
8399
PAIMON_RETURN_NOT_OK(
84100
WriteLiteralToRow(i, literal, bucket_key_types[i], bucket_key_arrow_types[i], &writer));
85101
}

src/paimon/core/bucket/bucket_select_converter_test.cpp

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

1919
#include "paimon/core/bucket/bucket_select_converter.h"
2020

21+
#include <limits>
2122
#include <optional>
2223
#include <string>
2324
#include <vector>
@@ -246,6 +247,55 @@ TEST_F(BucketSelectConverterTest, HiveBucketFunctionWithDecimal) {
246247
ASSERT_EQ(function->Bucket(row, num_buckets), selected_bucket.value());
247248
}
248249

250+
TEST_F(BucketSelectConverterTest, RescalesDecimalLiteralsExactly) {
251+
for (int32_t precision : {10, 20}) {
252+
for (BucketFunctionType function_type :
253+
{BucketFunctionType::DEFAULT, BucketFunctionType::HIVE}) {
254+
Decimal stored = Decimal::FromUnscaledLong(120, precision, 2);
255+
auto stored_predicate =
256+
PredicateBuilder::Equal(0, "amount", FieldType::DECIMAL, Literal(stored));
257+
ASSERT_OK_AND_ASSIGN(auto expected,
258+
BucketSelectConverter::Convert(stored_predicate, {"amount"},
259+
{arrow::decimal128(precision, 2)},
260+
function_type, 17, pool_.get()));
261+
ASSERT_TRUE(expected.has_value());
262+
for (const auto& query : {Decimal::FromUnscaledLong(12, precision, 1),
263+
Decimal::FromUnscaledLong(1200, precision, 3)}) {
264+
auto predicate =
265+
PredicateBuilder::Equal(0, "amount", FieldType::DECIMAL, Literal(query));
266+
ASSERT_OK_AND_ASSIGN(
267+
auto result, BucketSelectConverter::Convert(predicate, {"amount"},
268+
{arrow::decimal128(precision, 2)},
269+
function_type, 17, pool_.get()));
270+
ASSERT_EQ(result, expected);
271+
}
272+
}
273+
}
274+
}
275+
276+
TEST_F(BucketSelectConverterTest, InexactDecimalConversionReturnsNullopt) {
277+
for (const auto& value :
278+
{Decimal::FromUnscaledLong(123, 10, 3), Decimal::FromUnscaledLong(9999999999LL, 10, 0)}) {
279+
auto predicate = PredicateBuilder::Equal(0, "amount", FieldType::DECIMAL, Literal(value));
280+
ASSERT_OK_AND_ASSIGN(auto result, BucketSelectConverter::Convert(
281+
predicate, {"amount"}, {arrow::decimal128(10, 2)},
282+
BucketFunctionType::DEFAULT, 17, pool_.get()));
283+
ASSERT_FALSE(result.has_value());
284+
}
285+
}
286+
287+
TEST_F(BucketSelectConverterTest, NaNReturnsNullopt) {
288+
for (const auto& value : {Literal(std::numeric_limits<float>::quiet_NaN()),
289+
Literal(std::numeric_limits<double>::quiet_NaN())}) {
290+
auto type = value.GetType() == FieldType::FLOAT ? arrow::float32() : arrow::float64();
291+
auto predicate = PredicateBuilder::Equal(0, "key", value.GetType(), value);
292+
ASSERT_OK_AND_ASSIGN(auto result, BucketSelectConverter::Convert(
293+
predicate, {"key"}, {type},
294+
BucketFunctionType::DEFAULT, 17, pool_.get()));
295+
ASSERT_FALSE(result.has_value());
296+
}
297+
}
298+
249299
TEST_F(BucketSelectConverterTest, UnsupportedFieldTypeReturnsError) {
250300
auto predicate =
251301
PredicateBuilder::Equal(0, "items", FieldType::ARRAY, Literal(static_cast<int32_t>(42)));

src/paimon/core/operation/append_only_file_store_scan_test.cpp

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
#include <algorithm>
2222
#include <cstdint>
23+
#include <cstring>
2324
#include <map>
2425
#include <optional>
2526
#include <string>
@@ -61,7 +62,7 @@ class AppendBucketPruningTest : public testing::Test {
6162
const std::shared_ptr<Predicate>& predicate,
6263
const std::optional<int32_t>& bucket = std::nullopt) const {
6364
auto arrow_schema = DataField::ConvertDataFieldsToArrowSchema(
64-
{DataField(0, arrow::field("rowkey", arrow::utf8())),
65+
{DataField(0, arrow::field("rowkey", rowkey_type_)),
6566
DataField(1, arrow::field("value", arrow::int32()))});
6667
PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<TableSchema> schema,
6768
TableSchema::Create(schema_id_, arrow_schema, {}, {}, options_));
@@ -101,6 +102,33 @@ class AppendBucketPruningTest : public testing::Test {
101102
Literal(FieldType::STRING, "key", 3));
102103
}
103104

105+
template <typename T>
106+
void CheckMatchingValue(FieldType field_type, const T& query_value, const T& stored_value) {
107+
options_[Options::BUCKET] = "17";
108+
auto predicate = PredicateBuilder::Equal(0, "rowkey", field_type, Literal(query_value));
109+
ASSERT_OK_AND_ASSIGN(auto comparison,
110+
Literal(query_value).CompareTo(Literal(stored_value)));
111+
ASSERT_EQ(comparison, 0);
112+
BinaryRow stored_key = BinaryRowGenerator::GenerateRow({stored_value}, pool_.get());
113+
BinaryRow query_key = BinaryRowGenerator::GenerateRow({query_value}, pool_.get());
114+
int32_t bucket = DefaultBucketFunction().Bucket(stored_key, 17);
115+
ASSERT_NE(bucket, DefaultBucketFunction().Bucket(query_key, 17));
116+
SimpleStats stats = BinaryRowGenerator::GenerateStats(
117+
{stored_value, 0}, {stored_value, 100}, {0, 0}, pool_.get());
118+
ASSERT_OK_AND_ASSIGN(
119+
auto file,
120+
DataFileMeta::ForAppend("data.parquet", 100, 10, stats, 0, 9, 0, std::nullopt,
121+
std::nullopt, std::nullopt, std::nullopt, std::nullopt));
122+
ManifestEntry entry(FileKind::Add(), BinaryRow::EmptyRow(), bucket, 17, file);
123+
ASSERT_OK_AND_ASSIGN(auto stats_scan, CreateScan(predicate, bucket));
124+
ASSERT_OK_AND_ASSIGN(bool stats_match, stats_scan->FilterByStats(entry));
125+
ASSERT_TRUE(stats_match);
126+
ASSERT_OK_AND_ASSIGN(auto scan, CreateScan(predicate));
127+
ASSERT_OK_AND_ASSIGN(bool keep, scan->FilterManifestEntry(entry));
128+
ASSERT_TRUE(keep);
129+
}
130+
131+
std::shared_ptr<arrow::DataType> rowkey_type_ = arrow::utf8();
104132
static constexpr int32_t kNumBuckets = 4;
105133
int64_t schema_id_ = 0;
106134
std::shared_ptr<SchemaManager> schema_manager_;
@@ -149,6 +177,23 @@ TEST_F(AppendBucketPruningTest, DoesNotPruneDifferentSchema) {
149177
CheckBuckets(KeyEquals(), std::nullopt);
150178
}
151179

180+
TEST_F(AppendBucketPruningTest, PreservesCrossScaleDecimalMatch) {
181+
rowkey_type_ = arrow::decimal128(10, 2);
182+
CheckMatchingValue(FieldType::DECIMAL, Decimal::FromUnscaledLong(12, 10, 1),
183+
Decimal::FromUnscaledLong(120, 10, 2));
184+
}
185+
186+
TEST_F(AppendBucketPruningTest, PreservesDifferentNaNPayloadMatch) {
187+
rowkey_type_ = arrow::float64();
188+
uint64_t query_bits = 0x7ff8000000000000ULL;
189+
uint64_t stored_bits = 0x7ff8000000000001ULL;
190+
double query_value;
191+
double stored_value;
192+
std::memcpy(&query_value, &query_bits, sizeof(query_value));
193+
std::memcpy(&stored_value, &stored_bits, sizeof(stored_value));
194+
CheckMatchingValue(FieldType::DOUBLE, query_value, stored_value);
195+
}
196+
152197
TEST(AppendOnlyFileStoreScanTest, TestReconstructPredicateWithNonCastedFields) {
153198
std::string table_root =
154199
paimon::test::GetDataDir() +

0 commit comments

Comments
 (0)