[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #4387: Fix page index pruning fail on complex_expr

[GitBox <gi...@apache.org>]

alamb commented on code in PR #4387:
URL: https://github.com/apache/arrow-datafusion/pull/4387#discussion_r1032923875


##########
datafusion/core/src/physical_plan/file_format/parquet/page_filter.rs:
##########
@@ -126,46 +126,50 @@ pub(crate) fn build_page_filter(
         let mut row_selections = VecDeque::with_capacity(page_index_predicates.len());
         for predicate in page_index_predicates {
             // `extract_page_index_push_down_predicates` only return predicate with one col.
-            let col_id = *predicate.need_input_columns_ids().iter().next().unwrap();
-            let mut selectors = Vec::with_capacity(row_groups.len());
-            for r in row_groups.iter() {
-                let rg_offset_indexes = file_offset_indexes.get(*r);
-                let rg_page_indexes = file_page_indexes.get(*r);
-                if let (Some(rg_page_indexes), Some(rg_offset_indexes)) =
-                    (rg_page_indexes, rg_offset_indexes)
-                {
-                    selectors.extend(
-                        prune_pages_in_one_row_group(
-                            &groups[*r],
-                            &predicate,
-                            rg_offset_indexes.get(col_id),
-                            rg_page_indexes.get(col_id),
-                            groups[*r].column(col_id).column_descr(),
-                            file_metrics,
-                        )
-                        .map_err(|e| {
-                            ArrowError::ParquetError(format!(
-                                "Fail in prune_pages_in_one_row_group: {}",
-                                e
-                            ))
-                        }),
-                    );
-                } else {
-                    trace!(
+            //  when building `PruningPredicate`, some single column filter like `abs(i) = 1`
+            //  will be rewrite to `lit(true)`, so may have an empty required_columns.

Review Comment:
   Thank you for these comments, BTW -- they help both to review the code as well as read it 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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org