[GitHub] [arrow-rs] jhorstmann commented on a diff in pull request #1602: Don't access and validate offset buffer in ListArray::from(ArrayData)

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

jhorstmann commented on code in PR #1602:
URL: https://github.com/apache/arrow-rs/pull/1602#discussion_r854389543


##########
arrow/src/array/array_list.rs:
##########
@@ -524,6 +516,32 @@ mod tests {
         assert_eq!(list_array, another)
     }
 
+    #[test]
+    fn test_empty_list_array() {
+        // Construct an empty value array
+        let value_data = ArrayData::builder(DataType::Int32)
+            .len(0)
+            .add_buffer(Buffer::from_iter(std::iter::empty::<i32>()))

Review Comment:
   Good idea checking the C++ version. The docs for binary layout also mention
   
   > The offsets buffer contains length + 1 signed integers
   > ...
   > and the last slot is the length of the values array
   
   which would mean there has to be a single zero in the offsets buffer for an empty `ListArray`.
   
   If this is a requirements it would be better to validate it when creating `ArrayData`. The code in [`ArrayData::validate_each_offset`](https://github.com/apache/arrow-rs/blob/b4642ecf9cb4a3d8b5741655fdc7d06b71bf41e6/arrow/src/array/data.rs#L1045) explicitly allow this case though:
   
   ```rust
           // An empty binary-like array can have 0 offsets
           if self.len == 0 && offset_buffer.is_empty() {
               return Ok(());
           }
   ```
   
   I'm more hesitant to change this now, maybe let's wait for some more eyes on this.



-- 
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: github-unsubscribe@arrow.apache.org

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