wangyong9999 commented on code in PR #284:
URL: https://github.com/apache/paimon-cpp/pull/284#discussion_r3939676595
##########
src/paimon/core/operation/append_only_file_store_scan.cpp:
##########
@@ -86,6 +104,12 @@ Result<bool> AppendOnlyFileStoreScan::FilterByStats(const
ManifestEntry& entry)
if (!predicates_) {
return true;
}
+ // A historical file may use a different schema or bucket count after a
rescale.
+ // Keep the inferred bucket separate from the caller's explicit bucket
filter.
+ if (predicate_bucket_ && entry.TotalBuckets() == core_options_.GetBucket()
&&
Review Comment:
Keeping these fallbacks in this PR. They are conservative but do not lose
rows. Pruning across layouts would need to use each file's bucket count and
compatible bucket-key schema. That is a separate optimization; the description
now explicitly says that even an unrelated schema change disables inference for
older files.
##########
src/paimon/core/operation/append_only_file_store_scan.cpp:
##########
@@ -66,6 +69,21 @@ Result<std::unique_ptr<AppendOnlyFileStoreScan>>
AppendOnlyFileStoreScan::Create
table_schema, arrow_schema, core_options,
executor, pool));
PAIMON_RETURN_NOT_OK(
scan->SplitAndSetFilter(table_schema->PartitionKeys(), arrow_schema,
scan_filters));
+ const auto& bucket_keys = table_schema->BucketKeys();
Review Comment:
Updated the description and scan docs. This prunes data files at the
manifest-entry level; it still opens manifests and does not activate
min/max-bucket manifest skipping or the bucket-specific live-entry cache. I
have not run a many-manifest planning benchmark and am not claiming a
planning-time speedup.
##########
src/paimon/core/operation/append_only_file_store_scan.cpp:
##########
@@ -66,6 +69,21 @@ Result<std::unique_ptr<AppendOnlyFileStoreScan>>
AppendOnlyFileStoreScan::Create
table_schema, arrow_schema, core_options,
executor, pool));
PAIMON_RETURN_NOT_OK(
scan->SplitAndSetFilter(table_schema->PartitionKeys(), arrow_schema,
scan_filters));
+ const auto& bucket_keys = table_schema->BucketKeys();
+ int32_t num_buckets = core_options.GetBucket();
+ if (scan->predicates_ && !scan_filters->GetBucketFilter().has_value() &&
num_buckets > 0 &&
+ !bucket_keys.empty()) {
+ std::vector<std::shared_ptr<arrow::DataType>> bucket_key_types;
+ bucket_key_types.reserve(bucket_keys.size());
+ for (const auto& key : bucket_keys) {
+ PAIMON_ASSIGN_OR_RAISE(DataField field,
table_schema->GetField(key));
+ bucket_key_types.push_back(field.Type());
+ }
+ PAIMON_ASSIGN_OR_RAISE(scan->predicate_bucket_,
+ BucketSelectConverter::Convert(
+ scan->predicates_, bucket_keys,
bucket_key_types,
+ core_options.GetBucketFunctionType(),
num_buckets, pool.get()));
Review Comment:
Fixed in 5ebbaf33. The shared converter now uses
DecimalToDecimalCastExecutor and checks that the converted value equals the
original before hashing. Rounding and overflow fall back to no inferred bucket.
Added DEFAULT/HIVE tests for both compact and noncompact decimals, plus the
append regression for 1.2 against stored 1.20. The regression failed before the
fix and passes now.
##########
src/paimon/core/operation/append_only_file_store_scan.cpp:
##########
@@ -86,6 +104,12 @@ Result<bool> AppendOnlyFileStoreScan::FilterByStats(const
ManifestEntry& entry)
if (!predicates_) {
return true;
}
+ // A historical file may use a different schema or bucket count after a
rescale.
+ // Keep the inferred bucket separate from the caller's explicit bucket
filter.
+ if (predicate_bucket_ && entry.TotalBuckets() == core_options_.GetBucket()
&&
+ entry.File()->schema_id == table_schema_->Id() && entry.Bucket() !=
*predicate_bucket_) {
+ return false;
Review Comment:
Fixed in 5ebbaf33. FLOAT and DOUBLE NaN bucket-key literals now disable
inference in the shared converter. Added a regression with the two DOUBLE
payloads above: their hashes differ, their stats match, and the file is
retained. The test failed before the fix and passes now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
