wangyong9999 commented on code in PR #284:
URL: https://github.com/apache/paimon-cpp/pull/284#discussion_r3939628213
##########
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:
Both guards give up more than they need to. A different `TotalBuckets()`
does not make the file unprunable: its bucket is just `function.Bucket(key_row,
entry.TotalBuckets())`. `Convert()` already builds the key row and the bucket
function, so keep those two (or memoize per total-bucket count) and recompute
for that count instead of retaining every file of a rescaled partition.
Likewise the `schema_id` check disables pruning for every file older than the
last schema change, and adding a nullable column is enough to trigger that,
while the hash only depends on the bucket-key columns. `FilterByStats` already
loads the file's data schema a few lines below; compare the bucket-key field
types there and skip only when they differ.
##########
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:
Worth spelling out in the description: since the inferred bucket never
reaches `bucket_filter_`, planning still opens every manifest. Both the
min/max-bucket manifest skip in `FilterManifest` and the snapshot
live-manifest-entry cache are gated on `bucket_filter_`, and for the
point-lookup case that motivates this change they are most of the planning
cost. The entry-level check is the safe choice for mixed layouts, but a number
for planning time with inferred vs explicit bucket on a many-manifest table
would show whether this delivers the win by itself.
--
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]
