zhuqi-lucas commented on code in PR #23696:
URL: https://github.com/apache/datafusion/pull/23696#discussion_r3766671687
##########
datafusion/datasource-parquet/src/access_plan.rs:
##########
@@ -571,12 +571,81 @@ impl ParquetAccessPlan {
row_group_meta_data: &[RowGroupMetaData],
) -> Result<PreparedAccessPlan> {
let row_group_indexes = self.row_group_indexes();
+ // Carry `fully_matched` flags in the same order as
+ // `row_group_indexes` so downstream code (per-RG `RowFilter` skip)
+ // can look them up positionally.
+ let fully_matched: Vec<bool> = row_group_indexes
+ .iter()
+ .map(|&idx| self.fully_matched[idx])
+ .collect();
let row_selection =
self.into_overall_row_selection(row_group_meta_data)?;
- PreparedAccessPlan::new(row_group_indexes, row_selection)
+ let (row_group_indexes, fully_matched, row_selection) =
strip_empty_row_groups(
+ row_group_indexes,
+ fully_matched,
+ row_selection,
+ row_group_meta_data,
+ );
+
+ PreparedAccessPlan::new(row_group_indexes, fully_matched,
row_selection)
}
}
+/// Strip row groups whose post-pruning `RowSelection` selects zero rows.
+///
+/// arrow-rs's push decoder silently advances past such row groups inside
+/// `try_next_reader`, but the rest of DataFusion (per-RG metadata maps,
+/// the runtime dynamic-pruner, the per-RG `RowFilter` toggle) assumes a
+/// 1:1 correspondence between the prepared plan and the readers the
+/// decoder hands back. Removing these empty entries here keeps that
+/// invariant and lets downstream code consult per-RG state — like
+/// [`PreparedAccessPlan::fully_matched`] — without going out of sync.
+///
+/// The flat `RowSelection` is split per row group with
+/// [`RowSelection::split_off`] (mirroring arrow-rs's own logic) and the
+/// surviving segments are concatenated back into the result selection.
+/// When `row_selection` is `None` (no page-index pruning, no
+/// user-supplied selection) no row group can be empty and the inputs are
+/// returned unchanged.
Review Comment:
You are right that the empty-RG stripping itself is generic, so I am happy
to pull that generic part out into its own follow-up PR once this lands.
--
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]
