[GitHub] [arrow] alamb commented on a change in pull request #9588: ARROW-11799: [Rust] fix len of string and binary arrays created from unbound iterator

[GitBox <gi...@apache.org>]

alamb commented on a change in pull request #9588:
URL: https://github.com/apache/arrow/pull/9588#discussion_r584290701



##########
File path: rust/arrow/src/array/array_binary.rs
##########
@@ -258,6 +258,8 @@ where
             }
         }
 
+        // calculate actual data_len, which may be different from the iterator's upper bound
+        let data_len = offsets.len() - 1;

Review comment:
       I wonder if there is a problem of writing off the end of `null_buf` which [was allocated](https://github.com/apache/arrow/pull/9588/files#diff-0b271702d55fe980ca7906ba50e561f00e610ce3de70ea1b0782e1d6be6f60c3R252) to be only size of `data_len`?

##########
File path: rust/arrow/src/array/array_string.rs
##########
@@ -205,28 +205,30 @@ where
         let (_, data_len) = iter.size_hint();
         let data_len = data_len.expect("Iterator must be sized"); // panic if no upper bound.
 
-        let mut offsets =
-            MutableBuffer::new((data_len + 1) * std::mem::size_of::<OffsetSize>());
+        let offset_size = std::mem::size_of::<OffsetSize>();
+        let mut offsets = MutableBuffer::new((data_len + 1) * offset_size);
         let mut values = MutableBuffer::new(0);
         let mut null_buf = MutableBuffer::new_null(data_len);

Review comment:
       I have the same question about the null buffer's length here as well.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org