Skip to content

Commit b876d88

Browse files
committed
fix: exclude fully matched row groups from Bloom metrics
1 parent ce6a859 commit b876d88

2 files changed

Lines changed: 42 additions & 1 deletion

File tree

‎datafusion/datasource-parquet/src/opener/mod.rs‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3353,6 +3353,32 @@ mod test {
33533353
.expect("row_groups_pruned_statistics metric is emitted")
33543354
}
33553355

3356+
/// Bloom pruning counters after a scan, as `(pruned, matched)`.
3357+
///
3358+
/// Direct `MetricsSet` lookup, not plan display: idle Bloom metrics are
3359+
/// omitted from displayed plans even when the counters remain registered.
3360+
///
3361+
/// Opening one file still registers this name more than once:
3362+
/// `prepare_open_file` creates `ParquetFileMetrics`, and each
3363+
/// `ParquetFileReaderFactory::create_reader` call (initial reader plus
3364+
/// Bloom replacement reader) creates another independent set. Sum the
3365+
/// counters the same way `MetricsSet::sum_by_name` aggregates pruning
3366+
/// metrics. Only the opener's set is incremented by Bloom pruning; the
3367+
/// reader copies stay at zero.
3368+
fn bloom_filter_pruning_metrics(metrics: &ExecutionPlanMetricsSet) -> (usize, usize) {
3369+
use datafusion_physical_plan::metrics::MetricValue;
3370+
match metrics
3371+
.clone_inner()
3372+
.sum_by_name("row_groups_pruned_bloom_filter")
3373+
{
3374+
Some(MetricValue::PruningMetrics {
3375+
pruning_metrics, ..
3376+
}) => (pruning_metrics.pruned(), pruning_metrics.matched()),
3377+
Some(_) => panic!("row_groups_pruned_bloom_filter is not a pruning metric"),
3378+
None => panic!("row_groups_pruned_bloom_filter metric is registered"),
3379+
}
3380+
}
3381+
33563382
#[tokio::test]
33573383
async fn test_prune_all_null_column_equality_from_file_statistics() {
33583384
// Regression: a column whose file statistics say every value is
@@ -4612,6 +4638,16 @@ mod test {
46124638
// contribute to this metric.
46134639
bloom_bytes.push(counter_metric_value(&metrics, "bytes_scanned"));
46144640
assert_eq!(collect_int32_values(stream).await, vec![1, 1, 1, 1]);
4641+
let (pruned, matched) = bloom_filter_pruning_metrics(&metrics);
4642+
assert_eq!(pruned, 0);
4643+
if stats_pruning {
4644+
// [1,1,1] is fully matched by statistics and skips Bloom.
4645+
// [0,1,2] still evaluates Bloom for `a = 1` and cannot prune.
4646+
assert_eq!(matched, 1);
4647+
} else {
4648+
// Statistics pruning is off, so both row groups evaluate Bloom.
4649+
assert_eq!(matched, 2);
4650+
}
46154651
}
46164652
assert!(bloom_bytes[1] > 0, "partial row group needs Bloom I/O");
46174653
assert!(
@@ -4650,6 +4686,10 @@ mod test {
46504686
.unwrap();
46514687
assert_eq!(counter_metric_value(&metrics, "bytes_scanned"), 0);
46524688
assert_eq!(collect_int32_values(stream).await, vec![1, 1, 1]);
4689+
let (pruned, matched) = bloom_filter_pruning_metrics(&metrics);
4690+
// Bloom reads and evaluation are skipped for an entirely fully matched file.
4691+
assert_eq!(pruned, 0);
4692+
assert_eq!(matched, 0);
46534693
}
46544694

46554695
#[test]

‎datafusion/datasource-parquet/src/row_group_filter.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,8 @@ impl RowGroupAccessPlanFilter {
452452
}
453453

454454
if self.access_plan.is_fully_matched(idx) {
455-
metrics.row_groups_pruned_bloom_filter.add_matched(1);
455+
// Statistics already proved every row matches. Bloom did not
456+
// evaluate this group, so do not record a Bloom outcome.
456457
continue;
457458
}
458459

0 commit comments

Comments
 (0)