perf: elide redundant HashJoin dynamic-filter membership predicates for contiguous integer domains - #23934
Draft
hhhizzz wants to merge 18 commits into
Draft
perf: elide redundant HashJoin dynamic-filter membership predicates for contiguous integer domains#23934hhhizzz wants to merge 18 commits into
hhhizzz wants to merge 18 commits into
Conversation
hhhizzz
force-pushed
the
codex/q39-contiguous-dynamic-filter
branch
from
July 28, 2026 07:54
3a4c7b3 to
118fd47
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #23934 +/- ##
==========================================
+ Coverage 81.14% 81.16% +0.01%
==========================================
Files 1110 1112 +2
Lines 386137 387547 +1410
Branches 386137 387547 +1410
==========================================
+ Hits 313347 314567 +1220
- Misses 54324 54488 +164
- Partials 18466 18492 +26 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
hhhizzz
marked this pull request as ready for review
July 28, 2026 13:50
hhhizzz
force-pushed
the
codex/q39-contiguous-dynamic-filter
branch
from
July 30, 2026 05:22
5da6851 to
e1e6e19
Compare
Contributor
|
run benchmarks |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing codex/q39-contiguous-dynamic-filter (05b8e8c) to 6636d6b (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 codex/q39-contiguous-dynamic-filter (05b8e8c) to 6636d6b (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 codex/q39-contiguous-dynamic-filter (05b8e8c) to 6636d6b (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 codex/q39-contiguous-dynamic-filter (05b8e8c) to 6636d6b (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 codex/q39-contiguous-dynamic-filter (05b8e8c) to 6636d6b (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 codex/q39-contiguous-dynamic-filter (05b8e8c) to 6636d6b (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
Author
|
I'll try to repro the degression on my server and figure out the reason. |
hhhizzz
marked this pull request as draft
August 1, 2026 13:37
Contributor
|
run benchmarks |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing codex/q39-contiguous-dynamic-filter (c11a512) to b4cde12 (merge-base) diff Run configurationrun benchmark tpchResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing codex/q39-contiguous-dynamic-filter (c11a512) to b4cde12 (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 codex/q39-contiguous-dynamic-filter (c11a512) to b4cde12 (merge-base) diff Run configurationrun benchmark tpcdsResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing codex/q39-contiguous-dynamic-filter (c11a512) to b4cde12 (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 codex/q39-contiguous-dynamic-filter (c11a512) to b4cde12 (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 codex/q39-contiguous-dynamic-filter (c11a512) to b4cde12 (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
Author
|
The benchmark results match the intended scope of this change. In 10 paired |
hhhizzz
marked this pull request as ready for review
August 3, 2026 03:12
github-actions
Bot
added
core
hhhizzz
marked this pull request as draft
August 14, 2026 00:21
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
HashJoin dynamic filters normally combine inclusive min/max bounds with exact
membership:
Keeping both predicates is useful for a gapped build domain: bounds provide
cheap short-circuiting and statistics pruning, while membership removes values
in the gaps.
For a single-column integer domain, however, membership is exactly equivalent
to the bounds when:
The distinct build keys are a subset of the inclusive
[min, max]integerinterval. If the two sets have the same cardinality, they are equal.
Evaluating an
InListExprorHashTableLookupExprafter that proof addsper-row CPU cost without rejecting any additional probe rows.
This PR canonicalizes that exact case to bounds only. It deliberately preserves
the bounds for their pruning and short-circuit value, and retains the existing
bounds AND membershipform whenever equivalence cannot be proven.This differs from #23701: that PR proposes skipping membership based on filter
placement when
pushdown_filters=false. This change is based on domainequivalence and remains applicable when Arrow
RowFilterpushdown is enabled.What changes are included in this PR?
InListpushdown strategy; map-backed strategies reuse
Map::num_of_distinct_key().cardinality with the distinct-key count using a widened
u128comparison.InListExprorHashTableLookupExprafter the proof succeeds, while preserving the boundspredicate.
CollectLeftfilters and to each reportedPartitionedfilter branch. The count and bounds always describe the samebuild-key set.
dynamic_filter_membership_predicates_elidedmetric, incremented oncefor each membership predicate omitted during filter finalization.
bound types, missing or reversed bounds, and ranges whose cardinality cannot
be represented safely.
CollectLeftand
Partitionedmodes.a partition reported as non-empty must produce a filter.
There is no additional scan or deduplication pass, no new configuration, no
Arrow dependency change, and no public API change.
Are these changes tested?
Yes. Focused unit coverage verifies that:
InListinputs produce bounds only;InListinputs retain membership;ArrayMap) inputs omit the resultingHashTableLookupExpr;ArrayMap) inputs retain the resultingHashTableLookupExpr;closed;
CollectLeftand
Partitionedmodes;continue to pass.
The following checks pass:
The focused test run completed with 16 passed and 0 failed.
End-to-end benchmark
I compared the merge-base baseline with the current PR head on TPC-DS SF10,
all 99 queries, using ten paired release rounds with alternating AB/BA order.
This is non-formal evidence collected through the
datafusion-expKubernetesrunner.
Both arms used the same Arrow revision and the same verified
tpcds-sf10-v1dataset. Parquet pushdown filters, filter reordering, and pruning were enabled
for every run. Each arm used a 24 CPU / 48 GiB Pod, 24 partitions, batch size
8192, and one iteration per query.
b4cde12a85b1201cb6addd8d328d30f4c5691507c11a51282d12b84f00738485a300cfab96fb9530fd2a99e6de6ea0f8a03ee20782cf852726d996cbNegative changes favor the candidate.
The candidate was faster in all ten paired full-suite rounds. The guarded
performance comparison passed, and no query crossed the configured material
regression threshold.
Two small-query slowdown signals remain visible: Q8 increased by 11.12 ms
(+7.74%), and Q73 increased by 3.27 ms (+7.36%). Neither is a material
full-suite regression under the predeclared relative-plus-absolute guard.
All 1,980 query executions completed successfully and row counts matched.
The order-sensitive capture reported indeterminate mismatches for Q31, Q65,
and Q71 because their SQL ordering is not total. The same instability occurs
within the baseline (and the same two states occur on both arms for Q65/Q71);
there is no candidate-only result state. An order-independent result digest is
needed before treating these queries as a strict correctness pass.