mzabaluev-flarion commented on code in PR #8686:
URL: https://github.com/apache/arrow-rs/pull/8686#discussion_r2454336349


##########
arrow-row/src/lib.rs:
##########
@@ -908,14 +908,18 @@ impl RowConverter {
     /// [RowConverter]. It will panic if any rows are null. Operations on the 
returned [Rows] may
     /// panic if the data is malformed.
     pub fn from_binary(&self, array: BinaryArray) -> Rows {
+        let (offsets, values, nulls) = array.into_parts();
         assert_eq!(
-            array.null_count(),
+            nulls.map(|n| n.null_count()).unwrap_or(0),
             0,
             "can't construct Rows instance from array with nulls"
         );
+        let offsets = offsets.iter().map(|&i| i.as_usize()).collect();
+        // Try zero-copy, if it does not succeed, fall back to copying the 
values.
+        let buffer = values.into_vec().unwrap_or_else(|values| 
values.to_vec());
         Rows {
-            buffer: array.values().to_vec(),
-            offsets: array.offsets().iter().map(|&i| i.as_usize()).collect(),
+            buffer,
+            offsets,

Review Comment:
   > resulting in less heap fragmentation.
   
   This is hypothetical, and there is the new typecast offsets array allocated 
in that same expression. But I think it's generally better to drop allocations 
that end in this scope before allocating new buffers that survive it.



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