Repository navigation
refactor: build the hash join hash map from the concatenated build batch - #26793
Open
dwsmith1983 wants to merge 8 commits into
Open
dwsmith1983 wants to merge 8 commits into
dwsmith1983 wants to merge 8 commits into
Conversation
On the hash map build path the build batches were concatenated in reverse batch order, so build rows emitted by index (anti and semi joins, unmatched outer rows) came out last batch first. Batches are still inserted last to first so each key's chain keeps its earliest row on top, but every batch now gets its forward-order start offset and the batches are concatenated forward. Closes apache#26226
…uild-order # Conflicts: # datafusion/physical-plan/src/joins/hash_join/exec.rs
Drop the new inner duplicate-key test, which passes with or without the fix, and run join_inner_one_two_parts_left_randomly_ordered over the hash map and array map configs instead. Drop the prepared build test: prepared_build_matches_collect_left already covers multi-batch hash map builds. Reword the LeftAnti test doc so it does not read as an ordering guarantee.
Concatenate the build batches once for both the array map and hash map paths. The hash map is then filled from slices of the concatenated batch, last slice first, so its chains stay the same without per-batch offset tracking. The inputs are released before the map is allocated, which lowers the build-side peak memory. Closes apache#26792.
This was referenced Oct 27, 2026
dwsmith1983
marked this pull request as ready for review
October 27, 2026 15:43
…oin-concat-first # Conflicts: # datafusion/physical-plan/src/joins/hash_join/exec.rs
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.
Note
Stacked on #26228. Until that merges, this branch also carries its commits; the change here is the last commit. Opened as a draft for that reason.
Which issue does this PR close?
Rationale for this change
build_in_memoryhad two build paths that only shared a concat. TheArrayMappath concatenated first. TheJoinHashMappath hashed each buffered batch with its own offset and concatenated afterwards, and #26226 came from those offsets and the concat order disagreeing. After #26228 both paths concatenate the same way, so they can share one concat (as @jayzhan211 suggested and @comphead pointed out on #26228).Building after the concat also lowers the build-side peak on the non-prepared path. The inputs are released once their copy exists, so the hash map is no longer allocated while the inputs and the copy are both alive: the peak goes from about
inputs + map + copytomax(2 * inputs, copy + map).What changes are included in this PR?
build_in_memoryconcatenates once, then picks the map.array_map_key_rangegets the single concatenated batch, and the key arrays are evaluated once for both paths.max_batch_rowsrows at a time. Rows are still inserted from the highest index down, so the chains, and therefore the join output, are identical.hashes_bufferand the prepared scratch reservation stay bounded bymax_batch_rows. An empty build is guarded (max_batch_rowsis 0 there).HashJoinExecdoc and diagram describe the new build.What is the testing strategy for this PR?
Existing order tests still pass unchanged (
join_left_anti_keeps_build_order_across_batches,join_inner_one_two_parts_left_randomly_ordered,prepared_build_matches_collect_left). New tests:join_keeps_build_order_across_uneven_batches: build batches of 2, 3 and 2 rows, so the slices do not line up with the batches, with duplicate and NULL keys, Inner and LeftAnti, under both null equalities, over the hash map and array map configs.join_build_side_without_batches: an empty build on the hash map path.join_build_concat_is_reservednow checks the lower peak per map type: limits that fit the new order but not the old one, plus a case that fails at the copy (array map) or at the map once the inputs are gone (hash map). These fail on the previous code.Are there any user-facing changes?
No. Lower build-side peak memory for in-memory hash join builds.