perf: elide redundant HashJoin dynamic-filter membership predicates for contiguous integer domains - #23934
perf: elide redundant HashJoin dynamic-filter membership predicates for contiguous integer domains#23934hhhizzz wants to merge 5 commits into
Conversation
3a4c7b3 to
118fd47
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #23934 +/- ##
==========================================
+ Coverage 80.86% 80.87% +0.01%
==========================================
Files 1099 1099
Lines 374345 374696 +351
Branches 374345 374696 +351
==========================================
+ Hits 302710 303043 +333
- Misses 53595 53610 +15
- Partials 18040 18043 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
5da6851 to
e1e6e19
Compare
|
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 |
|
I'll try to repro the degression on my server and figure out the reason. |
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 unchanged baseline with this patch on TPC-DS SF10, all 99
queries, using 10 alternating release rounds. Both arms used the same Arrow
commit and enabled Parquet pushdown filters, filter reordering, and pruning.
All queries and rounds completed successfully.
bb75d925cb7a6b8e1c5dc22bead85efc49754fe3118fd470f306f87616468ff58060b71f0e761bdbb8b59fb38dcc5930d7a087bc3e17bd955a6c9f21b8b59fb38dcc5930d7a087bc3e17bd955a6c9f21Ratios are candidate/baseline; values below 1 favor the candidate.
No query triggered the predeclared median or p95
relative-plus-absolute regression guards.
The largest relative point slowdown was q41, with a ratio of
1.096873(95% CI
[1.045096, 1.170716]) and a paired median increase of4.778 ms(95% CI
[3.227, 8.604]). Its absolute increase remained below the regressionguard threshold.
The benchmark is end-to-end evidence for the patch. The runner did not collect
per-query membership-elision metric hits, so this is not a claim that every
TPC-DS query exercises the optimized path.
The current PR head also contains follow-up coverage tests and tightens an
unreachable fallback into an explicit invariant. These follow-up changes do not
alter the valid-path behavior measured above.
Are there any user-facing changes?
There are no public API or configuration changes. The rewrite is
semantics-preserving.
The current PR head also contains follow-up coverage tests and models non-empty
membership sources explicitly, eliminating an unreachable fallback without
changing valid-path behavior.