Conversation
github-actions
Bot
added
physical-expr
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #25469 +/- ##
========================================
Coverage 82.42% 82.42%
========================================
Files 1138 1138
Lines 435429 435583 +154
Branches 435429 435583 +154
========================================
+ Hits 358889 359023 +134
- Misses 54839 54847 +8
- Partials 21701 21713 +12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| fn struct_field_mapping( | ||
| &self, | ||
| literal_args: &[Option<ScalarValue>], | ||
| ) -> Option<StructFieldMapping> { | ||
| Some(StructFieldMapping { | ||
| field_accessor: Arc::new(ScalarUDF::from(GetFieldFunc::new())), | ||
| fields: (0..literal_args.len()) | ||
| .map(|i| (vec![ScalarValue::Utf8(Some(format!("c{i}")))], i)) | ||
| .collect(), | ||
| }) | ||
| } |
There was a problem hiding this comment.
StructFunc from datafusion-functions crate already provides support for this, but instead of adding another dependency I thought it might be a better idea to have it defined separately here instead
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn test_tuple_in_bloom_pruning_preserves_correlation() -> Result<()> { |
There was a problem hiding this comment.
The unit test nicely covers both pruning and the crossed-pair correlation case, while the SLT exercises the real Parquet scan path. Would it be valuable to combine these in one end-to-end test with multiple row groups? For example, the test could include:
- A row group containing an exact tuple match.
- A row group that statistics cannot eliminate but Bloom filters can, verifying that it is counted as pruned in row_groups_pruned_bloom_filter.
- A crossed-pair row group that satisfies the derived per-column guarantees and therefore survives Bloom pruning, but produces no rows after evaluation of the original tuple predicate.
This would verify that the derived per-column conditions are used only for pruning, while the original tuple predicate is still applied for exact row filtering.
There was a problem hiding this comment.
thanks for the review! this makes sense to me, will extend this test
3 participants
Which issue does this PR close?
Closes #25463
Rationale for this change
Multi-column joins can produce filters such as:
DataFusion can evaluate this filter on individual rows, but it does not extract the allowed values for each column. This prevents bloom filters from using those values to skip row groups.
What changes are included in this PR?
a IN (1, 2)andb IN (10, 20).(1, 20)are still rejected.This will only enable bloom filter pruning for row groups.
What is the testing strategy for this PR?
Added tests for value extraction, NULLs, dictionary values, and reordered named fields.
Are there any user-facing changes?
Queries with supported tuple IN filters, including dynamic filters from multi-column joins, may read fewer row groups when Bloom filters are available. Query results remain unchanged.