dwsmith1983 opened a new issue, #26792: URL: https://github.com/apache/datafusion/issues/26792
### Is your feature request related to a problem or challenge? `build_in_memory` in `datafusion/physical-plan/src/joins/hash_join/exec.rs` has two build paths. The `ArrayMap` branch concatenates the build batches first and builds from the single batch. The `JoinHashMap` branch hashes each buffered batch in reverse with per-batch offsets, and only concatenates afterwards. Once #26228 lands, both branches concatenate in the same forward order, so the two concat blocks are identical. @jayzhan211 suggested building from the concatenated batch in the hash map branch too (https://github.com/apache/datafusion/pull/26228), and @comphead pointed out that after #26228 this is close to a pure move. The current shape costs two things: - The offset bookkeeping (start at `num_rows`, subtract each batch's rows) is easy to get wrong. #26226 was a mismatch between those offsets and the concat order. - The hash map is allocated while the buffered batches are still alive, and the concat copy is made after that. For the non-prepared path, `concat_build_batches` frees the inputs once the copy exists, so concatenating first should let the map allocation happen after the inputs are gone. ### Describe the solution you'd like Hoist one concat above the `array_map_key_range` check, shared by both branches. In the hash map branch, walk the concatenated batch in reverse chunks of `max_batch_rows` (`batch.slice(offset, len)`, zero copy), so `hashes_buffer` stays bounded by `max_batch_rows` and the prepared-mode scratch reservation is unchanged. Each chunk's offset is then its slice start, so the separate offset tracking goes away. Edge case: `max_batch_rows` is 0 for an empty build, so a chunked loop needs a guard. Output must stay identical: the same rows in the same chains in the same order. The existing tests pin this (`join_left_anti_keeps_build_order_across_batches`, `join_inner_one_two_parts_left_randomly_ordered` over the hash map and array map configs, `prepared_build_matches_collect_left`). ### Describe alternatives you've considered Hashing the whole concatenated batch in one `update_hash` call is simpler, but `hashes_buffer` would grow to the full build size instead of `max_batch_rows`, and prepared mode reserves scratch by `max_batch_rows`. Leaving the code as it is after #26228 also works. If build benchmarks show no gain, the only argument for the change is the simpler indexing. ### Additional context Depends on #26228. I'll compare the `hj` benchmarks and build-side peak memory against `main`. ### Willingness to contribute I plan to contribute to this feature independently -- 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]
