adriangb opened a new pull request, #10287:
URL: https://github.com/apache/arrow-rs/pull/10287

   # Which issue does this PR close?
   
   <!-- We generally require a GitHub issue to be filed for all bug fixes and 
enhancements -->
   N/A — filing alongside; happy to open a tracking issue if preferred.
   
   # Rationale for this change
   
   `GenericByteViewArray::gc()` takes a multi-buffer **slow path** once a 
`Utf8View`/`BinaryView` column references more than `i32::MAX` (~2.1 GiB) of 
non-inline data. Because a single Arrow data buffer is addressed by a `u32` 
offset, the compacted output has to be split across multiple buffers there.
   
   That slow path built its copy groups by counting **only non-inline views** 
(`current_elements`), then copied a *contiguous index range* of that size. As a 
result:
   
   - every **inline** (short) view was dropped, so the output array was 
**shorter than its input**, and
   - the index-range / byte-budget mismatch could also let a single output 
buffer grow past `i32::MAX`.
   
   `gc()` is documented as a pure size optimisation and must never change the 
row count. Downstream code that rebuilds a `RecordBatch` from the GC'd columns 
therefore panicked on the "all columns must have the same row count" invariant 
whenever a view column crossed the ~2.1 GiB threshold.
   
   The existing `test_gc_huge_array` missed this because all of its views are 
non-inline, so the miscount happened to equal `len`.
   
   # What changes are included in this PR?
   
   - Rewrite the slow path as a single pass over **all** views: inline views 
are copied unchanged (they reference no buffer); non-inline views are packed 
into the current output buffer until adding the next one would exceed the max 
buffer size, at which point a new buffer is sealed. This preserves the row 
count and value order, and keeps each output buffer within the size limit.
   - Extract the body into a private 
`gc_with_max_buffer_size(max_buffer_size)`; `gc()` calls it with `i32::MAX`. 
This lets the split path be tested with a tiny threshold instead of allocating 
2+ GiB.
   - Add `test_gc_slow_path_preserves_inline_views`, which interleaves inline, 
large, and null views and drives the split via a small `max_buffer_size`, 
asserting the row count, per-value equality, buffer-size cap, and that the 
split actually occurred.
   
   # Are there any user-facing changes?
   
   No. Public API is unchanged; this is a correctness fix to `gc()`.


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

Reply via email to