alamb commented on code in PR #25460:
URL: https://github.com/apache/datafusion/pull/25460#discussion_r4076369484
##########
datafusion/datasource-parquet/src/opener/mod.rs:
##########
@@ -1291,35 +1294,92 @@ impl RowGroupsPrunedParquetOpen {
/// Returns true if the reader would benefit from a page index load, given
/// the current pruning predicate and row group access plan.
///
- /// The page index is used for data page pruning, and it is only useful
- /// when:
+ /// Offset indexes also allow an existing row selection to skip data pages,
+ /// even without a predicate or when row-group statistics fully match it.
+ /// Otherwise, the page index is useful for predicate-based pruning when:
///
/// 1. There is at least one row group that may have filtered rows
/// (if it is fully matched we know no rows will be filtered)
///
/// 2. There is a page index for at least one predicate column (some
/// parquet writers do not write the page index).
- fn should_load_page_index(&self) -> bool {
+ fn should_load_page_index(&self) -> Result<bool> {
+ if !self.prepared.loaded.prepared.enable_page_index {
+ return Ok(false);
+ }
+ let row_groups = &self.row_groups;
+ let parquet_metadata = self.prepared.loaded.reader_metadata.metadata();
+ // External row selections need offset indexes to skip pages without
+ // decoding them. They do not require column statistics or a predicate.
+ let mut selected_row_groups = row_groups
+ .row_group_indexes()
+ .filter(|&idx| {
+ let RowGroupAccess::Selection(selection) =
+ &row_groups.access_plan().inner()[idx]
+ else {
+ return false;
+ };
+ // Runs alternate between selected and skipped rows, so two
runs
+ // suffice. Stream bitmap runs without materializing all
selectors.
+ match selection.as_mask() {
+ Some(mask) => MaskRunIter::new(mask).nth(1).is_some(),
+ None => selection.iter().nth(1).is_some(),
+ }
+ })
+ .peekable();
+ if selected_row_groups.peek().is_some() {
Review Comment:
this is doing an awful lot of work to come up with an entire read plan and
then seems to just throw it away -- is there some way we can reuse what we have
/ save the plan in the future?
--
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]
