alamb opened a new pull request, #24126:
URL: https://github.com/apache/datafusion/pull/24126

   ## Which issue does this PR close?
   
   - Part of #21554
   - Follow on / POC built on top of #21566 (this PR's diff **includes** the 
commits from #21566 -- only the last commit is new)
   
   ## Rationale for this change
   
   POC exploring two extensions to the pruning setup cache from #21566:
   
   1. **Scan-scoped (not partition-scoped) caching**: in #21566 the cache is 
created in `create_morselizer`, which runs once per partition, so N partitions 
each redo the setup once per unique schema. This PR moves the cache onto 
`ParquetSource` so the per-partition source clones (made via `with_batch_size` 
in `FileScanConfig::open_with_args`) all share one cache, and each unique 
physical schema pays the rewrite/simplify/pruning-predicate cost once per scan. 
As a bonus, repeated executions of the same plan also reuse the cache.
   
   2. **Cache page pruning predicates too**: #21566 caches the adapted 
projection/predicate and the row-group `PruningPredicate`, but rebuilds the 
`PagePruningAccessPlanFilter` per file. Page predicate construction is CPU-only 
and depends only on the adapted predicate and physical file schema, so it can 
be cached under the same key (with `enable_page_index` added to the key).
   
   Because the cache now outlives a single morselizer, the cache key no longer 
stores raw expression addresses (`usize` from `Arc::as_ptr`). Plan-variant 
clones created by the `with_*` builder methods share the cache while carrying 
different predicates, and a dropped expression's address could be reused by a 
new allocation, producing a false cache hit (ABA). The key now holds the 
projection/predicate `Arc`s themselves and compares them by pointer identity: 
an entry keeps its expressions alive, so an address can never be reused while 
the cache references it.
   
   ## What changes are included in this PR?
   
   * Move `ParquetPruningSetupCache` ownership from `ParquetMorselizer` 
creation to a field on `ParquetSource`; `create_morselizer` now hands out `Arc` 
clones of the shared cache.
   * Harden `ParquetPruningSetupCacheKey`: hold `Arc<dyn PhysicalExpr>`s with 
manual pointer-identity `PartialEq`/`Hash` instead of raw addresses.
   * Add `page_pruning_predicate` to the cached `ParquetPruningSetup`, built in 
`build_pruning_setup` when `enable_page_index` is set; add `enable_page_index` 
to the cache key.
   
   ## Are these changes tested?
   
   Yes:
   * New test `test_pruning_setup_cache_shared_across_partition_clones` 
verifies per-partition source clones share the scan-wide cache.
   * New test `test_pruning_setup_cache_includes_page_pruning_predicate` 
verifies the page pruning predicate is cached and the adapter is still created 
only once for same-schema files.
   * All existing tests from #21566 and the rest of 
`datafusion-datasource-parquet` pass unchanged.
   
   ## Are there any user-facing changes?
   
   No. Internal optimization only.
   
   ## LLM-generated code disclosure
   
   This PR includes LLM-generated code and comments. All LLM-generated content 
has been manually reviewed.


-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to