Conversation
Keep small builds compact, retain large builds with batch-local keys and bounded coalescing, and gather only referenced sources. Cover computed and composite keys, encoded and nested payloads, perfect hashing, memory admission, and bounded final output. Add a reproducible build-layout benchmark suite.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #25889 +/- ##
==========================================
+ Coverage 82.58% 82.61% +0.03%
==========================================
Files 1144 1146 +2
Lines 441510 444014 +2504
Branches 441510 444014 +2504
==========================================
+ Hits 364615 366843 +2228
- Misses 54844 54980 +136
- Partials 22051 22191 +140 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
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?
Part of #23031. Related to #23032; this is a hash-join-specific alternative for comparing the implementation and performance tradeoffs, not a replacement for its nested-loop or piecewise-merge join work.
Rationale for this change
Hash joins currently concatenate the entire build relation into one Arrow batch. For a large build this requires admitting a second payload allocation while the input batches are still retained, and combines individually valid variable-width arrays into one offset-limited array.
This PR keeps the small-build path and can retain large builds as batches. The scope includes the full build/probe/output path and a reproducible benchmark suite, rather than only exposing a storage helper.
What changes are included in this PR?
The batched path currently applies to ordinary hash joins above the 64 MiB compact-build threshold. Prepared reusable builds and null-aware joins retain their existing contiguous state contracts. This does not add spilling, change other join operators, or wire the representation into Comet.
Dictionary-heavy builds that successfully compact do not receive the batched representation's peak-memory benefit. Copy admission uses the existing payload-buffer estimate, not a complete bound on Arrow kernel scratch or process RSS; actual retained buffers are reconciled afterward.
Are these changes tested?
0e64452c64659be1c71e90bbec67974119a2ab8a, including computed-key, encoding, all-join-type, fetch, memory-limit, slice-retention, metadata-capacity, probe-preprocessing, coalescing, gather, dictionary-capacity, optional-compaction rollback, and wide-output order/copy regressions.cargo clippy --workspace --all-targets --all-features -- -D warnings, formatting, license headers, and spelling checks passed.Local validation caveat: the dependency mirror does not yet serve the locked
thiserror 2.0.21. Validation copies usethiserrorandthiserror-implat2.0.20; the PR'sCargo.lockis unchanged. Both benchmark revisions use the same temporary dependency lockfile.The concurrent SQL logic run also emitted a non-failing 79.4 MB allocator-versus-reservation drift diagnostic. The runner explicitly warns that file/consumer attribution is unreliable with concurrent files; this is not evidence attributing the drift to the join, nor a claim of complete allocator accounting.
Performance
Matched measurements against base
1d9be2e10794994fc1495c35363658dc7db0adf7have driven fixes for tiny-batch and repeated dictionary workloads; correctness validation passes. The latest head's dictionary pilots passed, but the broader run exposed a material HJ Q11 slowdown and variation in compact-path controls. A subsequent fresh-process ABBA/BAAB diagnostic did not reproduce that slowdown, but unrelated Java workloads overlapped the diagnostic, so it does not settle performance attribution in either direction. Untimed plan captures confirmed that Q11 exercises batched ArrayMap output and Q20 is a compact ArrayMap control, with matching row and batch counts between revisions. The full run was also interrupted before its last TPC-H process when unrelated compilation started; all 22 persisted TPC-H value checks passed, but that run is incomplete performance evidence. All adverse, confounded, and interrupted runs are retained as investigation data. This remains a draft pending a quiet benchmark window, with no final speedup claim or performance table yet. Final evidence will compare the exact PR base with the final published head. The synthetic harness checks output row counts and matched build-ID sums, and separately records peak reservations, which are not process RSS.Reproduction settings for the final comparison:
release-nonlto, incremental compilation disabled, no extra Rust flags. No task-owned compilation or test jobs overlap the measurements. Host snapshots record unrelated activity; these are not dedicated-host timings.tpchgen-cli 3.0.0, one part, ZSTD(1); 4 partitions, batch size 8,192, greedy memory pool limited to16G. One warmup process per variant, then base/head/head/base with all three query iterations retained per process.Build the runner with
cargo build --profile release-nonlto -p datafusion-benchmarks --bin benchmark_runner. The synthetic benchmark command is:Run each SQL suite with
benchmark_runner hjorbenchmark_runner tpch --format parquet, adding--scale-factor 10 --path DATA_PARENT --partitions 4 --batch-size 8192 --mem-pool-type greedy --memory-limit 16G --iterations 3 --result-mode none.DATA_PARENTcontainstpch_sf10. Separate TPC-H warmups use--iterations 1 --result-mode persistto validate full result values before timing; the final results explain the Q11 tie-order and Q16 persistence-order qualifications.Are there any user-facing changes?
No public API or configuration changes. Large ordinary hash joins may use less build memory by avoiding the full payload copy. Query results and the existing Arrow key-coercion requirements are unchanged.