[arrow-rs] branch master updated: Skip null buffer when importing FFI ArrowArray struct if no null buffer in the spec (#3293)

[tu...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

tustvold pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git


The following commit(s) were added to refs/heads/master by this push:
     new 7d2139749 Skip null buffer when importing FFI ArrowArray struct if no null buffer in the spec (#3293)
7d2139749 is described below

commit 7d2139749029f78b1d88eddf24be664071e12686
Author: Liang-Chi Hsieh <vi...@gmail.com>
AuthorDate: Thu Dec 8 08:50:13 2022 -0800

    Skip null buffer when importing FFI ArrowArray struct if no null buffer in the spec (#3293)
    
    * Fix null buffer import/export behavior
    
    * Clippy
    
    Co-authored-by: Raphael Taylor-Davies <r....@googlemail.com>
---
 arrow/src/ffi.rs | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/arrow/src/ffi.rs b/arrow/src/ffi.rs
index 0c1c1fa54..abb53dff6 100644
--- a/arrow/src/ffi.rs
+++ b/arrow/src/ffi.rs
@@ -462,12 +462,18 @@ impl FFI_ArrowArray {
     /// This method releases `buffers`. Consumers of this struct *must* call `release` before
     /// releasing this struct, or contents in `buffers` leak.
     pub fn new(data: &ArrayData) -> Self {
-        // * insert the null buffer at the start
-        // * make all others `Option<Buffer>`.
-        let buffers = iter::once(data.null_buffer().cloned())
-            .chain(data.buffers().iter().map(|b| Some(b.clone())))
-            .collect::<Vec<_>>();
         let data_layout = layout(data.data_type());
+
+        let buffers = if data_layout.can_contain_null_mask {
+            // * insert the null buffer at the start
+            // * make all others `Option<Buffer>`.
+            iter::once(data.null_buffer().cloned())
+                .chain(data.buffers().iter().map(|b| Some(b.clone())))
+                .collect::<Vec<_>>()
+        } else {
+            data.buffers().iter().map(|b| Some(b.clone())).collect()
+        };
+
         // `n_buffers` is the number of buffers by the spec.
         let n_buffers = {
             data_layout.buffers.len() + {
@@ -616,8 +622,15 @@ pub trait ArrowArrayRef {
         let len = self.array().len();
         let offset = self.array().offset();
         let null_count = self.array().null_count();
-        let buffers = self.buffers()?;
-        let null_bit_buffer = self.null_bit_buffer();
+
+        let data_layout = layout(&data_type);
+        let buffers = self.buffers(data_layout.can_contain_null_mask)?;
+
+        let null_bit_buffer = if data_layout.can_contain_null_mask {
+            self.null_bit_buffer()
+        } else {
+            None
+        };
 
         let mut child_data: Vec<ArrayData> = (0..self.array().n_children as usize)
             .map(|i| {
@@ -649,11 +662,12 @@ pub trait ArrowArrayRef {
     }
 
     /// returns all buffers, as organized by Rust (i.e. null buffer is skipped)
-    fn buffers(&self) -> Result<Vec<Buffer>> {
-        (0..self.array().n_buffers - 1)
+    fn buffers(&self, can_contain_null_mask: bool) -> Result<Vec<Buffer>> {
+        // + 1: skip null buffer
+        let buffer_begin = can_contain_null_mask as i64;
+        (buffer_begin..self.array().n_buffers)
             .map(|index| {
-                // + 1: skip null buffer
-                let index = (index + 1) as usize;
+                let index = index as usize;
 
                 let len = self.buffer_len(index)?;
 
@@ -668,7 +682,7 @@ pub trait ArrowArrayRef {
                     }
                     None => Err(ArrowError::CDataInterface(format!(
                         "The external buffer at position {} is null.",
-                        index - 1
+                        index
                     ))),
                 }
             })