Repository: jorgecarleitao/arrow2
Pull request #1159 (2022-07-12 15:20:33)

Replaced `Result` by `panic` in boolean comparison
When two arrays do not have the same number of elements, it is a programmatic error to call a binary operator on them. This is better represented by a panic, since there is no way for a user to recover (it should have checked before). This is aligned with the rest of the binary operators we have.


codecov[bot]: # [Codecov](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1159?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) Report
> Merging [#1159](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1159?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) (cafe1a8) 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 `57.50%`.

```diff
@@            Coverage Diff             @@
##             main    #1159      +/-   ##
==========================================
- Coverage   83.67%   83.66%   -0.02%     
==========================================
  Files         365      365              
  Lines       35865    35863       -2     
==========================================
- Hits        30011    30004       -7     
- Misses       5854     5859       +5     
```


| [Impacted Files](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1159?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) | Coverage Δ | |
|---|---|---|
| [src/compute/boolean\_kleene.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1159/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2NvbXB1dGUvYm9vbGVhbl9rbGVlbmUucnM=) | `68.35% <38.46%> (-6.01%)` | :arrow_down: |
| [src/compute/boolean.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1159/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2NvbXB1dGUvYm9vbGVhbi5ycw==) | `97.22% <87.50%> (+2.62%)` | :arrow_up: |
| [src/compute/comparison/mod.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1159/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2NvbXB1dGUvY29tcGFyaXNvbi9tb2QucnM=) | `54.07% <100.00%> (-0.68%)` | :arrow_down: |
| [src/array/binary/mod.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1159/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2FycmF5L2JpbmFyeS9tb2QucnM=) | `88.88% <0.00%> (-1.24%)` | :arrow_down: |
| [src/array/utf8/mutable.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1159/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2FycmF5L3V0ZjgvbXV0YWJsZS5ycw==) | `90.52% <0.00%> (+0.27%)` | :arrow_up: |
| [src/io/ipc/read/reader.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1159/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.65% <0.00%> (+0.66%)` | :arrow_up: |
| [src/array/utf8/mod.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1159/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/1159?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/1159?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao). Last update [6de9edf...cafe1a8](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1159?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/compute/boolean.rs b/src/compute/boolean.rs
index 39084400f9..46d7266954 100644
--- a/src/compute/boolean.rs
+++ b/src/compute/boolean.rs
@@ -2,21 +2,20 @@
 use crate::array::{Array, BooleanArray};
 use crate::bitmap::{Bitmap, MutableBitmap};
 use crate::datatypes::DataType;
-use crate::error::{Error, Result};
 use crate::scalar::BooleanScalar;
 
 use super::utils::combine_validities;
 
 /// Helper function to implement binary kernels
-fn binary_boolean_kernel<F>(lhs: &BooleanArray, rhs: &BooleanArray, op: F) -> Result<BooleanArray>
+fn binary_boolean_kernel<F>(lhs: &BooleanArray, rhs: &BooleanArray, op: F) -> BooleanArray
 where
     F: Fn(&Bitmap, &Bitmap) -> Bitmap,
 {
-    if lhs.len() != rhs.len() {
-        return Err(Error::InvalidArgumentError(
-            "Cannot perform bitwise operation on arrays of different length".to_string(),
-        ));
-    }
+    assert_eq!(
+        lhs.len(),
+        rhs.len(),
+        "lhs and rhs must have the same length"
+    );
 
     let validity = combine_validities(lhs.validity(), rhs.validity());
 
@@ -25,48 +24,40 @@ where
 
     let values = op(left_buffer, right_buffer);
 
-    Ok(BooleanArray::new(DataType::Boolean, values, validity))
+    BooleanArray::new(DataType::Boolean, values, validity)
 }
 
-/// Performs `AND` operation on two arrays. If either left or right value is null then the
-/// result is also null.
-/// # Error
-/// This function errors when the arrays have different lengths.
-/// # Example
+/// Performs `&&` operation on two [`BooleanArray`], combining the validities.
+/// # Panics
+/// This function panics iff the arrays have different lengths.
+/// # Examples
 /// ```rust
 /// use arrow2::array::BooleanArray;
-/// use arrow2::error::Result;
 /// use arrow2::compute::boolean::and;
-/// # fn main() -> Result<()> {
+///
 /// let a = BooleanArray::from(&[Some(false), Some(true), None]);
 /// let b = BooleanArray::from(&[Some(true), Some(true), Some(false)]);
-/// let and_ab = and(&a, &b)?;
+/// let and_ab = and(&a, &b);
 /// assert_eq!(and_ab, BooleanArray::from(&[Some(false), Some(true), None]));
-/// # Ok(())
-/// # }
 /// ```
-pub fn and(lhs: &BooleanArray, rhs: &BooleanArray) -> Result<BooleanArray> {
+pub fn and(lhs: &BooleanArray, rhs: &BooleanArray) -> BooleanArray {
     binary_boolean_kernel(lhs, rhs, |lhs, rhs| lhs & rhs)
 }
 
-/// Performs `OR` operation on two arrays. If either left or right value is null then the
-/// result is also null.
-/// # Error
-/// This function errors when the arrays have different lengths.
-/// # Example
+/// Performs `||` operation on two [`BooleanArray`], combining the validities.
+/// # Panics
+/// This function panics iff the arrays have different lengths.
+/// # Examples
 /// ```rust
 /// use arrow2::array::BooleanArray;
-/// use arrow2::error::Result;
 /// use arrow2::compute::boolean::or;
-/// # fn main() -> Result<()> {
+///
 /// let a = BooleanArray::from(vec![Some(false), Some(true), None]);
 /// let b = BooleanArray::from(vec![Some(true), Some(true), Some(false)]);
-/// let or_ab = or(&a, &b)?;
+/// let or_ab = or(&a, &b);
 /// assert_eq!(or_ab, BooleanArray::from(vec![Some(true), Some(true), None]));
-/// # Ok(())
-/// # }
 /// ```
-pub fn or(lhs: &BooleanArray, rhs: &BooleanArray) -> Result<BooleanArray> {
+pub fn or(lhs: &BooleanArray, rhs: &BooleanArray) -> BooleanArray {
     binary_boolean_kernel(lhs, rhs, |lhs, rhs| lhs | rhs)
 }
 
@@ -76,11 +67,10 @@ pub fn or(lhs: &BooleanArray, rhs: &BooleanArray) -> Result<BooleanArray> {
 /// ```rust
 /// use arrow2::array::BooleanArray;
 /// use arrow2::compute::boolean::not;
-/// # fn main() {
+///
 /// let a = BooleanArray::from(vec![Some(false), Some(true), None]);
 /// let not_a = not(&a);
 /// assert_eq!(not_a, BooleanArray::from(vec![Some(true), Some(false), None]));
-/// # }
 /// ```
 pub fn not(array: &BooleanArray) -> BooleanArray {
     let values = !array.values();
@@ -88,9 +78,7 @@ pub fn not(array: &BooleanArray) -> BooleanArray {
     BooleanArray::new(DataType::Boolean, values, validity)
 }
 
-/// Returns a non-null [BooleanArray] with whether each value of the array is null.
-/// # Error
-/// This function never errors.
+/// Returns a non-null [`BooleanArray`] with whether each value of the array is null.
 /// # Example
 /// ```rust
 /// use arrow2::array::BooleanArray;
@@ -112,16 +100,15 @@ pub fn is_null(input: &dyn Array) -> BooleanArray {
     BooleanArray::new(DataType::Boolean, values, None)
 }
 
-/// Returns a non-null [BooleanArray] with whether each value of the array is not null.
+/// Returns a non-null [`BooleanArray`] with whether each value of the array is not null.
 /// # Example
 /// ```rust
 /// use arrow2::array::BooleanArray;
 /// use arrow2::compute::boolean::is_not_null;
-/// # fn main() {
+///
 /// let a = BooleanArray::from(&vec![Some(false), Some(true), None]);
 /// let a_is_not_null = is_not_null(&a);
 /// assert_eq!(a_is_not_null, BooleanArray::from_slice(&vec![true, true, false]));
-/// # }
 /// ```
 pub fn is_not_null(input: &dyn Array) -> BooleanArray {
     let values = match input.validity() {
@@ -142,12 +129,12 @@ pub fn is_not_null(input: &dyn Array) -> BooleanArray {
 /// use arrow2::array::BooleanArray;
 /// use arrow2::compute::boolean::and_scalar;
 /// use arrow2::scalar::BooleanScalar;
-/// # fn main() {
+///
 /// let array = BooleanArray::from_slice(&[false, false, true, true]);
 /// let scalar = BooleanScalar::new(Some(true));
 /// let result = and_scalar(&array, &scalar);
 /// assert_eq!(result, BooleanArray::from_slice(&[false, false, true, true]));
-/// # }
+///
 /// ```
 pub fn and_scalar(array: &BooleanArray, scalar: &BooleanScalar) -> BooleanArray {
     match scalar.value() {
@@ -186,7 +173,7 @@ pub fn or_scalar(array: &BooleanArray, scalar: &BooleanScalar) -> BooleanArray {
     }
 }
 
-/// Check if any of the values in the array is `true`
+/// Returns whether any of the values in the array is `true`
 pub fn any(array: &BooleanArray) -> bool {
     if array.is_empty() {
         false
diff --git a/src/compute/boolean_kleene.rs b/src/compute/boolean_kleene.rs
index 7bd412bfce..48d90433ee 100644
--- a/src/compute/boolean_kleene.rs
+++ b/src/compute/boolean_kleene.rs
@@ -1,35 +1,31 @@
 //! Boolean operators of [Kleene logic](https://en.wikipedia.org/wiki/Three-valued_logic#Kleene_and_Priest_logics).
 use crate::datatypes::DataType;
-use crate::error::{Error, Result};
 use crate::scalar::BooleanScalar;
 use crate::{
-    array::BooleanArray,
+    array::{Array, BooleanArray},
     bitmap::{binary, quaternary, ternary, unary, Bitmap, MutableBitmap},
 };
 
 /// Logical 'or' operation on two arrays with [Kleene logic](https://en.wikipedia.org/wiki/Three-valued_logic#Kleene_and_Priest_logics)
-/// # Errors
-/// This function errors if the operands have different lengths.
+/// # Panics
+/// This function panics iff the arrays have a different length
 /// # Example
 ///
 /// ```rust
-/// # use arrow2::error::Result;
 /// use arrow2::array::BooleanArray;
 /// use arrow2::compute::boolean_kleene::or;
-/// # fn main() -> Result<()> {
+///
 /// let a = BooleanArray::from(&[Some(true), Some(false), None]);
 /// let b = BooleanArray::from(&[None, None, None]);
-/// let or_ab = or(&a, &b)?;
+/// let or_ab = or(&a, &b);
 /// assert_eq!(or_ab, BooleanArray::from(&[Some(true), None, None]));
-/// # Ok(())
-/// # }
 /// ```
-pub fn or(lhs: &BooleanArray, rhs: &BooleanArray) -> Result<BooleanArray> {
-    if lhs.len() != rhs.len() {
-        return Err(Error::InvalidArgumentError(
-            "Cannot perform bitwise operation on arrays of different length".to_string(),
-        ));
-    }
+pub fn or(lhs: &BooleanArray, rhs: &BooleanArray) -> BooleanArray {
+    assert_eq!(
+        lhs.len(),
+        rhs.len(),
+        "lhs and rhs must have the same length"
+    );
 
     let lhs_values = lhs.values();
     let rhs_values = rhs.values();
@@ -90,36 +86,29 @@ pub fn or(lhs: &BooleanArray, rhs: &BooleanArray) -> Result<BooleanArray> {
         }
         (None, None) => None,
     };
-    Ok(BooleanArray::new(
-        DataType::Boolean,
-        lhs_values | rhs_values,
-        validity,
-    ))
+    BooleanArray::new(DataType::Boolean, lhs_values | rhs_values, validity)
 }
 
 /// Logical 'and' operation on two arrays with [Kleene logic](https://en.wikipedia.org/wiki/Three-valued_logic#Kleene_and_Priest_logics)
-/// # Errors
-/// This function errors if the operands have different lengths.
+/// # Panics
+/// This function panics iff the arrays have a different length
 /// # Example
 ///
 /// ```rust
-/// # use arrow2::error::Result;
 /// use arrow2::array::BooleanArray;
 /// use arrow2::compute::boolean_kleene::and;
-/// # fn main() -> Result<()> {
+///
 /// let a = BooleanArray::from(&[Some(true), Some(false), None]);
 /// let b = BooleanArray::from(&[None, None, None]);
-/// let and_ab = and(&a, &b)?;
+/// let and_ab = and(&a, &b);
 /// assert_eq!(and_ab, BooleanArray::from(&[None, Some(false), None]));
-/// # Ok(())
-/// # }
 /// ```
-pub fn and(lhs: &BooleanArray, rhs: &BooleanArray) -> Result<BooleanArray> {
-    if lhs.len() != rhs.len() {
-        return Err(Error::InvalidArgumentError(
-            "Cannot perform bitwise operation on arrays of different length".to_string(),
-        ));
-    }
+pub fn and(lhs: &BooleanArray, rhs: &BooleanArray) -> BooleanArray {
+    assert_eq!(
+        lhs.len(),
+        rhs.len(),
+        "lhs and rhs must have the same length"
+    );
 
     let lhs_values = lhs.values();
     let rhs_values = rhs.values();
@@ -179,11 +168,7 @@ pub fn and(lhs: &BooleanArray, rhs: &BooleanArray) -> Result<BooleanArray> {
         }
         (None, None) => None,
     };
-    Ok(BooleanArray::new(
-        DataType::Boolean,
-        lhs_values & rhs_values,
-        validity,
-    ))
+    BooleanArray::new(DataType::Boolean, lhs_values & rhs_values, validity)
 }
 
 /// Logical 'or' operation on an array and a scalar value with [Kleene logic](https://en.wikipedia.org/wiki/Three-valued_logic#Kleene_and_Priest_logics)
@@ -193,12 +178,11 @@ pub fn and(lhs: &BooleanArray, rhs: &BooleanArray) -> Result<BooleanArray> {
 /// use arrow2::array::BooleanArray;
 /// use arrow2::scalar::BooleanScalar;
 /// use arrow2::compute::boolean_kleene::or_scalar;
-/// # fn main() {
+///
 /// let array = BooleanArray::from(&[Some(true), Some(false), None]);
 /// let scalar = BooleanScalar::new(Some(false));
 /// let result = or_scalar(&array, &scalar);
 /// assert_eq!(result, BooleanArray::from(&[Some(true), Some(false), None]));
-/// # }
 /// ```
 pub fn or_scalar(array: &BooleanArray, scalar: &BooleanScalar) -> BooleanArray {
     match scalar.value() {
@@ -226,12 +210,11 @@ pub fn or_scalar(array: &BooleanArray, scalar: &BooleanScalar) -> BooleanArray {
 /// use arrow2::array::BooleanArray;
 /// use arrow2::scalar::BooleanScalar;
 /// use arrow2::compute::boolean_kleene::and_scalar;
-/// # fn main() {
+///
 /// let array = BooleanArray::from(&[Some(true), Some(false), None]);
 /// let scalar = BooleanScalar::new(None);
 /// let result = and_scalar(&array, &scalar);
 /// assert_eq!(result, BooleanArray::from(&[None, Some(false), None]));
-/// # }
 /// ```
 pub fn and_scalar(array: &BooleanArray, scalar: &BooleanScalar) -> BooleanArray {
     match scalar.value() {
@@ -250,3 +233,25 @@ pub fn and_scalar(array: &BooleanArray, scalar: &BooleanScalar) -> BooleanArray
         }
     }
 }
+
+/// Returns whether any of the values in the array is `true`
+pub fn any(array: &BooleanArray) -> bool {
+    if array.is_empty() {
+        false
+    } else if array.validity().is_some() {
+        array.into_iter().any(|v| v == Some(true))
+    } else {
+        let vals = array.values();
+        vals.unset_bits() != vals.len()
+    }
+}
+
+/// Returns whether all values in the array are `true`
+pub fn all(array: &BooleanArray) -> bool {
+    if array.is_empty() || array.null_count() > 0 {
+        false
+    } else {
+        let vals = array.values();
+        vals.unset_bits() == 0
+    }
+}
diff --git a/src/compute/comparison/mod.rs b/src/compute/comparison/mod.rs
index 8f67856b90..4ad7920705 100644
--- a/src/compute/comparison/mod.rs
+++ b/src/compute/comparison/mod.rs
@@ -515,21 +515,20 @@ fn finish_eq_validities(
         (Some(lhs), None) => compute::boolean::and(
             &BooleanArray::new(DataType::Boolean, lhs, None),
             &output_without_validities,
-        )
-        .unwrap(),
+        ),
         (None, Some(rhs)) => compute::boolean::and(
             &output_without_validities,
             &BooleanArray::new(DataType::Boolean, rhs, None),
-        )
-        .unwrap(),
+        ),
         (Some(lhs), Some(rhs)) => {
             let lhs = BooleanArray::new(DataType::Boolean, lhs, None);
             let rhs = BooleanArray::new(DataType::Boolean, rhs, None);
             let eq_validities = compute::comparison::boolean::eq(&lhs, &rhs);
-            compute::boolean::and(&output_without_validities, &eq_validities).unwrap()
+            compute::boolean::and(&output_without_validities, &eq_validities)
         }
     }
 }
+
 fn finish_neq_validities(
     output_without_validities: BooleanArray,
     validity_lhs: Option<Bitmap>,
@@ -540,18 +539,18 @@ fn finish_neq_validities(
         (Some(lhs), None) => {
             let lhs_negated =
                 compute::boolean::not(&BooleanArray::new(DataType::Boolean, lhs, None));
-            compute::boolean::or(&lhs_negated, &output_without_validities).unwrap()
+            compute::boolean::or(&lhs_negated, &output_without_validities)
         }
         (None, Some(rhs)) => {
             let rhs_negated =
                 compute::boolean::not(&BooleanArray::new(DataType::Boolean, rhs, None));
-            compute::boolean::or(&output_without_validities, &rhs_negated).unwrap()
+            compute::boolean::or(&output_without_validities, &rhs_negated)
         }
         (Some(lhs), Some(rhs)) => {
             let lhs = BooleanArray::new(DataType::Boolean, lhs, None);
             let rhs = BooleanArray::new(DataType::Boolean, rhs, None);
             let neq_validities = compute::comparison::boolean::neq(&lhs, &rhs);
-            compute::boolean::or(&output_without_validities, &neq_validities).unwrap()
+            compute::boolean::or(&output_without_validities, &neq_validities)
         }
     }
 }
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/compute/boolean.rs b/tests/it/compute/boolean.rs
index fb0d1c6da3..37174f224a 100644
--- a/tests/it/compute/boolean.rs
+++ b/tests/it/compute/boolean.rs
@@ -7,7 +7,7 @@ use std::iter::FromIterator;
 fn array_and() {
     let a = BooleanArray::from_slice(vec![false, false, true, true]);
     let b = BooleanArray::from_slice(vec![false, true, false, true]);
-    let c = and(&a, &b).unwrap();
+    let c = and(&a, &b);
 
     let expected = BooleanArray::from_slice(vec![false, false, false, true]);
 
@@ -18,7 +18,7 @@ fn array_and() {
 fn array_or() {
     let a = BooleanArray::from_slice(vec![false, false, true, true]);
     let b = BooleanArray::from_slice(vec![false, true, false, true]);
-    let c = or(&a, &b).unwrap();
+    let c = or(&a, &b);
 
     let expected = BooleanArray::from_slice(vec![false, true, true, true]);
 
@@ -49,7 +49,7 @@ fn array_or_validity() {
         Some(false),
         Some(true),
     ]);
-    let c = or(&a, &b).unwrap();
+    let c = or(&a, &b);
 
     let expected = BooleanArray::from(vec![
         None,
@@ -100,7 +100,7 @@ fn array_and_validity() {
         Some(false),
         Some(true),
     ]);
-    let c = and(&a, &b).unwrap();
+    let c = and(&a, &b);
 
     let expected = BooleanArray::from(vec![
         None,
@@ -128,7 +128,7 @@ fn array_and_sliced_same_offset() {
 
     let a = a.slice(8, 4);
     let b = b.slice(8, 4);
-    let c = and(&a, &b).unwrap();
+    let c = and(&a, &b);
 
     let expected = BooleanArray::from_slice(vec![false, false, false, true]);
 
@@ -147,7 +147,7 @@ fn array_and_sliced_same_offset_mod8() {
     let a = a.slice(0, 4);
     let b = b.slice(8, 4);
 
-    let c = and(&a, &b).unwrap();
+    let c = and(&a, &b);
 
     let expected = BooleanArray::from_slice(vec![false, false, false, true]);
 
@@ -163,7 +163,7 @@ fn array_and_sliced_offset1() {
 
     let a = a.slice(8, 4);
 
-    let c = and(&a, &b).unwrap();
+    let c = and(&a, &b);
 
     let expected = BooleanArray::from_slice(vec![false, false, false, true]);
 
@@ -179,7 +179,7 @@ fn array_and_sliced_offset2() {
 
     let b = b.slice(8, 4);
 
-    let c = and(&a, &b).unwrap();
+    let c = and(&a, &b);
 
     let expected = BooleanArray::from_slice(vec![false, false, false, true]);
 
@@ -204,7 +204,7 @@ fn array_and_validity_offset() {
     let b = b.slice(2, 4);
     let b = b.as_any().downcast_ref::<BooleanArray>().unwrap();
 
-    let c = and(a, b).unwrap();
+    let c = and(a, b);
 
     let expected = BooleanArray::from(vec![Some(false), Some(false), None, Some(true)]);
 
diff --git a/tests/it/compute/boolean_kleene.rs b/tests/it/compute/boolean_kleene.rs
index 3bf0b8ef37..c1c034ca9f 100644
--- a/tests/it/compute/boolean_kleene.rs
+++ b/tests/it/compute/boolean_kleene.rs
@@ -26,7 +26,7 @@ fn and_generic() {
         Some(false),
         Some(true),
     ]);
-    let c = and(&lhs, &rhs).unwrap();
+    let c = and(&lhs, &rhs);
 
     let expected = BooleanArray::from(&[
         None,
@@ -67,7 +67,7 @@ fn or_generic() {
         Some(false),
         Some(true),
     ]);
-    let c = or(&a, &b).unwrap();
+    let c = or(&a, &b);
 
     let expected = BooleanArray::from(&[
         None,
@@ -90,7 +90,7 @@ fn or_right_nulls() {
 
     let b = BooleanArray::from(&[Some(true), Some(false), None, Some(true), Some(false), None]);
 
-    let c = or(&a, &b).unwrap();
+    let c = or(&a, &b);
 
     let expected = BooleanArray::from(&[
         Some(true),
@@ -117,7 +117,7 @@ fn or_left_nulls() {
 
     let b = BooleanArray::from_slice(&[false, false, false, true, true, true]);
 
-    let c = or(&a, &b).unwrap();
+    let c = or(&a, &b);
 
     let expected = BooleanArray::from(vec![
         Some(true),
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())
+}