mapleFU commented on code in PR #10287:
URL: https://github.com/apache/arrow-rs/pull/10287#discussion_r3569338832


##########
arrow-array/src/array/byte_view_array.rs:
##########
@@ -556,67 +569,61 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
             let data_blocks = vec![data_block];
             (views_buf, data_blocks)
         } else {
-            // slow path, need to split into multiple buffers
-
-            struct GcCopyGroup {
-                total_buffer_bytes: usize,
-                total_len: usize,
-            }
-
-            impl GcCopyGroup {
-                fn new(total_buffer_bytes: u32, total_len: usize) -> Self {
-                    Self {
-                        total_buffer_bytes: total_buffer_bytes as usize,
-                        total_len,
-                    }
+            // slow path: the non-inline data does not fit in a single buffer
+            // (a buffer offset is a `u32`, so no buffer may exceed 
`i32::MAX`),
+            // so it must be split across several buffers.
+            //
+            // Iterate over *all* views in order — this is essential for
+            // correctness, as gc must preserve the row count. 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 `max_buffer_size`, at which point a new buffer is
+            // started.
+            //
+            // `total_large` is the *sum* of all non-inline bytes — it is what
+            // pushed us onto this path, so it can be many GiB. Size each 
buffer
+            // reservation to the data still to be copied, capped at one 
buffer's
+            // worth (`max_buffer_size`), so we neither reserve the whole
+            // multi-GiB total up front nor pad the final buffer out to a full
+            // `max_buffer_size` when only a little data is left.
+            let mut remaining_large = total_large;
+            let mut views_buf = Vec::with_capacity(len);
+            let mut data_blocks: Vec<Buffer> = Vec::new();
+            let mut data_buf: Vec<u8> = 
Vec::with_capacity(remaining_large.min(max_buffer_size));

Review Comment:
   Yes, visiting one pass is lightweight, so I uses group here



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