Skip to content

Enable dynamic filters for range-partitioned joins#23854

Open
peterxcli wants to merge 5 commits into
apache:mainfrom
peterxcli:feat/hash-join-dynamic-filter-with-range-partition
Open

Enable dynamic filters for range-partitioned joins#23854
peterxcli wants to merge 5 commits into
apache:mainfrom
peterxcli:feat/hash-join-dynamic-filter-with-range-partition

Conversation

@peterxcli

@peterxcli peterxcli commented Jul 23, 2026

Copy link
Copy Markdown
Member

Which issue does this PR close?

Rationale for this change

Partitioned hash joins build one dynamic filter per build partition. Existing routing uses hash(key) % N, which cannot reproduce a Range partitioning layout.
Compatible Range co-partitioned joins instead need to route probe rows using their existing ordering and split points.

What changes are included in this PR?

  • Enable dynamic filter pushdown for hash joins with compatible Range-partitioned inputs.
  • Build a searched CASE expression that routes probe rows to the corresponding partition filter using the Range ordering and split points.
  • Move the TopK lexicographic filter builder into the shared ordering module for reuse.

Are these changes tested?

unit test.

Are there any user-facing changes?

no

@github-actions github-actions Bot added the physical-plan Changes to the physical-plan crate label Jul 23, 2026

@saadtajwar saadtajwar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! 😁

Comment thread datafusion/physical-plan/src/joins/hash_join/shared_bounds.rs Outdated
Comment thread datafusion/physical-plan/src/joins/hash_join/shared_bounds.rs Outdated
let mut filters: Vec<Arc<dyn PhysicalExpr>> = Vec::with_capacity(thresholds.len());

let mut prev_sort_expr: Option<Arc<dyn PhysicalExpr>> = None;
for (sort_expr, value) in sort_exprs.iter().zip(thresholds.iter()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

meganit: this is a pretty long for loop with a lot of logic in it, is it possible to make this more concise? Specifically I think having a lot of mutable variables can make this a bit hard to reason about - but that could just be me & my smooth brain lol

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually I just move the TopK::build_filter_expression to here, so this can be shared to build the comparison expression.

@peterxcli
peterxcli requested a review from saadtajwar July 24, 2026 19:15
@peterxcli

Copy link
Copy Markdown
Member Author

cc @gene-bordegaray @jayshrivastava @stuhood @adriangb — kindly pinging on this one. Please take a look if you have time, thanks!

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.59259% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.71%. Comparing base (1763417) to head (1a141f3).
⚠️ Report is 18 commits behind head on main.

Files with missing lines Patch % Lines
...physical-plan/src/joins/hash_join/shared_bounds.rs 93.44% 4 Missing and 4 partials ⚠️
...tafusion/physical-plan/src/joins/hash_join/exec.rs 91.17% 3 Missing ⚠️
datafusion/physical-plan/src/ordering.rs 94.73% 0 Missing and 3 partials ⚠️
datafusion/physical-plan/src/topk/mod.rs 33.33% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #23854      +/-   ##
==========================================
- Coverage   80.71%   80.71%   -0.01%     
==========================================
  Files        1090     1091       +1     
  Lines      369398   370486    +1088     
  Branches   369398   370486    +1088     
==========================================
+ Hits       298170   299038     +868     
- Misses      53458    53620     +162     
- Partials    17770    17828      +58     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-plan Changes to the physical-plan crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support dynamic filters for range co-partitioned joins

3 participants