Skip to content

perf: elide redundant HashJoin dynamic-filter membership predicates for contiguous integer domains - #23934

Open
hhhizzz wants to merge 4 commits into
apache:mainfrom
hhhizzz:codex/q39-contiguous-dynamic-filter
Open

perf: elide redundant HashJoin dynamic-filter membership predicates for contiguous integer domains#23934
hhhizzz wants to merge 4 commits into
apache:mainfrom
hhhizzz:codex/q39-contiguous-dynamic-filter

Conversation

@hhhizzz

@hhhizzz hhhizzz commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

HashJoin dynamic filters normally combine inclusive min/max bounds with exact
membership:

min <= probe_key
AND probe_key <= max
AND probe_key IN (build_keys)

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:

distinct_key_count == max - min + 1

The distinct build keys are a subset of the inclusive [min, max] integer
interval. If the two sets have the same cardinality, they are equal.
Evaluating an InListExpr or HashTableLookupExpr after that proof adds
per-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 membership form 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 domain
equivalence and remains applicable when Arrow RowFilter pushdown is enabled.

What changes are included in this PR?

  • Carry the build hash map's existing distinct-key count with the InList
    pushdown strategy; map-backed strategies reuse Map::num_of_distinct_key().
  • For a single, same-typed integer bound, compare the inclusive range
    cardinality with the distinct-key count using a widened u128 comparison.
  • Skip construction of the redundant InListExpr or
    HashTableLookupExpr after the proof succeeds, while preserving the bounds
    predicate.
  • Apply the same proof to CollectLeft filters and to each reported
    Partitioned filter branch. The count and bounds always describe the same
    build-key set.
  • Add the dynamic_filter_membership_predicates_elided metric, incremented once
    for each membership predicate omitted during filter finalization.
  • Fail closed for gapped domains, multiple join keys, non-integer or mismatched
    bound types, missing or reversed bounds, and ranges whose cardinality cannot
    be represented safely.
  • Propagate dynamic-filter expression construction errors in both CollectLeft
    and Partitioned modes.
  • Replace an unreachable partition-filter fallback with an explicit invariant:
    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:

  • contiguous and duplicate-containing InList inputs produce bounds only;
  • gapped InList inputs retain membership;
  • contiguous map-backed (ArrayMap) inputs omit the resulting
    HashTableLookupExpr;
  • gapped map-backed (ArrayMap) inputs retain the resulting
    HashTableLookupExpr;
  • signed ranges crossing zero are handled;
  • multi-column, non-integer, mismatched-type, reversed, and extreme ranges fail
    closed;
  • missing and null bounds fail closed;
  • bound conversion failures fail closed;
  • membership expression construction errors propagate in both CollectLeft
    and Partitioned modes;
  • the elision metric increments only when the proof succeeds;
  • existing partitioned CASE, bounds-only, and cancellation composition tests
    continue to pass.

The following checks pass:

cargo fmt --all -- --check
cargo clippy -p datafusion-physical-plan --lib --all-features -- -D warnings
cargo test -p datafusion-physical-plan joins::hash_join::shared_bounds::tests --lib

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.

Component Baseline Candidate
DataFusion bb75d925cb7a6b8e1c5dc22bead85efc49754fe3 118fd470f306f87616468ff58060b71f0e761bdb
Arrow b8b59fb38dcc5930d7a087bc3e17bd955a6c9f21 b8b59fb38dcc5930d7a087bc3e17bd955a6c9f21

Ratios are candidate/baseline; values below 1 favor the candidate.

Metric Ratio 95% CI Elapsed-time change
Suite total 0.983529 [0.976737, 0.989774] -1.65%
Equal-query geomean 0.975289 [0.968237, 0.982635] -2.47%
q39 0.967708 [0.949456, 0.985307] -3.23%
q39 separate-arm p95 0.946249 [0.933645, 0.987039] -5.38%

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 of 4.778 ms
(95% CI [3.227, 8.604]). Its absolute increase remained below the regression
guard 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.

@github-actions github-actions Bot added the physical-plan Changes to the physical-plan crate label Jul 28, 2026
@hhhizzz
hhhizzz force-pushed the codex/q39-contiguous-dynamic-filter branch from 3a4c7b3 to 118fd47 Compare July 28, 2026 07:54
@github-actions github-actions Bot added the sqllogictest SQL Logic Tests (.slt) label Jul 28, 2026
@codecov-commenter

codecov-commenter commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.71%. Comparing base (bb75d92) to head (5da6851).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #23934      +/-   ##
==========================================
+ Coverage   80.68%   80.71%   +0.03%     
==========================================
  Files        1095     1095              
  Lines      372534   372903     +369     
  Branches   372534   372903     +369     
==========================================
+ Hits       300579   300999     +420     
+ Misses      54015    53936      -79     
- Partials    17940    17968      +28     

☔ 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.

@hhhizzz
hhhizzz marked this pull request as ready for review July 28, 2026 13:50
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 sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Avoid redundant HashJoin dynamic-filter membership checks for contiguous integer build keys

2 participants