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

   
   PR: fix: deduplicate StringView/BinaryView data buffer references in 
CollectLeft HashJoinExec
   Which issue does this PR close?
   Closes #25712
   Rationale for this change
   Queries using chained CollectLeft hash joins on StringView / BinaryView 
(e.g., Utf8View, BinaryView) payload columns exhibited exponential buffer 
reference growth, causing ~10x query slowdowns and ~7x higher RSS on TPC-DS Q64 
with pushdown_filters = true.
   
   Root cause: Arrow's concat kernel appends data_buffers from each input batch 
without deduplication. When N build batches share K underlying allocations, the 
concatenated view array holds N × K buffer references. Each subsequent 
CollectLeft join concatenates those bloated arrays again, multiplying the count 
further (1 → 6 → 42 → 252 → 1,512 → 10,584 → 63,504 → 317,520 → 1,587,600 
references per column). The memory metrics code 
(RecordBatchMemoryCounter::count_buffer_memory_size) and the join output (take 
on the build side) then walk every one of those references on each probe batch, 
burning CPU even though there are only 2 distinct allocations.
   
   What changes are included in this PR?
   Two new private functions are added in 
datafusion/physical-plan/src/joins/hash_join/exec.rs:
   
   deduplicate_view_array_buffers<T: ByteViewType>
   
   Walks data_buffers() of a GenericByteViewArray, keying each buffer by its 
raw allocation address (as_ptr() as usize).
   Builds a deduplicated buffer vector and an index remap table.
   Rewrites the 4-byte buffer_index inside each non-inline 128-bit view 
descriptor (length > 12) to point into the deduplicated vector.
   Zero string bytes are copied. Inline values (length ≤ 12) are left 
untouched. If no duplicates exist, the function returns a zero-allocation clone 
of the array.
   deduplicate_record_batch_view_buffers
   
   Applies the deduplication to every Utf8View and BinaryView column in a 
RecordBatch; other columns are forwarded unchanged.
   Returns a cheap clone if the batch contains no view columns.
   The call is inserted in concat_build_batches immediately after 
concat_batches, before the memory accounting adjustment, so the reservation 
correctly reflects the deduplicated size.
   
   What is the testing strategy for this PR?
   A new unit test concat_build_batches_deduplicates_view_buffers is added to 
the existing tests module in exec.rs:
   
   Builds a StringViewArray with long (non-inline) strings that create a data 
buffer.
   Replicates it into 3 RecordBatches that share the same buffer allocation.
   Asserts that raw concat_batches produces 3 buffer references (confirming the 
bug is reproducible).
   Asserts that concat_build_batches deduplicates them down to 1 — preventing 
the compounding that causes the issue.
   The fix is validated by the full existing concat_build_batches_* test suite, 
which covers:
   
   Correctness equivalence with concat_batches
   Memory reservation accounting for multi-batch and single-batch cases
   View-array data not being reserved twice
   Are there any user-facing changes?
   No API changes. The fix is internal to HashJoinExec. Users will observe 
significantly lower peak memory and faster query execution for CollectLeft 
joins on Utf8View / BinaryView columns, particularly in chained-join plans such 
as TPC-DS Q64.


-- 
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