alamb opened a new pull request, #11609: URL: https://github.com/apache/arrow-rs/pull/11609
# Which issue does this PR close? - Part of #11428 - Follow up to #10555 (see [this review thread](https://github.com/apache/arrow-rs/pull/10555#discussion_r4160451573)) # Rationale for this change `ParquetPushDecoder::scan_plan` is meant to report exactly what the decoder will read. Today the decoder (`InMemoryRowGroup::fetch_ranges`) and the planner (`Planner::plan_next_row_group` + `RowGroupContext::first_range`) each have their own copy of the per-stage loop that decides which column chunks a stage fetches: expanding the selection to batch boundaries for cached predicate columns, skipping columns that an earlier stage already read, and picking the selection for each column. Only the equality tests keep those two copies in sync. This PR puts that decision in one function so the two paths cannot drift. # What changes are included in this PR? - Add `column_fetches` in `in_memory_row_group.rs`: the one function that decides which column chunks a decoding stage fetches and with what selection, yielding one `ColumnFetch` per column. `DataRequestBuilder::build` (the decoder) flattens it into the data request; the planner builds its cursors from it. `InMemoryRowGroup::fetch_ranges` and `FetchRanges` are removed, and `columns_to_fetch` / `column_selection` become private helpers of `column_fetches`. - Add `StageSchedule::is_read_before`, so the planner derives "an earlier stage already read this column" from the schedule. The decoder keeps using its loaded column chunks, which is the same set. - The planner now starts each stage lazily from the schedule, instead of precomputing a column list per stage per row group. `StagePlan` and the per-row-group `vec![false; num_columns]` are gone. - Move `column_chunk_range` next to `ColumnFetch` and add `offset_index_for_row_group`, replacing three hand-rolled copies of the offset-index lookup. What remains separate on purpose: - The planner still applies the offset/limit budget before the projection stage only when there are no predicates. The decoder applies it after the predicates run, which the planner cannot simulate without data. - The chunk-to-page split and the heap merge stay in the planner. The decoder requests whole column chunks without a selection, so making it request the plan's ranges would change `NeedsData` to page granularity. That is a separate decision (and where the batch-granularity work is heading anyway). # Are these changes tested? Yes, by the existing tests. The scan plan tests check that the plan's bytes equal the decoder's requests without a filter, that the plan covers every later request at every step with a filter, and the lazy-plan test still checks one heap entry per projected column. The push decoder equivalence and fuzz tests cover the decoder side. Not benchmarked locally. The planner now calls `is_read_before` once per column per stage (a few mask lookups) and no longer allocates a bool per column per row group, so I expect no change, but `push_decoder/scan_plan` on a wide file would confirm. # Are there any significant user-facing changes? No. `RowGroupPageIndex` now derives `Clone`, which is an additive API change. -- 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]
