Repository navigation
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #25822 +/- ##
========================================
Coverage 82.72% 82.73%
========================================
Files 1147 1147
Lines 448179 448437 +258
Branches 448179 448437 +258
========================================
+ Hits 370751 371004 +253
- Misses 54898 54904 +6
+ Partials 22530 22529 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
a99b6c7 to
b876d88
Compare
|
Rebased onto current main and updated the accounting for the fully matched row-group path introduced by #25854. Extended the existing scan test to verify Bloom matched/pruned counters for statistics-disabled, mixed, and entirely fully matched cases. Restoring the previous increment makes the mixed-case assertion fail. Focused Bloom, row-group, display, and affected SQL logic tests pass locally, along with formatting checks. |
|
@xudong963 Thanks for approving the earlier workflow runs. I’ve since updated the Bloom accounting for the fully matched path introduced by #25854 and added regression tests. Fork CI is green on 22cf2a2; the upstream workflows are awaiting approval again. Could you please approve the latest runs when convenient? |
xudong963
left a comment
There was a problem hiding this comment.
THanks for the fix, solid fix
|
@xudong963 Thanks for reviewing and approving! Could you please merge this when convenient? |
Plan to leave it for two days to see if others wanna have a look |
Makes sense. |
Remove the idle Bloom pruning metric from the hash-join regression introduced by apache#25602. Statistics retain two of four row groups, while the fixture has no Bloom filters and records no Bloom pruning outcome.
22cf2a2 to
278c7db
Compare
|
@xudong963 I rebased onto current main and fixed the merge-queue failure in dynamic_row_group_pruning.slt. The relevant tests and extended suite pass locally. The new upstream workflow runs are awaiting approval again — could you please approve them when convenient? Thanks! |
Which issue does this PR close?
Rationale for this change
row_groups_pruned_bloom_filtercurrently reports retained row groups as Bloom filter matches even when Bloom pruning was not evaluated—for example, when Bloom filters are disabled or unavailable, no relevant Bloom statistics were loaded, or predicate evaluation failed.This makes
EXPLAIN ANALYZEsuggest that Bloom filters evaluated and matched row groups when they did not participate in the pruning decision. It also displays an idle Bloom pruning metric when all its counters are zero.A retained row group is not necessarily a Bloom filter match. The metric should describe actual Bloom pruning outcomes.
What changes are included in this PR?
predicate_evaluation_errors.row_groups_pruned_bloom_filterfrom physical-plan displays when all of its pruning counters are zero.The row-group access decisions and query results are unchanged.
What is the testing strategy for this PR?
Added
bloom_filter_pruning_error_retains_row_group_without_match, which verifies that a Bloom predicate evaluation error:predicate_evaluation_errors;The display tests verify that:
The following existing test coverage was also run:
parquet_integrationrow-group-pruning tests;core_integrationexplain-analyze tests;Formatting checks also pass.
Are there any user-facing changes?
Yes. Parquet Bloom filter pruning metrics in
EXPLAIN ANALYZEand other physical-plan displays more accurately represent actual Bloom filter evaluation. An entirely idlerow_groups_pruned_bloom_filtermetric is no longer displayed.There are no public API changes and no changes to query results or row-group access decisions.