haohuaijin commented on code in PR #25460:
URL: https://github.com/apache/datafusion/pull/25460#discussion_r4052785359
##########
datafusion/datasource-parquet/src/opener/mod.rs:
##########
@@ -1291,35 +1294,86 @@ 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() {
+ let prepared = &self.prepared.loaded.prepared;
+ // Resolve the same file-column projection as the decoder,
excluding
+ // virtual columns and respecting nested field projections.
+ let projection = match prepared.virtual_state.as_deref() {
+ None => prepared.projection.clone(),
+ Some(state) =>
prepared.projection.clone().try_map_exprs(|expr| {
+ replace_columns_with_literals(expr,
state.null_replacements())
+ })?,
+ };
+ let read_plan = build_projection_read_plan(
+ projection.expr_iter(),
+ &prepared.physical_file_schema,
+ parquet_metadata.file_metadata().schema_descr(),
+ );
+ if selected_row_groups.any(|idx| {
Review Comment:
> parse_offset_index sets both indexes to None if any column chunk in any
row group has no offset index (parquet 59.3 file/metadata/parser.rs:309-321).
On a mixed file this returns true, pays the fetch,
we already upgrade to parquet 60, this limit is removed, in
https://github.com/apache/arrow-rs/pull/10719
--
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]
