Repository navigation
perf: normalize float hash join build keys once - #25972
Merged
jayzhan211 merged 27 commits intoOct 3, 2026
Merged
Conversation
A probe batch is emitted in chunks of at most batch_size rows, and each chunk rebuilt the multi-column key comparator from scratch: fresh boxed closures plus a scan of every float key column, on both sides, for -0.0. The comparator is now created lazily once per probe batch and kept on the stream until the next batch arrives. The array map lookup cloned its index scratch vectors into every chunk's arrays. It now moves them in and takes the allocations back after the output batch is built. build_batch_from_indices and build_batch_empty_build_side accept a SchemaRef, so each output batch shares the schema instead of deep copying it. Float plus integer key join, 60K build rows, 17.1M output rows: query 150 ms to 108 ms, HashJoinExec elapsed_compute 147 ms to 104 ms. With low fanout (one chunk per probe batch) and on hj Q23-25 and TPC-H q7/q9 at SF1, timings stay within run-to-run noise.
The comparator is only valid for one probe batch. Storing it on ProcessProbeBatchState ties its lifetime to the batch, so the manual resets in fetch_probe_batch and on the limit path go away and the probe arrays are released as soon as the batch finishes.
for_each_scope_match passed a fresh comparator slot to each chunk even though the build and probe scope arrays stay the same for the whole loop, so multi-column scopes rebuilt the comparator per chunk. Keep one slot for the loop. Spell out on equal_rows_arr that the same-arrays rule is not checked and that an empty slot is always safe. Add a null-aware anti join test with a two-column correlation scope that splits each scope lookup into several chunks.
The hash join key comparator is built once per probe batch, and each build rewrote -0.0 to +0.0 across the whole build key column. A join on float keys therefore rescanned the build side for every probe batch. Normalize the build keys once in collect_left_input and keep them as the stored build keys. The in-list and bounds dynamic filters and the build null check still read the original keys, and a rewritten copy is charged to the build reservation. Hash join's lookup now uses a comparator that skips the build-side pass; other joins keep the existing behavior. On TPC-H SF1 a (BIGINT, DOUBLE) key join drops from 96 ms to 26 ms of join time, in line with the same join on integer keys.
Contributor
Author
dwsmith1983
marked this pull request as ready for review
October 2, 2026 13:48
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #25972 +/- ##
========================================
Coverage 82.61% 82.62%
========================================
Files 1147 1147
Lines 444812 445004 +192
Branches 444812 445004 +192
========================================
+ Hits 367502 367670 +168
- Misses 54996 55007 +11
- Partials 22314 22327 +13 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…float-keys # Conflicts: # datafusion/physical-plan/src/joins/hash_join/exec.rs # datafusion/physical-plan/src/joins/hash_join/stream.rs # datafusion/physical-plan/src/joins/utils.rs
jayzhan211
approved these changes
Oct 3, 2026
jayzhan211
left a comment
Contributor
There was a problem hiding this comment.
Thanks @dwsmith1983 !
1 of 6 tasks
Omega359
pushed a commit
to Omega359/arrow-datafusion
that referenced
this pull request
Oct 11, 2026
## Which issue does this PR close? - Follow-up to apache#25658, from @jayzhan211's review there. No separate issue. This branch is stacked on apache#25658, so until that merges the diff here includes its commits. Only the last commit is new. ## Rationale for this change Hash joins on float keys get much slower as the build side grows. The comparator that checks key equality is built once per probe batch, and building it rewrites `-0.0` to `+0.0` across the whole build key column so that `-0.0 = 0.0` holds. That scan repeats for every probe batch even though the build side never changes, so a float-key join costs build rows times probe batches, while the same join on integer keys does not. ## What changes are included in this PR? - `collect_left_input` normalizes the build key columns once and keeps the result as the join's build keys. The in-list and bounds dynamic filters and the build-side null check still read the original keys, which they already did before this point. - A rewritten copy is only made when a column actually holds `-0.0`; otherwise the stored key shares the original buffer. Any copy is charged to the build-side memory reservation. - `JoinKeyComparator::new_with_normalized_left` and `equal_rows_arr_with_normalized_left` skip the build-side pass. Hash join's lookup uses them; sort-merge, piecewise merge, ASOF and symmetric hash joins keep the existing behavior. The query from the review, TPC-H SF1, `target_partitions = 1`, `HashJoinExec` `join_time`, median of five interleaved runs: ```sql SELECT count(*) FROM (SELECT l_orderkey AS k, CAST(l_quantity AS DOUBLE) AS q FROM lineitem WHERE l_orderkey < 1500000) b JOIN (SELECT l_orderkey AS k, CAST(l_quantity AS DOUBLE) AS q FROM lineitem) p ON b.k = p.k AND b.q = p.q; ``` | keys | before | after | |---|---|---| | `(BIGINT, DOUBLE)` | 96.4 ms | 25.7 ms | | `(BIGINT, BIGINT)` control | 24.9 ms | 24.8 ms | ## What is the testing strategy for this PR? New tests: - `join_key_comparator_with_normalized_left_skips_left_normalization` in `joins/utils.rs` pins the new constructor's contract. - `collect_left_input_stores_normalized_float_keys` checks that Float64 and Float32 build keys are stored with `+0.0` while the batch keeps `-0.0`, that no copy is made without `-0.0`, and that the copy is charged to the reservation. - `join_inner_single_float_key_negative_zero` covers a single float key with `-0.0` and `+0.0` on both sides, over batch sizes and both null equality modes. - `test_null_aware_left_anti_float_scope_negative_zero` covers both null-aware scope lookups with float scope keys. `join_inner_multi_key_float_zero_and_nulls_across_chunks` and the existing join tests and `joins`, `null_aware` and `subquery` sqllogictest files pass unchanged. ## Are there any user-facing changes? No.
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?
This branch is stacked on #25658, so until that merges the diff here includes its commits. Only the last commit is new.
Rationale for this change
Hash joins on float keys get much slower as the build side grows. The comparator that checks key equality is built once per probe batch, and building it rewrites
-0.0to+0.0across the whole build key column so that-0.0 = 0.0holds. That scan repeats for every probe batch even though the build side never changes, so a float-key join costs build rows times probe batches, while the same join on integer keys does not.What changes are included in this PR?
collect_left_inputnormalizes the build key columns once and keeps the result as the join's build keys. The in-list and bounds dynamic filters and the build-side null check still read the original keys, which they already did before this point.-0.0; otherwise the stored key shares the original buffer. Any copy is charged to the build-side memory reservation.JoinKeyComparator::new_with_normalized_leftandequal_rows_arr_with_normalized_leftskip the build-side pass. Hash join's lookup uses them; sort-merge, piecewise merge, ASOF and symmetric hash joins keep the existing behavior.The query from the review, TPC-H SF1,
target_partitions = 1,HashJoinExecjoin_time, median of five interleaved runs:(BIGINT, DOUBLE)(BIGINT, BIGINT)controlWhat is the testing strategy for this PR?
New tests:
join_key_comparator_with_normalized_left_skips_left_normalizationinjoins/utils.rspins the new constructor's contract.collect_left_input_stores_normalized_float_keyschecks that Float64 and Float32 build keys are stored with+0.0while the batch keeps-0.0, that no copy is made without-0.0, and that the copy is charged to the reservation.join_inner_single_float_key_negative_zerocovers a single float key with-0.0and+0.0on both sides, over batch sizes and both null equality modes.test_null_aware_left_anti_float_scope_negative_zerocovers both null-aware scope lookups with float scope keys.join_inner_multi_key_float_zero_and_nulls_across_chunksand the existing join tests andjoins,null_awareandsubquerysqllogictest files pass unchanged.Are there any user-facing changes?
No.