dwsmith1983 opened a new pull request, #25972:
URL: https://github.com/apache/datafusion/pull/25972

   ## Which issue does this PR close?
   
   - Follow-up to #25658, from @jayzhan211's review there. No separate issue.
   
   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.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 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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to