DRAFT - 21566 - #24161
Draft
kosiew wants to merge 18 commits into
Draft
Conversation
Add a scan-local ParquetPruningSetupCache in opener.rs to reuse adapted projection, adapted predicate, and row-group pruning predicate setup for same-schema files. Implement PhysicalExprAdapterFactory::supports_reusable_rewrites() in schema_rewriter.rs to allow default adapters while defaulting custom ones to opt-out. Connect the cache to ParquetMorselizer in source.rs and introduce a regression test to ensure the setup is reused correctly across same-schema files while keeping it conservative to avoid per-file literal replacements.
Simplify the pruning setup cache in opener.rs by removing the wrapper entry struct, deriving PartialEq/Eq, and returning cloned setup values instead of an extra Arc. Extract the cache-or-build branch into build_or_get_pruning_setup and avoid repeated literal_columns.is_empty() checks. Move returned pruning setup fields directly into prepared and simplify the test-only counting adapter and cache regression test loop. Tighten the supports_reusable_rewrites doc comment in schema_rewriter.rs without changing the public interface.
Refactor cache in opener.rs to utilize a HashMap, computing cold misses outside the mutex for better performance, with a re-check on insert. Add cache-boundary tests for non-reusable adapters and diverse physical schemas. Clarify documentation in schema_rewriter.rs for supports_reusable_rewrites() to specify the same logical/physical schema rewrite inputs.
Enhance opener.rs with cache-key rationale comments for clarity. Update ParquetPruningSetupCache to handle concurrent cold misses by implementing a per-entry pending/ready state. Clarify the public contract of supports_reusable_rewrites() in schema_rewriter.rs.
- Added missing local imports for TimeUnit and ParquetAccessPlan. - Removed stale DataFusionError import. - Updated fully qualified RecordBatch types to avoid lint issues. - Rewired three cache tests to use the local ParquetMorselizerBuilder helper instead of an outdated API. - Ensured all tests passed by running `cargo test -p datafusion-datasource-parquet --lib -- opener::test`, with all 23 opener tests successful.
- Disabled pruning setup cache when predicate contains dynamic filters. - Disabled cache when projection contains input_file_name(). Added tests for: - Dynamic filter stale snapshot regression. - input_file_name() projection not populating reusable cache.
- Added cache entries type alias for improved readability. - Deduplicated lock-poison error construction for consistency. - Extracted cache eligibility helper to streamline code. - Implemented shared test helper for input_file_name_expr(). - Added a two-file cache test helper and removed repeated loops for efficiency.
- Removed `ParquetPruningSetupCacheEntry`, `Pending`, `Ready`, `Failed` states, and associated per-entry synchronization logic (Mutex, Condvar). - Introduced a new cache structure using `Mutex<HashMap<ParquetPruningSetupCacheKey, ParquetPruningSetup>>`. - Updated cache retrieval logic: - On hit: return a cloned entry. - On miss: build a new entry, insert it, and return. - On error: no insertion is performed.
- Add new cache module (`opener/pruning_cache.rs`) using `parking_lot::Mutex` - Add cache eligibility documentation and literal map input handling - Convert `PreparedParquetOpen` cache to `Option` and expose setup logic via its methods - Update adapter reuse documentation and add a default‑factory safety comment - Introduce a shared cache test fixture
…teral cache‑bypass - Test with two same‑schema Parquet files having distinct partition values. - Validates reading rows (3, 0) and confirms the cache remains empty.
…nt unbounded growth - Replace unbounded cache with LRU bounded to 64 entries - Evict LRU schema setup when cap reached - Add unit test for eviction/rebuild
…locking in mod.rs
github-actions
Bot
added
documentation
Contributor
Author
|
run benchmark clickbench_pushdown |
github-actions
Bot
added
core
github-actions
Bot
added
execution
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing test-21566 (61b7538) to 70c26a0 (merge-base) diff Run configurationrun benchmark clickbench_pushdownResults will be posted here when complete File an issue against this benchmark runner |
Contributor
Author
|
run benchmark clickbench_1 clickbench_partitioned |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing test-21566 (61b7538) to 70c26a0 (merge-base) diff Run configurationrun benchmark clickbench_1Results will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing test-21566 (61b7538) to 70c26a0 (merge-base) diff Run configurationrun benchmark clickbench_partitionedResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing test-21566 (61b7538) to 70c26a0 (merge-base) diff Run configurationrun benchmark clickbench_pushdownCPU Details (lscpu)Details
Resource Usageclickbench_pushdown — base (merge-base)
clickbench_pushdown — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing test-21566 (61b7538) to 70c26a0 (merge-base) diff Run configurationrun benchmark clickbench_partitionedCPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing test-21566 (61b7538) to 70c26a0 (merge-base) diff Run configurationrun benchmark clickbench_1CPU Details (lscpu)Details
Resource Usageclickbench_1 — base (merge-base)
clickbench_1 — branch
File an issue against this benchmark runner |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Investigate benchmark perf of #21566