You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "QinXi-ai (via GitHub)" <gi...@apache.org> on 2026/09/10 16:07:20 UTC

[PR] feat: add opt-in probe selection for partitioned inner hash joins [datafusion]

QinXi-ai opened a new pull request, #25159:
URL: https://github.com/apache/datafusion/pull/25159

   ## Which issue does this PR close?
   
   Partially addresses #15382. This PR covers the probe side of eligible partitioned inner hash joins; it does not close the broader issue.
   
   ## Rationale for this change
   
   Hash repartitioning currently copies every probe payload column before the join knows which rows match. For wide inputs with few matches, much of that copying produces rows the join immediately discards.
   
   This change provides an opt-in path that shares the original probe batch across partitions and materializes payload rows only when producing join results. Join keys are still gathered into compact arrays for lookup and collision checks.
   
   ## What changes are included in this PR?
   
   - Add `datafusion.execution.enable_hash_join_probe_selection`, defaulting to `false`, with configuration documentation and SHOW ALL expectations.
   - Use a private selection exchange when a partitioned INNER `HashJoinExec` directly consumes an unordered hash `RepartitionExec` on matching column keys, with more than one partition and an unlimited memory pool. Other plans retain the existing exchange, including finite-memory execution and dynamic/null-aware filtering.
   - Keep payload ownership and its memory reservation shared across bounded output queues. Tie producer lifetime to consumers and release unclaimed output queues on cancellation. Forward source errors and task panics to consumers.
   - Probe compact keys, resolve hash collisions, then translate selected positions back to original rows before join filters and output projection.
   - Add physical-plan differential/lifecycle tests, SQL integration coverage, and a reproducible full-operator benchmark with path-activation and output-cardinality assertions.
   - Include two Windows-only lint compatibility cleanups: remove an existing redundant fallback semicolon and acknowledge the async signature of an example whose Windows branch is synchronous.
   
   The public `RecordBatchStream` contract and serialized plan format are unchanged. The private exchange avoids exposing selection-bearing batches to operators that expect ordinary batches. Build-side selection, spilling selection batches, other join types, and generalized selection propagation remain follow-up work.
   
   ## What is the testing strategy for this PR?
   
   Differential tests compare complete output multisets with the materialized path across seeds, partition counts, map implementations, null equality, duplicate keys, filters, and small output batches. Additional tests cover unsupported-plan fallback, cancellation with an exchange retained, source errors/panics with idle outputs, and reservation failure. SQL tests exercise Int64, Utf8 and Utf8View keys with filters and embedded projection, asserting both exact rows and the path-activation metric.
   
   Windows validation passed full-target/full-feature Clippy with warnings denied, formatting, the five selection tests with forced hash collisions, SQL integration, and the changed SHOW ALL expectations. The core library's 467 tests also passed. The full extended workspace run was stopped after an existing spill test exceeded its 20-second attempt budget under load; the same test passed all 12 attempts after heavy concurrent work stopped, without changing its threshold. The complete information_schema run was stopped after three of four files completed; its changed SHOW ALL section plus original configuration cleanup passed separately. Full workspace and full-file SLT completion remain unverified, and Linux CI has not run.
   
   Benchmark measurements include both exchanges, build, probe, output collection, and reservation release; input generation is outside timing. Cases vary payload width/type, partition count, match coverage and skew. The path remains disabled by default because narrow and skewed cases can regress. Reported reservation peaks are engine accounting; process working-set measurements are reported separately.
   
   In the final 1,344-measurement Windows run (32 cases, 21 alternating samples per path), median paired speedups were 11.27x for Utf8 and 2.62x for wide integer payloads at 8 partitions and 10% uniform match coverage. Narrow 32-partition cases regressed to about 0.80x–0.85x. These are operator benchmarks, not general SQL or production claims. Default-off/base comparisons were noisy: three full-scan median time ratios were 1.05/1.03/1.13; a follow-up with adjacent, order-alternated processes on four representative cases gave 1.00/1.03/0.95. These measurements do not establish zero default-path overhead.
   
   Memory accounting also has a tradeoff: the Utf8/8/10% uniform case increased peak reservations from 9.77 MB to 44.29 MB because original batches remain retained. Separate process runs covering uniform and skewed inputs observed approximately 91–94 MB peak Windows working set with the option off and 77 MB with it on. Reservations and process working set measure different things; this is another reason to keep the initial path opt-in and restricted to unlimited pools.
   
   ## Are there any user-facing changes?
   
   Yes: the new opt-in execution configuration and `probe_selection_partitions` metric. Existing behavior remains the default. No public API or wire-format changes are required.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: add opt-in probe selection for partitioned inner hash joins [datafusion]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #25159:
URL: https://github.com/apache/datafusion/pull/25159#issuecomment-5949483106

   ## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/25159?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :x: Patch coverage is `91.66667%` with `22 lines` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 82.60%. Comparing base ([`4d167a1`](https://app.codecov.io/gh/apache/datafusion/commit/4d167a167228a424ef8cc29fd6bea5f17fc23c2a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`f90f64b`](https://app.codecov.io/gh/apache/datafusion/commit/f90f64bb77e340789a38f08a9a0350082527a015?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 1 commits behind head on main.
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/datafusion/pull/25159?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [...ion/physical-plan/src/joins/hash\_join/selection.rs](https://app.codecov.io/gh/apache/datafusion/pull/25159?src=pr&el=tree&filepath=datafusion%2Fphysical-plan%2Fsrc%2Fjoins%2Fhash_join%2Fselection.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9waHlzaWNhbC1wbGFuL3NyYy9qb2lucy9oYXNoX2pvaW4vc2VsZWN0aW9uLnJz) | 87.33% | [11 Missing and 8 partials :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/25159?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...tafusion/physical-plan/src/joins/hash\_join/exec.rs](https://app.codecov.io/gh/apache/datafusion/pull/25159?src=pr&el=tree&filepath=datafusion%2Fphysical-plan%2Fsrc%2Fjoins%2Fhash_join%2Fexec.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9waHlzaWNhbC1wbGFuL3NyYy9qb2lucy9oYXNoX2pvaW4vZXhlYy5ycw==) | 97.22% | [0 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/25159?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...fusion/physical-plan/src/joins/hash\_join/stream.rs](https://app.codecov.io/gh/apache/datafusion/pull/25159?src=pr&el=tree&filepath=datafusion%2Fphysical-plan%2Fsrc%2Fjoins%2Fhash_join%2Fstream.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9waHlzaWNhbC1wbGFuL3NyYy9qb2lucy9oYXNoX2pvaW4vc3RyZWFtLnJz) | 97.61% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/25159?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@           Coverage Diff            @@
   ##             main   #25159    +/-   ##
   ========================================
     Coverage   82.60%   82.60%            
   ========================================
     Files        1145     1146     +1     
     Lines      444057   444290   +233     
     Branches   444057   444290   +233     
   ========================================
   + Hits       366796   367001   +205     
   - Misses      54996    55009    +13     
   - Partials    22265    22280    +15     
   ```
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/25159?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: add opt-in probe selection for partitioned inner hash joins [datafusion]

Posted by "QinXi-ai (via GitHub)" <gi...@apache.org>.
QinXi-ai commented on PR #25159:
URL: https://github.com/apache/datafusion/pull/25159#issuecomment-5621723633

   I am opening this as a draft to get feedback on the design direction before extending the implementation.
   
   The main questions I would appreciate feedback on are:
   
   1. Is a private, probe-side selection exchange owned by HashJoinExec an acceptable first step for #15382, or should this wait for a reusable selection representation across operators?
   2. Is the initial scope useful: opt-in partitioned INNER joins, direct matching hash repartition, and unlimited memory pools? In particular, the original-batch retention increases peak reservations in the low-match Utf8 case even though the measured process working set decreases.
   3. What benchmark and validation gates would you want before considering this ready? The Windows operator results show substantial gains for wide, low-match inputs, regressions for some narrow inputs, and noisy default-off comparisons. Full workspace/SLT completion and Linux CI are still outstanding as detailed in the description.
   
   The intent is to evaluate this limited design, not to close the broader issue or enable it by default. I would be happy to split out the benchmark or adjust the integration boundary based on maintainer feedback.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: add opt-in probe selection for partitioned inner hash joins [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #25159:
URL: https://github.com/apache/datafusion/pull/25159#issuecomment-6506175139

   Thanks for the contribution! I am closing this under DataFusion's AI Dump policy.
   
   https://datafusion.apache.org/contributor-guide/ai-policy.html#what-is-an-ai-dump-and-why-it-is-not-helpful
   
   This has the hallmarks of AI code that was not reviewed by the author: an 1,100-line opt-in hash join feature draft with new config, benchmarks and lifecycle tests, with boilerplate local validation text duplicated across the author's PRs and no reviewer able to engage.
   
   If you would like to continue, please open a focused PR in your own words that you have reviewed and understand, and we are happy to help with review.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: add opt-in probe selection for partitioned inner hash joins [datafusion]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25159:
URL: https://github.com/apache/datafusion/pull/25159#issuecomment-5949151608

   <!-- semver-check-comment -->
   Thank you for opening this pull request!
   
   Reviewer note: [cargo-semver-checks](https://github.com/obi1kenobi/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>
   <summary>Details</summary>
   
   ```
        Cloning apache/main
       Building datafusion v55.1.0 (current)
          Built [  62.122s] (current)
        Parsing datafusion v55.1.0 (current)
         Parsed [   0.039s] (current)
       Building datafusion v55.1.0 (baseline)
          Built [  60.608s] (baseline)
        Parsing datafusion v55.1.0 (baseline)
         Parsed [   0.039s] (baseline)
       Checking datafusion v55.1.0 -> v55.1.0 (no change; assume patch)
        Checked [   0.867s] 223 checks: 223 pass, 31 skip
        Summary no semver update required
       Finished [ 125.388s] datafusion
       Building datafusion-common v55.1.0 (current)
          Built [  33.995s] (current)
        Parsing datafusion-common v55.1.0 (current)
         Parsed [   0.069s] (current)
       Building datafusion-common v55.1.0 (baseline)
          Built [  33.920s] (baseline)
        Parsing datafusion-common v55.1.0 (baseline)
         Parsed [   0.069s] (baseline)
       Checking datafusion-common v55.1.0 -> v55.1.0 (no change; assume patch)
        Checked [   1.119s] 223 checks: 222 pass, 1 fail, 0 warn, 31 skip
   
   --- failure constructible_struct_adds_field: struct exhaustively constructible through public API adds field ---
   
   Description:
   A pub struct that could be exhaustively constructed with a literal using only public API has a new pub field, breaking existing exhaustive literals.
           ref: https://doc.rust-lang.org/reference/expressions/struct-expr.html
          impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.50.0/src/lints/constructible_struct_adds_field.ron
   
   Failed in:
     field ExecutionOptions.enable_hash_join_probe_selection in /home/runner/work/datafusion/datafusion/datafusion/common/src/config.rs:896
   
        Summary semver requires new major version: 1 major and 0 minor checks failed
       Finished [  70.248s] datafusion-common
       Building datafusion-physical-plan v55.1.0 (current)
          Built [  39.822s] (current)
        Parsing datafusion-physical-plan v55.1.0 (current)
         Parsed [   0.184s] (current)
       Building datafusion-physical-plan v55.1.0 (baseline)
          Built [  40.027s] (baseline)
        Parsing datafusion-physical-plan v55.1.0 (baseline)
         Parsed [   0.186s] (baseline)
       Checking datafusion-physical-plan v55.1.0 -> v55.1.0 (no change; assume patch)
        Checked [   0.959s] 223 checks: 223 pass, 31 skip
        Summary no semver update required
       Finished [  82.407s] datafusion-physical-plan
       Building datafusion-sqllogictest v55.1.0 (current)
          Built [ 103.836s] (current)
        Parsing datafusion-sqllogictest v55.1.0 (current)
         Parsed [   0.024s] (current)
       Building datafusion-sqllogictest v55.1.0 (baseline)
          Built [ 104.419s] (baseline)
        Parsing datafusion-sqllogictest v55.1.0 (baseline)
         Parsed [   0.025s] (baseline)
       Checking datafusion-sqllogictest v55.1.0 -> v55.1.0 (no change; assume patch)
        Checked [   0.125s] 223 checks: 223 pass, 31 skip
        Summary no semver update required
       Finished [ 211.352s] datafusion-sqllogictest
   ```
   
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: add opt-in probe selection for partitioned inner hash joins [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed pull request #25159: feat: add opt-in probe selection for partitioned inner hash joins
URL: https://github.com/apache/datafusion/pull/25159


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org