alamb commented on code in PR #25335:
URL: https://github.com/apache/datafusion/pull/25335#discussion_r4030062366


##########
datafusion/datasource-parquet/src/page_filter.rs:
##########
@@ -708,14 +710,13 @@ fn prune_pages_in_one_row_group(
     Some((RowSelection::from(vec), values))
 }
 
-/// Implement [`PruningStatistics`] for one column's PageIndex (column_index + 
offset_index)
+/// Implement [`PruningStatistics`] for one column's [`PageIndexProvider`]
 #[derive(Debug)]
 struct PagesPruningStatistics<'a> {
     row_group_index: usize,
     row_group_metadatas: &'a [RowGroupMetaData],
     converter: StatisticsConverter<'a>,
-    column_index: &'a ParquetColumnIndex,
-    offset_index: &'a ParquetOffsetIndex,
+    page_index: &'a dyn PageIndexProvider,

Review Comment:
   This is the nice new API that @etseidl added upstream in 
https://github.com/apache/arrow-rs/pull/10842 to encapsulate and lay the 
foundation for supplying the page indexes via API



##########
datafusion/sqllogictest/test_files/push_down_filter_parquet.slt:
##########
@@ -319,7 +319,7 @@ EXPLAIN ANALYZE SELECT * FROM topk_multi_col ORDER BY b ASC 
NULLS LAST, a DESC L
 ----
 Plan with Metrics
 01)SortExec: TopK(fetch=2), expr=[b@1 ASC NULLS LAST, a@0 DESC], 
preserve_partitioning=[false], filter=[b@1 < bb OR b@1 = bb AND (a@0 IS NULL OR 
a@0 > ac)], metrics=[output_rows=2, output_batches=1, row_replacements=2]
-02)--DataSourceExec: file_groups={1 group: 
[[WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/push_down_filter_parquet/topk_multi_col.parquet]]},
 projection=[a, b, c], file_type=parquet, predicate=DynamicFilter [ b@1 < bb OR 
b@1 = bb AND (a@0 IS NULL OR a@0 > ac) ], sort_order_for_reorder=[b@1 ASC NULLS 
LAST, a@0 DESC], dynamic_rg_pruning=eligible, pruning_predicate=b_null_count@1 
!= row_count@2 AND b_min@0 < bb OR b_null_count@1 != row_count@2 AND b_min@0 <= 
bb AND bb <= b_max@3 AND (a_null_count@4 > 0 OR a_null_count@4 != row_count@2 
AND a_max@5 > ac), required_guarantees=[], metrics=[output_rows=4, 
output_batches=1, files_ranges_pruned_statistics=1 total → 1 matched, 
row_groups_pruned_statistics=1 total → 1 matched, 
row_groups_pruned_bloom_filter=1 total → 1 matched, page_index_pages_pruned=0 
total → 0 matched, page_index_rows_pruned=0 total → 0 matched, 
limit_pruned_row_groups=0 total → 0 matched, batches_split=0, 
file_open_errors=0, file_scan_errors=0, file
 s_opened=1, files_processed=1, num_predicate_creation_errors=0, 
predicate_evaluation_errors=0, pushdown_rows_matched=4, pushdown_rows_pruned=0, 
predicate_cache_inner_records=8, predicate_cache_records=8, 
scan_efficiency_ratio=21.62% (222/1.03 K)]
+02)--DataSourceExec: file_groups={1 group: 
[[WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/push_down_filter_parquet/topk_multi_col.parquet]]},
 projection=[a, b, c], file_type=parquet, predicate=DynamicFilter [ b@1 < bb OR 
b@1 = bb AND (a@0 IS NULL OR a@0 > ac) ], sort_order_for_reorder=[b@1 ASC NULLS 
LAST, a@0 DESC], dynamic_rg_pruning=eligible, pruning_predicate=b_null_count@1 
!= row_count@2 AND b_min@0 < bb OR b_null_count@1 != row_count@2 AND b_min@0 <= 
bb AND bb <= b_max@3 AND (a_null_count@4 > 0 OR a_null_count@4 != row_count@2 
AND a_max@5 > ac), required_guarantees=[], metrics=[output_rows=4, 
output_batches=1, files_ranges_pruned_statistics=1 total → 1 matched, 
row_groups_pruned_statistics=1 total → 1 matched, 
row_groups_pruned_bloom_filter=1 total → 1 matched, page_index_pages_pruned=0 
total → 0 matched, page_index_rows_pruned=0 total → 0 matched, 
limit_pruned_row_groups=0 total → 0 matched, batches_split=0, 
file_open_errors=0, file_scan_errors=0, file
 s_opened=1, files_processed=1, num_predicate_creation_errors=0, 
predicate_evaluation_errors=0, pushdown_rows_matched=4, pushdown_rows_pruned=0, 
predicate_cache_inner_records=8, predicate_cache_records=8, 
scan_efficiency_ratio=21.94% (222/1.01 K)]

Review Comment:
   In case it is not clear (I had to double check) the difference here is
   
   ```diff
   - scan_efficiency_ratio=21.62% (222/1.03 K)]
   + scan_efficiency_ratio=21.94% (222/1.01 K)]
   ```
   
   Specifically since the file size is slightly different, this metric is also 
reported slightly differently



##########
datafusion/sqllogictest/test_files/push_down_filter_parquet.slt:
##########
@@ -319,7 +319,7 @@ EXPLAIN ANALYZE SELECT * FROM topk_multi_col ORDER BY b ASC 
NULLS LAST, a DESC L
 ----
 Plan with Metrics
 01)SortExec: TopK(fetch=2), expr=[b@1 ASC NULLS LAST, a@0 DESC], 
preserve_partitioning=[false], filter=[b@1 < bb OR b@1 = bb AND (a@0 IS NULL OR 
a@0 > ac)], metrics=[output_rows=2, output_batches=1, row_replacements=2]
-02)--DataSourceExec: file_groups={1 group: 
[[WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/push_down_filter_parquet/topk_multi_col.parquet]]},
 projection=[a, b, c], file_type=parquet, predicate=DynamicFilter [ b@1 < bb OR 
b@1 = bb AND (a@0 IS NULL OR a@0 > ac) ], sort_order_for_reorder=[b@1 ASC NULLS 
LAST, a@0 DESC], dynamic_rg_pruning=eligible, pruning_predicate=b_null_count@1 
!= row_count@2 AND b_min@0 < bb OR b_null_count@1 != row_count@2 AND b_min@0 <= 
bb AND bb <= b_max@3 AND (a_null_count@4 > 0 OR a_null_count@4 != row_count@2 
AND a_max@5 > ac), required_guarantees=[], metrics=[output_rows=4, 
output_batches=1, files_ranges_pruned_statistics=1 total → 1 matched, 
row_groups_pruned_statistics=1 total → 1 matched, 
row_groups_pruned_bloom_filter=1 total → 1 matched, page_index_pages_pruned=0 
total → 0 matched, page_index_rows_pruned=0 total → 0 matched, 
limit_pruned_row_groups=0 total → 0 matched, batches_split=0, 
file_open_errors=0, file_scan_errors=0, file
 s_opened=1, files_processed=1, num_predicate_creation_errors=0, 
predicate_evaluation_errors=0, pushdown_rows_matched=4, pushdown_rows_pruned=0, 
predicate_cache_inner_records=8, predicate_cache_records=8, 
scan_efficiency_ratio=21.62% (222/1.03 K)]
+02)--DataSourceExec: file_groups={1 group: 
[[WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/push_down_filter_parquet/topk_multi_col.parquet]]},
 projection=[a, b, c], file_type=parquet, predicate=DynamicFilter [ b@1 < bb OR 
b@1 = bb AND (a@0 IS NULL OR a@0 > ac) ], sort_order_for_reorder=[b@1 ASC NULLS 
LAST, a@0 DESC], dynamic_rg_pruning=eligible, pruning_predicate=b_null_count@1 
!= row_count@2 AND b_min@0 < bb OR b_null_count@1 != row_count@2 AND b_min@0 <= 
bb AND bb <= b_max@3 AND (a_null_count@4 > 0 OR a_null_count@4 != row_count@2 
AND a_max@5 > ac), required_guarantees=[], metrics=[output_rows=4, 
output_batches=1, files_ranges_pruned_statistics=1 total → 1 matched, 
row_groups_pruned_statistics=1 total → 1 matched, 
row_groups_pruned_bloom_filter=1 total → 1 matched, page_index_pages_pruned=0 
total → 0 matched, page_index_rows_pruned=0 total → 0 matched, 
limit_pruned_row_groups=0 total → 0 matched, batches_split=0, 
file_open_errors=0, file_scan_errors=0, file
 s_opened=1, files_processed=1, num_predicate_creation_errors=0, 
predicate_evaluation_errors=0, pushdown_rows_matched=4, pushdown_rows_pruned=0, 
predicate_cache_inner_records=8, predicate_cache_records=8, 
scan_efficiency_ratio=21.94% (222/1.01 K)]

Review Comment:
   The same pattern exists in the other tests below as well



-- 
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]

Reply via email to