ranflarion opened a new pull request, #24820: URL: https://github.com/apache/datafusion/pull/24820
## Which issue does this PR close? - Closes #24819. ## Rationale for this change `NestedLoopJoinExec` buffered the whole build side into one batch via `concat_batches`, which doubles peak memory while the copy runs (inputs and output coexist), and the concat output is never reserved in the memory pool, so the doubled peak is invisible to it. On an ~880 MB build side (repro in #24819, runnable with stock `datafusion-cli`), main peaks at 1738 MB RSS; with `--memory-limit 1g` it completes while peaking at 1739 MB, 1.7x its own limit. A single allocation also caps any one string column at `i32::MAX` bytes (the overflow reported in #23032) and can only be spilled or released wholesale. This PR keeps the build side as target-batch-size chunks instead. Same repro after the change: 899 MB peak (1.02x the build side), 900 MB under `-m 1g`. Wall time on the repro improves ~20% since the concat copy is gone. ## What changes are included in this PR? - `JoinLeftData` holds `Vec<RecordBatch>` chunks with prefix-sum `row_offsets`, a binary-search `locate(global_row) -> (chunk, local_row)`, and the build schema (so an empty build side keeps its shape). Zero-row chunks are dropped at construction. - `collect_left_input` feeds arrow's `BatchCoalescer` with `with_biggest_coalesce_batch_size(target/2)`: small batches are compacted to target size, batches at or above half the target pass through zero-copy. Reservation still charges each input batch's `get_array_memory_size` as before. - The build-side spill path writes through the same coalescer, so the spill file holds uniformly sized chunks and the memory-limited replay uses the read-back batches as the chunk list directly — the per-pass `concat_batches` in the memory-limited rebuild is gone too. - Probe and unmatched-left emission never cross a chunk boundary: ranges are clamped at chunk ends, `take`/slice use chunk-local indices, and the visited-left bitmap keeps global row numbers, so bitmap semantics (including the multi-partition rules from #24675 and the deferred emission from #24746) are unchanged. The output `BatchCoalescer` re-coalesces the occasionally smaller batch emitted at a chunk tail. One behavior note: a sliced batch at or above half the target size is now retained as-is, keeping its parent allocation alive, where the old concat incidentally un-pinned it by copying. The reservation charges the full parent buffers, so the pool over-counts rather than under-counts in that case; slice-aware accounting (dedup by allocation) is a planned follow-up. ## Are these changes tested? Existing coverage: the full NLJ suite including the memory-limited matrix and the one-shot re-execution test, `physical-plan` lib tests, the join fuzz suite (`--features extended_tests`), the `memory_limit` integration tests, and the join sqllogictests all pass. New unit tests: chunks retain the input buffers by pointer identity (the zero-copy bypass), and zero-row chunks are dropped with `locate()` boundary checks. Probe-throughput parity, medians over interleaved runs of ~1.6e10 pair evaluations, release builds, same machine: | build side arrives as | main | this PR | |---|---|---| | 250,000 batches of 8 rows | 7.358 s | 7.204 s | | 8192-row batches | 7.075 s | 7.039 s | ## Are there any user-facing changes? No. Plans, results, and metrics are unchanged; only the build side's in-memory layout and its peak memory differ. -- 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]
