Skip to content

Resolve page pruning StatisticsConverter once per file instead of once per row group - #24182

Draft
alamb wants to merge 2 commits into
apache:mainfrom
alamb:cache_parquet_column
Draft

Resolve page pruning StatisticsConverter once per file instead of once per row group#24182
alamb wants to merge 2 commits into
apache:mainfrom
alamb:cache_parquet_column

Conversation

@alamb

@alamb alamb commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Note: this PR is stacked on #24150 -- please review only the final commit until that merges.

Which issue does this PR close?

Rationale for this change

During page index pruning, prune_plan_with_page_index_and_metrics creates a StatisticsConverter for each predicate inside the per-row-group loop. Each construction calls parquet_column, which does a linear scan of both the Arrow schema fields and the Parquet leaf columns to resolve the predicate column, so the same column resolution is recomputed once per (predicate, row group).

For files with many row groups this adds up: the single-file ClickBench hits.parquet has 226 row groups and 105 columns, so a query with 2 pushed-down predicate columns resolved the same 2 columns 452 times per file open.

What changes are included in this PR?

  • Hoist the (predicate, StatisticsConverter) construction out of the row-group loop so each predicate column is resolved once per file, and pass the converter down by reference (prune_pages_in_one_row_group and PagesPruningStatistics now borrow it).
  • Minor side effect: predicates skipped because they are multi-column or fail converter creation are now logged once per file rather than once per row group.

Are these changes tested?

Covered by existing tests: datafusion-datasource-parquet lib tests and the page_pruning integration tests in datafusion/core/tests/parquet/ all pass. No behavior change intended.

Are there any user-facing changes?

No.

alamb and others added 2 commits August 8, 2026 08:33
The footer metadata records the column/offset index offsets even when
the page index itself has not been loaded. If no offsets are present the
file has no page index and attempting to load it can never succeed, so
avoid the deep clone of ParquetMetaData in that case.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…roup

Hoist the per-predicate StatisticsConverter construction (which resolves
the parquet leaf column via a linear scan of both schemas in
parquet_column) out of the row-group loop in
prune_plan_with_page_index_and_metrics, so the resolution happens once
per predicate per file instead of once per (predicate, row group).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the datasource Changes to the datasource crate label Aug 8, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.96855% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.06%. Comparing base (7d31a09) to head (7f46ba2).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
datafusion/datasource-parquet/src/page_filter.rs 77.41% 7 Missing ⚠️
datafusion/datasource-parquet/src/opener/mod.rs 99.21% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #24182    +/-   ##
========================================
  Coverage   81.06%   81.06%            
========================================
  Files        1106     1106            
  Lines      382252   382356   +104     
  Branches   382252   382356   +104     
========================================
+ Hits       309856   309960   +104     
+ Misses      54090    54088     -2     
- Partials    18306    18308     +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

datasource Changes to the datasource crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants