POC: page-granular scan planning for the Parquet push decoder - #10555
Draft
adriangb wants to merge 2 commits into
Draft
POC: page-granular scan planning for the Parquet push decoder#10555adriangb wants to merge 2 commits into
adriangb wants to merge 2 commits into
Conversation
The push decoder reports what it needs one row group at a time: NeedsData does not resolve until every projected byte of the row group is buffered. Callers that schedule their own I/O therefore must buffer a whole row group before decoding starts, and have no way to ask what the next batch needs. Add `plan_scan_ranges`, which decomposes a scan (row groups x projection x optional row selection) into the individual pages it will read, in the order decoding needs them, each tagged with the span of selected rows it serves. Callers can then fetch only what the next batch requires, drop pages once the decode cursor passes them, and read ahead as far as their own byte budget allows. The plan is demand, not schedule: request sizing, readahead depth and range coalescing stay caller policy, since they depend on the storage medium rather than the file. Row tags rather than a fixed row or byte quantum keep it usable by callers budgeting in either unit, which matters for schemas with large values where a fixed row count implies an unbounded byte count. Returns None when the file has no offset index, so callers fall back to row-group-granular fetching. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two CI failures on the new module: * Rustdoc `-D warnings` rejected the doc example: it lived in the module-level docs of a private module, tripping `rustdoc::private_doc_tests`. The prose was invisible to users for the same reason, since only the re-exported items are public. Move the motivation, units discussion and example onto `plan_scan_ranges`, which is where a reader looking up the API will actually find them, and leave the module header as a pointer. * Clippy `-D warnings` rejected `ArrowReaderOptions::with_page_index`, deprecated since 57.2.0. Use `with_page_index_policy` in the test and in the doc link that points callers at it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
Which issue does this PR close?
Rationale for this change
ParquetPushDecoderreports what it needs one row group at a time:DecodeResult::NeedsDatadoes not resolve until every projected byte of the row group is buffered. That is the right granularity when the caller wants a whole row group's reader, but for a caller scheduling its own I/O it means:This adds
plan_scan_ranges, which decomposes a scan into the individual pages it will read, in the order decoding needs them, each tagged with the span of selected rows it serves. A caller can then fetch only what the next batch needs, drop pages once its decode cursor passes them, and read ahead as far as its own byte budget allows.The plan is demand, not schedule: it says which bytes the query will read and when they are first needed. Request sizing, readahead depth and whether to merge nearby ranges stay caller policy, because they depend on the storage medium rather than on the file. (
ObjectStore::get_rangesalready coalesces for stores that use the default implementation, which is one reason this deliberately does not.)Units: why row tags rather than a fixed quantum
The obvious shape would be
next_batch_ranges()orranges_for(n_rows). Both bake in a unit the caller may not want. A fixed row count can imply an unbounded byte count — 8192 rows of 1MB values is 8GB — while a pure byte budget cannot express "enough to make progress".So each planned range carries
first_row/last_rowin selected-row space instead. A caller budgeting in bytes takes pages until the budget fills; a caller thinking in rows takes pages until the tags cover its window; andlast_rowis what makes eviction possible at all.One floor remains and is documented:
ParquetRecordBatchReaderdecodesbatch_sizerows at a time, so no fetch plan can subdivide a batch. For schemas with very large values the lever is a smallerbatch_size, or byte-based batch sizing, which is a separate gap.What changes are included in this PR?
A single new module,
parquet::arrow::push_decoder::scan_plan, exportingplan_scan_ranges,ScanPlanandPlannedRange. No existing code path is touched — nothing calls it inside the crate yet, so this is purely additive.plan_scan_ranges(metadata, row_groups, projection, selection)returnsNonewhen the file has no offset index, since page locations are what make page-granular planning possible; callers fall back to row-group-granular fetching.The natural follow-up, if this shape is acceptable, is to use it inside the decoder itself so
NeedsDatacan resolve at batch granularity rather than row-group granularity, and to makeRowFilterevaluation incremental (today predicates are evaluated eagerly over a whole row group insidebuild(), which is why the DataFusion side below falls back to row-group granularity whenever filter pushdown is on).Are these changes tested?
Yes — six unit tests covering page ordering, projection, selection-driven page skipping (including selected-row-space tagging), row-group subsets, the no-offset-index fallback, and that planned bytes exactly cover the projected column chunks when nothing is skipped.
It is also exercised end to end by a DataFusion POC that schedules I/O against this plan: apache/datafusion#24086. Measured there against DataFusion
main, with simulated object-store latency:ClickBench is unchanged there — because the published
hits_*.parquetfiles contain no page index at all (0 of 105 columns), so page-granular planning cannot run on them. Rewriting three of those files withwrite_page_index=Trueand changing nothing else makes the same queries 1.54x faster (36 queries: 35 faster, 0 slower, 1 neutral), which is a reasonable argument that the capability is worth having even where files do not use it yet.Are there any user-facing changes?
New public API only (
plan_scan_ranges,ScanPlan,PlannedRange); no behavior change to existing paths. Marked draft because the API shape is the thing worth discussing — in particular whether this belongs as a free function or as a method on the decoder that can also advance it.🤖 Generated with Claude Code