wangyong9999 commented on code in PR #284:
URL: https://github.com/apache/paimon-cpp/pull/284#discussion_r3940540600
##########
src/paimon/core/operation/file_store_scan.cpp:
##########
@@ -557,6 +557,13 @@ Result<bool> FileStoreScan::FilterManifestEntry(const
ManifestEntry& entry) cons
if (bucket_filter_ != std::nullopt && entry.Bucket() !=
bucket_filter_.value()) {
return false;
}
+ // Hash with the file's bucket count, never the current table's count. An
older schema
+ // may encode bucket keys differently, so leave those files to the
existing stats filter.
+ if (bucket_selector_ && entry.TotalBuckets() > 0 &&
+ entry.File()->schema_id == table_schema_->Id() &&
Review Comment:
Fixed in 7c9545a4. Bucket compatibility now compares the ordered field IDs,
Arrow types and bucket function, and caches the result per schema ID. An
unrelated column change no longer disables pruning; incompatible bucket-key
layouts still fall back. Added Append/KV regressions for compatible historical
schemas and an incompatible STRING/BINARY case. Historical schema reads now use
the existing concurrent map, since manifest filtering can load them in parallel.
##########
src/paimon/core/operation/file_store_scan.cpp:
##########
@@ -602,6 +609,19 @@ Status FileStoreScan::SplitAndSetFilter(const
std::vector<std::string>& partitio
}
}
bucket_filter_ = scan_filters->GetBucketFilter();
+ const auto& bucket_keys = table_schema_->BucketKeys();
+ if (predicates_ && !bucket_filter_ && core_options_.GetBucket() > 0 &&
!bucket_keys.empty()) {
Review Comment:
Fixed in 7c9545a4. Inferred scans can use the live-entry cache again. The
key includes the current schema ID as well as bucket and bucket count: files
needing a schema fallback must not disappear when another query fills the
cache. Candidates retain matching buckets, other bucket counts and other schema
IDs; the selector and predicates run after lookup. Cold inferred reads keep all
manifests, while explicit filtering keeps its existing path.
The tests verify cache hits across fresh scans, explicit/inferred isolation,
and two keys that share the current bucket but need different historical
buckets. Disabling inferred cache eligibility makes all 8 cache-enabled
integration cases fail. With the fix, 56 core tests, 15 cache tests and 81
integration tests pass. Build and pre-commit checks pass; upstream CI still
needs maintainer approval.
--
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]
