Conversation
github-actions
Bot
added
documentation
|
Thank you for opening this pull request! Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch). Details |
gruuya
force-pushed
the
hash-join-dynamic-pruning-minmax
branch
from
September 14, 2026 12:17
7b174d8 to
f116035
Compare
gruuya
changed the title
gruuya
force-pushed
the
hash-join-dynamic-pruning-minmax
branch
from
September 14, 2026 12:32
f116035 to
542e11a
Compare
gruuya
force-pushed
the
hash-join-dynamic-pruning-minmax
branch
from
September 14, 2026 12:58
542e11a to
4b9a166
Compare
gruuya
force-pushed
the
hash-join-dynamic-pruning-minmax
branch
from
September 14, 2026 13:04
4b9a166 to
e09baaa
Compare
Contributor
Author
|
run benchmark tpch |
|
Hi @gruuya, thanks for the request (#25292 (comment)). Only whitelisted users can trigger benchmarks. Allowed users: 2010YOUY01, Dandandan, Fokko, Jefffrey, Omega359, Rachelint, Rich-T-kid, adriangb, alamb, asubiotto, avantgardnerio, brunal, buraksenn, cetra3, codephage2020, coderfender, comphead, erenavsarogullari, etseidl, friendlymatthew, gabotechs, geoffreyclaude, grtlr, haohuaijin, jayzhan211, jonathanc-n, kevinjqliu, klion26, kosiew, kumarUjjawal, kunalsinghdadhwal, liamzwbao, mbutrovich, mkleen, mzabaluev, neilconway, rluvaton, sdf-jkl, timsaucer, xudong963, zhuqi-lucas. File an issue against this benchmark runner |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #25292 +/- ##
========================================
Coverage 82.42% 82.42%
========================================
Files 1140 1140
Lines 435586 435968 +382
Branches 435586 435968 +382
========================================
+ Hits 359042 359363 +321
- Misses 54828 54869 +41
- Partials 21716 21736 +20 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
@gruuya You can open a PR like it, to request permission: |
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
jayzhan211
reviewed
Sep 18, 2026
jayzhan211
left a comment
jayzhan211
left a comment
Contributor
There was a problem hiding this comment.
Thanks @gruuya , here is a suggestion:
The sorted domain is rebuilt for every PruningPredicate, about 3× per file (file, row-group, page). Each build pushes up to 100k scalars, then runs sort_unstable + dedup. The OnceLock scalar cache is also dropped by with_new_children, which runs per file through the dynamic-filter remap and the expr adapter.
A/B on default config (release-nonlto): 2000-file Parquet probe, unclustered keys so nothing prunes. Off is hash_join_dynamic_pruning_max_distinct_values = 0.
| build side | off | on (default) |
|---|---|---|
| 100k Int64 keys | 0.087–0.094 s | 0.71–0.78 s |
| 20k string keys | 0.090–0.095 s | 0.238–0.264 s |
Repro:
COPY (SELECT value AS a, value % 2000 AS p FROM generate_series(1, 400000) t(value))
TO 'many_int/' STORED AS PARQUET PARTITIONED BY (p);
COPY (SELECT value * 4 AS k FROM generate_series(1, 100000) t(value))
TO 'dim_int.parquet' STORED AS PARQUET;
CREATE EXTERNAL TABLE probe_i STORED AS PARQUET LOCATION 'many_int/';
CREATE EXTERNAL TABLE dim_i STORED AS PARQUET LOCATION 'dim_int.parquet';
SELECT count(*) FROM dim_i JOIN probe_i ON dim_i.k = probe_i.a;
SET datafusion.optimizer.hash_join_dynamic_pruning_max_distinct_values = 0;
SELECT count(*) FROM dim_i JOIN probe_i ON dim_i.k = probe_i.a;Sharing the scalar cache across with_new_children (diff below) is needed but recovered nothing on its own (0.74–0.79 s). The domain sort dominates.
Fix: build the sorted, deduplicated domain once per build side, behind the shared Arc. Have the pruning expression binary-search it, so per-file cost is O(log n). Until then, default the feature off or lower the cap.
- pruning_scalars: LazyPruningScalars,
+ pruning_scalars: Arc<LazyPruningScalars>,
@@ fn with_new_children
- Ok(Arc::new(HashTableLookupExpr::new(
- children,
- self.random_state.clone(),
- Arc::clone(&self.map),
- self.description.clone(),
- self.pruning_scalars.raw.clone(),
- )))
+ Ok(Arc::new(HashTableLookupExpr {
+ on_columns: children,
+ random_state: self.random_state.clone(),
+ map: Arc::clone(&self.map),
+ description: self.description.clone(),
+ pruning_scalars: Arc::clone(&self.pruning_scalars),
+ }))
gruuya
force-pushed
the
hash-join-dynamic-pruning-minmax
branch
2 times, most recently
from
September 20, 2026 07:36
6d1dfad to
2b782bf
Compare
Contributor
Author
|
Thanks @jayzhan211, that's a good point. I've clauded up a fix for that, alongside some other improvements that the agent flagged:
That said, the example you provide is a worst case one for this feature: Numbers I'm seeing > COPY (SELECT value AS a, value % 2000 AS p FROM generate_series(1, 400000) t(value))
TO 'many_int/' STORED AS PARQUET PARTITIONED BY (p);
COPY (SELECT value * 4 AS k FROM generate_series(1, 100000) t(value))
TO 'dim_int.parquet' STORED AS PARQUET;
CREATE EXTERNAL TABLE probe_i STORED AS PARQUET LOCATION 'many_int/';
CREATE EXTERNAL TABLE dim_i STORED AS PARQUET LOCATION 'dim_int.parquet';
+--------+
| count |
+--------+
| 400000 |
+--------+
1 row(s) fetched.
Elapsed 1.142 seconds.
+--------+
| count |
+--------+
| 100000 |
+--------+
1 row(s) fetched.
Elapsed 0.007 seconds.
0 row(s) fetched.
Elapsed 0.227 seconds.
0 row(s) fetched.
Elapsed 0.001 seconds.
> SELECT count(*) FROM dim_i JOIN probe_i ON dim_i.k = probe_i.a; SELECT count(*) FROM dim_i JOIN probe_i ON dim_i.k = probe_i.a; SELECT count(*) FROM dim_i JOIN probe_i ON dim_i.k = probe_i.a;
+----------+
| count(*) |
+----------+
| 100000 |
+----------+
1 row(s) fetched.
Elapsed 0.088 seconds.
+----------+
| count(*) |
+----------+
| 100000 |
+----------+
1 row(s) fetched.
Elapsed 0.072 seconds.
+----------+
| count(*) |
+----------+
| 100000 |
+----------+
1 row(s) fetched.
Elapsed 0.072 seconds.
> SET datafusion.optimizer.hash_join_dynamic_pruning_max_distinct_values = 0;
0 row(s) fetched.
Elapsed 0.000 seconds.
> SELECT count(*) FROM dim_i JOIN probe_i ON dim_i.k = probe_i.a; SELECT count(*) FROM dim_i JOIN probe_i ON dim_i.k = probe_i.a; SELECT count(*) FROM dim_i JOIN probe_i ON dim_i.k = probe_i.a;
+----------+
| count(*) |
+----------+
| 100000 |
+----------+
1 row(s) fetched.
Elapsed 0.090 seconds.
+----------+
| count(*) |
+----------+
| 100000 |
+----------+
1 row(s) fetched.
Elapsed 0.070 seconds.
+----------+
| count(*) |
+----------+
| 100000 |
+----------+
1 row(s) fetched.
Elapsed 0.080 seconds.So I'm wondering what this means about the default value of
Also any chance you or @2010YOUY01 can kick-off the tpcds benchmarks, since I don't have the permissions yet? |
gruuya
force-pushed
the
hash-join-dynamic-pruning-minmax
branch
from
September 20, 2026 08:38
2b782bf to
91a2f15
Compare
Contributor
|
run benchmarks |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing hash-join-dynamic-pruning-minmax (91a2f15) to a522cd5 (merge-base) diff Run configurationrun benchmark tpcdsResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing hash-join-dynamic-pruning-minmax (91a2f15) to a522cd5 (merge-base) diff Run configurationrun benchmark clickbench_partitionedResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing hash-join-dynamic-pruning-minmax (91a2f15) to a522cd5 (merge-base) diff Run configurationrun benchmark tpchResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing hash-join-dynamic-pruning-minmax (91a2f15) to a522cd5 (merge-base) diff Run configurationrun benchmark tpchCPU Details (lscpu)Details
Resource Usagetpch — base (merge-base)
tpch — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing hash-join-dynamic-pruning-minmax (91a2f15) to a522cd5 (merge-base) diff Run configurationrun benchmark tpcdsCPU Details (lscpu)Details
Resource Usagetpcds — base (merge-base)
tpcds — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing hash-join-dynamic-pruning-minmax (91a2f15) to a522cd5 (merge-base) diff Run configurationrun benchmark clickbench_partitionedCPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
Contributor
|
Thanks @gruuya, the caching fix looks good, and on ≈ off in the unclustered case matches what I'd expect now. On the default: rather than picking between 100k / 32K / 0, I wonder if we can drop hash_join_dynamic_pruning_max_distinct_values (and _max_size) altogether. The cap is only needed because we keep the exact key set: an O(n log n) sort under the OnceLock on the first file open, 8·n bytes that aren't accounted for, and the raw key array pinned by the plan. But pruning only asks "is there a build key in [min, max]?", and any superset of the keys answers that soundly. Precision is limited by how many containers there are, not by how many keys, so a fixed-size summary should lose very little. Concretely, a bucket bitmap over the key bounds we already compute in collect_left_input:
On your example from the description (40k keys, range ≈ 2M, bucket width 4, row groups 1000 wide) I'd expect the same 2000 → 200 row groups. It also simplifies the PR a bit:
Trade-offs I can see:
I haven't benchmarked any of this yet. Happy to prototype it on top of your branch and post A/B numbers (your clustered case, the unclustered repro, and a clustered build side past 100k keys) if you think it's worth pursuing. If you'd rather land the current approach first, I'd lean towards a lower default for now and do this as a follow-up. |
Contributor
Author
|
Ok, I get it now, the bucket bitmap approach you suggest does seem like a more sophisticated approach @jayzhan211. We trade-off pruning precision against pruning speed: make it fixed cost but we sometimes scan more false positives than we need to. The pathological case in that regards seems to be when there are a lot of (file/row-group/page) containers, and the build side has few-ish values spanning a big range. In that case each bucket spans a large-ish sub-range as well, and the ones that are populated by the sparse values will falsely "light-up" many redundant containers alongside the correct one. Probably mitigated easily to some degree by just defaulting to 2^20 for the bucket size. Either way it would be a net win over the default state today. Let me see how much of the PR is salvageable if we pick that direction. Doesn't make much sense to me to merge this with the new config and then remove it soon after in the follow-up. |
Contributor
Author
@jayzhan211 if you're still up for doing this please let me know. I do have a PoC clauded up but i'd need to do a self-review first before opening up a PR, and it might take you less time to do the same. Benchmarking shows the PoC is within the margin of this branch when it comes to the unclustered scenario (which is the default in tpc-{h,ds}), but it's slightly more complex. |
This was referenced Sep 22, 2026
Contributor
Author
|
Alright, I've unslopped and polished the PoC, and opened a PR that uses bitmap buckets: #25602 I think that's a more elegant approach to the problem; same effect (clustered dynamic pruning through a join), but without any extra configs, so on by default (though it only works for integer-like keys for now). |
Exposes HashTableLookupExpr's single-column build values through the same compact sorted-domain rewrite ordinary large IN-lists already get (PrimitiveInListPruningExpr/StringInListPruningExpr), so row-group/file min/max stats alone can exclude containers - no bloom filter fetch, no LiteralGuarantee. On by default via hash_join_dynamic_pruning_max_distinct_values (default 100_000); set to 0 to disable. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Cache the built domain expression on the lookup and rebind it to each container's statistics, and see through the CASE that routes a partitioned join's per-partition filters. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gruuya
force-pushed
the
hash-join-dynamic-pruning-minmax
branch
from
September 22, 2026 12:22
91a2f15 to
39f2d8c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Rationale for this change
Avoid scanning redundant files/row-groups/pages in the probe side of hash joins, based on the values dictated by the build side.
What changes are included in this PR?
PushdownStrategy::Map/HashTableLookupExprto carry the build side values from a hash joinbuild_predicate_expressionto build the associated pruning expression fromHashTableLookupExprbuild_predicate_expressionso that it now pushes down pruning forCaseExprs, since that also unlocks the partitioned hash-join scenario this pr targetsThis then exploits the pre-existing compact pruning mechanism (
CompactInListDomain), whereby build-side values are first sorted, and a binary search can then eliminate all containers that don't span a single value, even if naively container's range overlaps the broad min/max bounds of the build-side values.What is the testing strategy for this PR?
Unit tests added covering all changes.
Also tested manually that the problem from the issue is resolved now
Note that the scanned rows are shrunk 10x (
output_rows=1.99 Mvsoutput_rows=200.0 K), and consequently the execution time is improved 5x (0.096vs0.019seconds). It would be good to benchmark this more broadly.Are there any user-facing changes?
Yes, the two new configs mirroring the in-list ones, as well as the construction API for
HashTableLookupExpr, which now accepts an optional values arg too.