zhuqi-lucas opened a new pull request, #24509: URL: https://github.com/apache/datafusion/pull/24509
## Which issue does this PR close? - Closes #24287. ## Rationale for this change Split out from #23696 (per @adriangb's [decomposition proposal](https://github.com/apache/datafusion/pull/23696#issuecomment-5346992228)) as an independent, standalone fix. A row group whose `RowSelection` selects zero rows after page-index pruning is silently advanced past by arrow-rs's push decoder inside `try_next_reader`, **without handing back a reader**. The rest of DataFusion — per-RG metadata maps and the runtime dynamic pruner — assumes a 1:1 correspondence between the prepared plan and the readers the decoder hands back, so a silently-skipped empty row group leaves that bookkeeping off by one. This is the same invariant behind the #24352 / #24355 family, both of which shipped as standalone fixes. Stripping the empty entries up front restores the invariant at the source, independently of any feature that consumes per-RG state. ## What changes are included in this PR? - `access_plan.rs`: `PreparedAccessPlan::prepare` now calls a new generic `strip_empty_row_groups(row_group_indexes, row_selection, row_group_meta_data) -> (Vec<usize>, Option<RowSelection>)`, which splits the flat `RowSelection` per row group (mirroring arrow-rs's own `split_off` logic) and drops any row group whose segment selects nothing. When there is no row selection nothing can be empty and the inputs pass through unchanged. - `sort.rs`: `test_prepared_access_plan_reverse_empty_selection` — an all-skipped plan now strips to an *empty* plan, so the assertion is `row_group_indexes.is_empty()` / `row_selection.is_none()` rather than "the selection selects 0 rows". No `fully_matched` coupling, no public API change, no new metric, no slt churn. ## Are these changes tested? Yes — unit tests in `access_plan.rs`: - `test_strip_empty_row_groups_drops_only_empties` — a 4-RG plan where RG 1 and RG 3 select nothing; asserts they are dropped and the surviving indexes + re-concatenated selection stay aligned. - `test_strip_empty_row_groups_none_selection_unchanged` — `None` selection passes through. - Plus the updated `sort.rs` reverse test. All existing `access_plan` / `sort` / parquet slt tests pass. ## Are there any user-facing changes? No. -- 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]
