Repository navigation
Conversation
150f3df to
8783f23
Compare
8783f23 to
04430bb
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #25592 +/- ##
==========================================
+ Coverage 82.60% 82.61% +0.01%
==========================================
Files 1145 1145
Lines 444057 444584 +527
Branches 444057 444584 +527
==========================================
+ Hits 366796 367294 +498
+ Misses 54996 54994 -2
- Partials 22265 22296 +31 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Thanks for the contribution! I am closing this under DataFusion's AI Dump policy. This has the hallmarks of AI code that was not reviewed by the author: a 900-line alternative implementation for an issue already being worked elsewhere, with boilerplate local validation text duplicated across the author's PRs. 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. |
Which issue does this PR close?
Closes #25392.
This is a draft alternative to #25434, which addresses the same issue. It is not ready for merge.
Rationale for this change
Generic hash joins currently reserve lookup buckets for every build row even when many rows share a key. The row-index chain must still retain every row, but the lookup table only needs one entry per distinct hash. Reserving unused buckets can reject joins that would otherwise fit in a bounded memory pool.
This version retains hashbrown's existing
HashTable, grows its lookup index as distinct hashes arrive, and accounts for the old and new tables during growth. It avoids a custom compact table and post-build compaction or rebuild. It still has a sampling heuristic and Rust trait/API changes, so I do not claim that it is simpler overall than #25434. The alternative remains useful for comparing these tradeoffs.What changes are included in this PR?
NullAwareModehandling for correlatedNOT INis preserved.Prepared builds retain a fully preallocated lookup table charged to their single durable reservation. Applying adaptive map ownership there would underreport
reserved_bytes()and add a growth peak on top of the admitted concatenation copy; the existing prepared-build regression tests cover both contracts. The specialized perfect-hashArrayMappath is unchanged. This adds no spilling behavior. The upstream build-batch concatenation and computed-key reservations are retained, but this PR does not independently validate all payload-memory accounting.build_mem_usedreports retained map storage, not the temporary reservation peak or process RSS. The allocation estimate is tied to hashbrown 0.17's layout and is checked against actual allocation sizes in tests.What is the testing strategy for this PR?
Unit tests cover U32/U64 index widths, duplicate updates at full capacity, reservation-before-allocation, old/new overlap, rejected hints, cleanup, repeated and periodic hashes, and string-key HashJoin behavior in CollectLeft and Partitioned modes.
Validated on October 2, 2026 after merging
mainat4d167a1, PR head0697bc3(Windows x64, Rust 1.98.1):cargo fmt --all --check, the repository Markdown formatter andgit diff --check origin/main: passed.cargo clippy --offline --locked --all-targets --all-features -- -D warnings: passed, without lint allowances.cargo test --offline --locked -p datafusion-physical-plan --lib: 2,413 passed, 0 failed.The complete extended workspace suite and performance benchmarks were not rerun for this maintenance update. Earlier Windows validation had broader environment/baseline blockers; those historical attempts are not a full pass for this head. The PR remains a draft alternative pending upstream CI, comparative performance evidence and maintainer design feedback.
The following benchmark measurements are historical results from the previous base,
95bb0a0; they have not been rerun against4d167a1and do not establish performance for the current head. The native Criterion benchmark builds 200,000 UTF-8 rows in either 8,192-row batches or one batch and probes with one absent key. The identical benchmark source was built separately against that base and the prior PR head, with separate target directories. Input generation is outside timing. Two Windows runs used 10 samples / 1 s warmup / 2 s measurement and a focused recheck with 20 samples / 2 s warmup / 5 s measurement. Median times in milliseconds, first run / recheck:95bb0a0These are preliminary Windows measurements with visible run-to-run variation. They show a low-cardinality benefit in this workload and do not reproduce a large high-cardinality regression in the native benchmark, but they do not establish performance across distributions or platforms. An earlier, different map-only microbenchmark on a pre-rebase base measured a 14–15% unique-key slowdown; it remains historical evidence, not a result for this current branch. More complete CI and performance validation are needed before leaving Draft.
To run the benchmark:
cargo bench --locked -p datafusion-physical-plan --features test_utils \ --bench hash_join_semi_anti -- 'generic_hash_join_build/'For a base comparison, copy this benchmark file to the base checkout and use separate target directories.
Are there any user-facing changes?
Duplicate-heavy generic hash joins can use smaller memory pools while preserving all matching rows. No SQL syntax or configuration changes are introduced.
Rust API changes:
JoinHashMapType::update_from_iterand the shared insertion helper now returnResult<()>; the trait gains a requiredsizemethod and a defaultreserve_from_batchmethod, and the insertion helper accepts an optional reservation. Downstream implementations or callers of these public, primarily internal interfaces need adjustment. Please apply theapi changelabel.This contribution is AI-assisted.