Skip to content

Commit

Permalink
Skip null buffer when importing FFI ArrowArray struct if no null buff…
Browse files Browse the repository at this point in the history
…er in the spec (#3293)

* Fix null buffer import/export behavior

* Clippy

Co-authored-by: Raphael Taylor-Davies <r.taylordavies@googlemail.com>
  • Loading branch information
viirya and tustvold authored Dec 8, 2022
1 parent bf1cccb commit 7d21397
Showing 1 changed file with 26 additions and 12 deletions.
38 changes: 26 additions & 12 deletions arrow/src/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() + {
Expand Down Expand Up @@ -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| {
Expand Down Expand Up @@ -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)?;

Expand All @@ -668,7 +682,7 @@ pub trait ArrowArrayRef {
}
None => Err(ArrowError::CDataInterface(format!(
"The external buffer at position {} is null.",
index - 1
index
))),
}
})
Expand Down

0 comments on commit 7d21397

Please sign in to comment.