Repository: jorgecarleitao/arrow2
Pull request #1160 (2022-07-12 15:26:47)

Minor cleanup of internal namings

codecov[bot]: # [Codecov](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1160?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) Report
> Merging [#1160](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1160?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) (bddd935) into [main](https://codecov.io/gh/jorgecarleitao/arrow2/commit/6de9edf5e131a71e34f5ec10f0b304a68b27fdf9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) (6de9edf) will **decrease** coverage by `0.01%`.
> The diff coverage is `84.61%`.

```diff
@@            Coverage Diff             @@
##             main    #1160      +/-   ##
==========================================
- Coverage   83.67%   83.66%   -0.02%     
==========================================
  Files         365      365              
  Lines       35865    35861       -4     
==========================================
- Hits        30011    30004       -7     
- Misses       5854     5857       +3     
```


| [Impacted Files](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1160?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) | Coverage Δ | |
|---|---|---|
| [src/io/flight/mod.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1160/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2lvL2ZsaWdodC9tb2QucnM=) | `67.00% <80.00%> (-0.33%)` | :arrow_down: |
| [src/io/ipc/read/schema.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1160/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2lvL2lwYy9yZWFkL3NjaGVtYS5ycw==) | `95.49% <83.33%> (+0.92%)` | :arrow_up: |
| [src/io/ipc/read/common.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1160/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2lvL2lwYy9yZWFkL2NvbW1vbi5ycw==) | `94.90% <100.00%> (-0.02%)` | :arrow_down: |
| [src/io/ipc/read/array/boolean.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1160/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2lvL2lwYy9yZWFkL2FycmF5L2Jvb2xlYW4ucnM=) | `90.74% <0.00%> (-7.41%)` | :arrow_down: |
| [src/io/ipc/read/stream\_async.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1160/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2lvL2lwYy9yZWFkL3N0cmVhbV9hc3luYy5ycw==) | `74.65% <0.00%> (-2.06%)` | :arrow_down: |
| [src/io/ipc/read/file\_async.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1160/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2lvL2lwYy9yZWFkL2ZpbGVfYXN5bmMucnM=) | `60.44% <0.00%> (-0.75%)` | :arrow_down: |
| [src/bitmap/immutable.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1160/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2JpdG1hcC9pbW11dGFibGUucnM=) | `83.95% <0.00%> (-0.62%)` | :arrow_down: |
| [src/io/ipc/read/reader.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1160/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2lvL2lwYy9yZWFkL3JlYWRlci5ycw==) | `96.32% <0.00%> (+0.33%)` | :arrow_up: |
| [src/array/utf8/mod.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1160/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2FycmF5L3V0ZjgvbW9kLnJz) | `86.58% <0.00%> (+0.95%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1160?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1160?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao). Last update [6de9edf...bddd935](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1160?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao).


Patch:
diff --git a/src/ffi/array.rs b/src/ffi/array.rs
index 099424c6f9..46858deff7 100644
--- a/src/ffi/array.rs
+++ b/src/ffi/array.rs
@@ -1,5 +1,4 @@
 //! Contains functionality to load an ArrayData from the C Data Interface
-use std::ptr::NonNull;
 use std::sync::Arc;
 
 use crate::{
@@ -172,67 +171,94 @@ impl ArrowArray {
     }
 }
 
-/// interprets the buffer `index` as a [`Buffer`].
 /// # Safety
-/// The caller must guarantee that the buffer `index` corresponds to a buffer of type `T`.
-/// This function assumes that the buffer created from FFI is valid; this is impossible to prove.
-unsafe fn create_buffer<T: NativeType>(
+/// The caller must ensure that the buffer at index `i` is not mutably shared.
+unsafe fn get_buffer_ptr<T: NativeType>(
     array: &ArrowArray,
     data_type: &DataType,
-    owner: InternalArrowArray,
     index: usize,
-) -> Result<Buffer<T>> {
+) -> Result<*mut T> {
     if array.buffers.is_null() {
-        return Err(Error::OutOfSpec("The array buffers are null".to_string()));
+        return Err(Error::oos(format!(
+            "An ArrowArray of type {data_type:?} must have non-null buffers"
+        )));
     }
 
+    if array
+        .buffers
+        .align_offset(std::mem::align_of::<*mut *const u8>())
+        != 0
+    {
+        return Err(Error::oos(format!(
+            "An ArrowArray of type {data_type:?}
+            must have buffer {index} aligned to type {}",
+            std::any::type_name::<*mut *const u8>()
+        )));
+    }
     let buffers = array.buffers as *mut *const u8;
 
-    assert!(index < array.n_buffers as usize);
+    if index >= array.n_buffers as usize {
+        return Err(Error::oos(format!(
+            "An ArrowArray of type {data_type:?} 
+             must have buffer {index}."
+        )));
+    }
+
     let ptr = *buffers.add(index);
-    let ptr = NonNull::new(ptr as *mut T);
+    if ptr.is_null() {
+        return Err(Error::oos(format!(
+            "An array of type {data_type:?}
+            must have a non-null buffer {index}"
+        )));
+    }
+    if ptr.align_offset(std::mem::align_of::<T>()) != 0 {
+        return Err(Error::oos(format!(
+            "An ArrowArray of type {data_type:?}
+            must have buffer {index} aligned to type {}",
+            std::any::type_name::<T>()
+        )));
+    }
+    // note: we can't prove that this pointer is not mutably shared - part of the safety invariant
+    Ok(ptr as *mut T)
+}
+
+/// returns the buffer `i` of `array` interpreted as a [`Buffer`].
+/// # Safety
+/// This function is safe iff:
+/// * the buffers up to position `index` are valid for the declared length
+/// * the buffers' pointers are not mutably shared for the lifetime of `owner`
+unsafe fn create_buffer<T: NativeType>(
+    array: &ArrowArray,
+    data_type: &DataType,
+    owner: InternalArrowArray,
+    index: usize,
+) -> Result<Buffer<T>> {
+    let ptr = get_buffer_ptr(array, data_type, index)?;
 
     let len = buffer_len(array, data_type, index)?;
     let offset = buffer_offset(array, data_type, index);
-    let bytes = ptr
-        .map(|ptr| Bytes::from_foreign(ptr.as_ptr(), len, owner))
-        .ok_or_else(|| Error::OutOfSpec(format!("The buffer at position {} is null", index)))?;
+    let bytes = Bytes::from_foreign(ptr, len, owner);
 
     Ok(Buffer::from_bytes(bytes).slice(offset, len - offset))
 }
 
-/// returns a new buffer corresponding to the index `i` of the FFI array. It may not exist (null pointer).
-/// `bits` is the number of bits that the native type of this buffer has.
-/// The size of the buffer will be `ceil(self.length * bits, 8)`.
-/// # Panic
-/// This function panics if `i` is larger or equal to `n_buffers`.
+/// returns the buffer `i` of `array` interpreted as a [`Bitmap`].
 /// # Safety
-/// This function assumes that `ceil(self.length * bits, 8)` is the size of the buffer
+/// This function is safe iff:
+/// * the buffer at position `index` is valid for the declared length
+/// * the buffers' pointer is not mutable for the lifetime of `owner`
 unsafe fn create_bitmap(
     array: &ArrowArray,
+    data_type: &DataType,
     owner: InternalArrowArray,
     index: usize,
 ) -> Result<Bitmap> {
-    if array.buffers.is_null() {
-        return Err(Error::OutOfSpec("The array buffers are null".to_string()));
-    }
-    let len = array.length as usize;
-    let offset = array.offset as usize;
-    let buffers = array.buffers as *mut *const u8;
-
-    assert!(index < array.n_buffers as usize);
-    let ptr = *buffers.add(index);
+    let ptr = get_buffer_ptr(array, data_type, index)?;
 
+    let len: usize = array.length.try_into().expect("length to fit in `usize`");
+    let offset: usize = array.offset.try_into().expect("Offset to fit in `usize`");
     let bytes_len = bytes_for(offset + len);
-    let ptr = NonNull::new(ptr as *mut u8);
-    let bytes = ptr
-        .map(|ptr| Bytes::from_foreign(ptr.as_ptr(), bytes_len, owner))
-        .ok_or_else(|| {
-            Error::OutOfSpec(format!(
-                "The buffer {} is a null pointer and cannot be interpreted as a bitmap",
-                index
-            ))
-        })?;
+    let bytes = Bytes::from_foreign(ptr, bytes_len, owner);
 
     Ok(Bitmap::from_bytes(bytes, offset + len).slice(offset, len))
 }
@@ -243,20 +269,18 @@ fn buffer_offset(array: &ArrowArray, data_type: &DataType, i: usize) -> usize {
         (LargeUtf8, 2) | (LargeBinary, 2) | (Utf8, 2) | (Binary, 2) => 0,
         (FixedSizeBinary, 1) => {
             if let DataType::FixedSizeBinary(size) = data_type.to_logical_type() {
-                (array.offset as usize) * *size
+                let offset: usize = array.offset.try_into().expect("Offset to fit in `usize`");
+                offset * *size
             } else {
                 unreachable!()
             }
         }
-        _ => array.offset as usize,
+        _ => array.offset.try_into().expect("Offset to fit in `usize`"),
     }
 }
 
 /// Returns the length, in slots, of the buffer `i` (indexed according to the C data interface)
-// Rust implementation uses fixed-sized buffers, which require knowledge of their `len`.
-// for variable-sized buffers, such as the second buffer of a stringArray, we need
-// to fetch offset buffer's len to build the second buffer.
-fn buffer_len(array: &ArrowArray, data_type: &DataType, i: usize) -> Result<usize> {
+unsafe fn buffer_len(array: &ArrowArray, data_type: &DataType, i: usize) -> Result<usize> {
     Ok(match (data_type.to_physical_type(), i) {
         (PhysicalType::FixedSizeBinary, 1) => {
             if let DataType::FixedSizeBinary(size) = data_type.to_logical_type() {
@@ -308,32 +332,70 @@ fn buffer_len(array: &ArrowArray, data_type: &DataType, i: usize) -> Result<usiz
     })
 }
 
-fn create_child(
+/// Safety
+/// This function is safe iff:
+/// * `array.children` at `index` is valid
+/// * `array.children` is not mutably shared for the lifetime of `parent`
+/// * the pointer of `array.children` at `index` is valid
+/// * the pointer of `array.children` at `index` is not mutably shared for the lifetime of `parent`
+unsafe fn create_child(
     array: &ArrowArray,
-    field: &DataType,
+    data_type: &DataType,
     parent: InternalArrowArray,
     index: usize,
 ) -> Result<ArrowArrayChild<'static>> {
-    let data_type = get_child(field, index)?;
-    assert!(index < array.n_children as usize);
-    assert!(!array.children.is_null());
-    unsafe {
-        let arr_ptr = *array.children.add(index);
-        assert!(!arr_ptr.is_null());
-        let arr_ptr = &*arr_ptr;
+    let data_type = get_child(data_type, index)?;
+
+    // catch what we can
+    if array.children.is_null() {
+        return Err(Error::oos(format!(
+            "An ArrowArray of type {data_type:?} must have non-null children"
+        )));
+    }
+
+    if index >= array.n_children as usize {
+        return Err(Error::oos(format!(
+            "An ArrowArray of type {data_type:?} 
+             must have child {index}."
+        )));
+    }
+
+    // Safety - part of the invariant
+    let arr_ptr = unsafe { *array.children.add(index) };
 
-        Ok(ArrowArrayChild::new(arr_ptr, data_type, parent))
+    // catch what we can
+    if arr_ptr.is_null() {
+        return Err(Error::oos(format!(
+            "An array of type {data_type:?}
+            must have a non-null child {index}"
+        )));
     }
+
+    // Safety - invariant of this function
+    let arr_ptr = unsafe { &*arr_ptr };
+    Ok(ArrowArrayChild::new(arr_ptr, data_type, parent))
 }
 
-fn create_dictionary(
+/// Safety
+/// This function is safe iff:
+/// * `array.dictionary` is valid
+/// * `array.dictionary` is not mutably shared for the lifetime of `parent`
+unsafe fn create_dictionary(
     array: &ArrowArray,
     data_type: &DataType,
     parent: InternalArrowArray,
 ) -> Result<Option<ArrowArrayChild<'static>>> {
     if let DataType::Dictionary(_, values, _) = data_type {
         let data_type = values.as_ref().clone();
-        assert!(!array.dictionary.is_null());
+        // catch what we can
+        if array.dictionary.is_null() {
+            return Err(Error::oos(format!(
+                "An array of type {data_type:?}
+                must have a non-null dictionary"
+            )));
+        }
+
+        // safety: part of the invariant
         let array = unsafe { &*array.dictionary };
         Ok(Some(ArrowArrayChild::new(array, data_type, parent)))
     } else {
@@ -356,31 +418,35 @@ pub trait ArrowArrayRef: std::fmt::Debug {
         if self.array().null_count() == 0 {
             Ok(None)
         } else {
-            create_bitmap(self.array(), self.owner(), 0).map(Some)
+            create_bitmap(self.array(), self.data_type(), self.owner(), 0).map(Some)
         }
     }
 
     /// # Safety
-    /// The caller must guarantee that the buffer `index` corresponds to a bitmap.
-    /// This function assumes that the bitmap created from FFI is valid; this is impossible to prove.
+    /// The caller must guarantee that the buffer `index` corresponds to a buffer.
+    /// This function assumes that the buffer created from FFI is valid; this is impossible to prove.
     unsafe fn buffer<T: NativeType>(&self, index: usize) -> Result<Buffer<T>> {
         create_buffer::<T>(self.array(), self.data_type(), self.owner(), index)
     }
 
     /// # Safety
-    /// The caller must guarantee that the buffer `index` corresponds to a bitmap.
-    /// This function assumes that the bitmap created from FFI is valid; this is impossible to prove.
+    /// This function is safe iff:
+    /// * the buffer at position `index` is valid for the declared length
+    /// * the buffers' pointer is not mutable for the lifetime of `owner`
     unsafe fn bitmap(&self, index: usize) -> Result<Bitmap> {
-        create_bitmap(self.array(), self.owner(), index)
+        create_bitmap(self.array(), self.data_type(), self.owner(), index)
     }
 
     /// # Safety
-    /// The caller must guarantee that the child `index` is valid per c data interface.
+    /// * `array.children` at `index` is valid
+    /// * `array.children` is not mutably shared for the lifetime of `parent`
+    /// * the pointer of `array.children` at `index` is valid
+    /// * the pointer of `array.children` at `index` is not mutably shared for the lifetime of `parent`
     unsafe fn child(&self, index: usize) -> Result<ArrowArrayChild> {
         create_child(self.array(), self.data_type(), self.parent().clone(), index)
     }
 
-    fn dictionary(&self) -> Result<Option<ArrowArrayChild>> {
+    unsafe fn dictionary(&self) -> Result<Option<ArrowArrayChild>> {
         create_dictionary(self.array(), self.data_type(), self.parent().clone())
     }
 
diff --git a/src/io/flight/mod.rs b/src/io/flight/mod.rs
index 59ba0a83d8..4072c0d0d0 100644
--- a/src/io/flight/mod.rs
+++ b/src/io/flight/mod.rs
@@ -24,11 +24,11 @@ pub use crate::io::ipc::write::common::WriteOptions;
 /// # Errors
 /// This function errors iff `fields` is not consistent with `columns`
 pub fn serialize_batch(
-    columns: &Chunk<Box<dyn Array>>,
+    chunk: &Chunk<Box<dyn Array>>,
     fields: &[IpcField],
     options: &WriteOptions,
 ) -> Result<(Vec<FlightData>, FlightData)> {
-    if fields.len() != columns.arrays().len() {
+    if fields.len() != chunk.arrays().len() {
         return Err(Error::InvalidArgumentError("The argument `fields` must be consistent with the columns' schema. Use e.g. &arrow2::io::flight::default_ipc_fields(&schema.fields)".to_string()));
     }
 
@@ -38,7 +38,7 @@ pub fn serialize_batch(
     };
 
     let (encoded_dictionaries, encoded_batch) =
-        encode_chunk(columns, fields, &mut dictionary_tracker, options)
+        encode_chunk(chunk, fields, &mut dictionary_tracker, options)
             .expect("DictionaryTracker configured above to not error on replacement");
 
     let flight_dictionaries = encoded_dictionaries.into_iter().map(Into::into).collect();
@@ -63,15 +63,14 @@ pub fn serialize_schema_to_result(
     ipc_fields: Option<&[IpcField]>,
 ) -> SchemaResult {
     SchemaResult {
-        schema: schema_as_flatbuffer(schema, ipc_fields),
+        schema: _serialize_schema(schema, ipc_fields),
     }
 }
 
 /// Serializes a [`Schema`] to [`FlightData`].
 pub fn serialize_schema(schema: &Schema, ipc_fields: Option<&[IpcField]>) -> FlightData {
-    let data_header = schema_as_flatbuffer(schema, ipc_fields);
     FlightData {
-        data_header,
+        data_header: _serialize_schema(schema, ipc_fields),
         ..Default::default()
     }
 }
@@ -93,7 +92,7 @@ pub fn serialize_schema_to_info(
     Ok(schema)
 }
 
-fn schema_as_flatbuffer(schema: &Schema, ipc_fields: Option<&[IpcField]>) -> Vec<u8> {
+fn _serialize_schema(schema: &Schema, ipc_fields: Option<&[IpcField]>) -> Vec<u8> {
     if let Some(ipc_fields) = ipc_fields {
         write::schema_to_bytes(schema, ipc_fields)
     } else {
diff --git a/src/io/ipc/read/common.rs b/src/io/ipc/read/common.rs
index 5638a5289e..306df0061c 100644
--- a/src/io/ipc/read/common.rs
+++ b/src/io/ipc/read/common.rs
@@ -226,8 +226,8 @@ fn first_dict_field<'a>(
     Err(Error::from(OutOfSpecKind::InvalidId { requested_id: id }))
 }
 
-/// Read the dictionary from the buffer and provided metadata,
-/// updating the `dictionaries` with the resulting dictionary
+/// Reads a dictionary from the reader,
+/// updating `dictionaries` with the resulting dictionary
 #[allow(clippy::too_many_arguments)]
 pub fn read_dictionary<R: Read + Seek>(
     batch: arrow_format::ipc::DictionaryBatchRef,
@@ -269,7 +269,7 @@ pub fn read_dictionary<R: Read + Seek>(
                 fields: vec![first_ipc_field.clone()],
                 is_little_endian: ipc_schema.is_little_endian,
             };
-            let columns = read_record_batch(
+            let chunk = read_record_batch(
                 batch,
                 &fields,
                 &ipc_schema,
@@ -282,8 +282,7 @@ pub fn read_dictionary<R: Read + Seek>(
                 file_size,
                 scratch,
             )?;
-            let mut arrays = columns.into_arrays();
-            arrays.pop().unwrap()
+            chunk.into_arrays().pop().unwrap()
         }
         _ => {
             return Err(Error::from(OutOfSpecKind::InvalidIdDataType {
diff --git a/src/io/ipc/read/schema.rs b/src/io/ipc/read/schema.rs
index b54292baab..7f970786aa 100644
--- a/src/io/ipc/read/schema.rs
+++ b/src/io/ipc/read/schema.rs
@@ -328,17 +328,16 @@ fn get_data_type(
 }
 
 /// Deserialize an flatbuffers-encoded Schema message into [`Schema`] and [`IpcSchema`].
-pub fn deserialize_schema(bytes: &[u8]) -> Result<(Schema, IpcSchema)> {
-    let message = arrow_format::ipc::MessageRef::read_as_root(bytes)
+pub fn deserialize_schema(message: &[u8]) -> Result<(Schema, IpcSchema)> {
+    let message = arrow_format::ipc::MessageRef::read_as_root(message)
         .map_err(|err| Error::oos(format!("Unable deserialize message: {:?}", err)))?;
 
-    let schema = match message.header()?.ok_or_else(|| {
-        Error::oos("Unable to convert flight data header to a record batch".to_string())
-    })? {
+    let schema = match message
+        .header()?
+        .ok_or_else(|| Error::oos("Unable to convert header to a schema".to_string()))?
+    {
         arrow_format::ipc::MessageHeaderRef::Schema(schema) => Ok(schema),
-        _ => Err(Error::nyi(
-            "flight currently only supports reading RecordBatch messages",
-        )),
+        _ => Err(Error::nyi("The message is expected to be a Schema message")),
     }?;
 
     fb_to_schema(schema)


Test patch:
diff --git a/tests/it/io/parquet/mod.rs b/tests/it/io/parquet/mod.rs
index 6710bf5769..346208e895 100644
--- a/tests/it/io/parquet/mod.rs
+++ b/tests/it/io/parquet/mod.rs
@@ -1394,3 +1394,38 @@ fn large_list_large_binary() -> Result<()> {
     array.try_extend(data).unwrap();
     list_array_generic(false, array.into())
 }
+
+#[test]
+fn list_utf8_nullable() -> Result<()> {
+    let data = vec![
+        Some(vec![Some("a".to_string())]),
+        None,
+        Some(vec![None, Some("b".to_string())]),
+        Some(vec![]),
+        Some(vec![Some("c".to_string())]),
+        None,
+    ];
+    let mut array =
+        MutableListArray::<i32, _>::new_with_field(MutableUtf8Array::<i32>::new(), "item", true);
+    array.try_extend(data).unwrap();
+    list_array_generic(true, array.into())
+}
+
+#[test]
+fn list_int_nullable() -> Result<()> {
+    let data = vec![
+        Some(vec![Some(1)]),
+        None,
+        Some(vec![None, Some(2)]),
+        Some(vec![]),
+        Some(vec![Some(3)]),
+        None,
+    ];
+    let mut array = MutableListArray::<i32, _>::new_with_field(
+        MutablePrimitiveArray::<i32>::new(),
+        "item",
+        true,
+    );
+    array.try_extend(data).unwrap();
+    list_array_generic(true, array.into())
+}