This is an automated email from the ASF dual-hosted git repository.
Jefffrey pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new 027b45f2a9 fix(arrow-data): account for struct parent offsets (#10934)
027b45f2a9 is described below
commit 027b45f2a982832c93d159de6718d69d0e8b60e3
Author: Jaideep Pyne <[email protected]>
AuthorDate: Wed Sep 2 18:14:18 2026 +0530
fix(arrow-data): account for struct parent offsets (#10934)
Closes #10933.
Struct `ArrayData` parent offsets compose with child offsets, but a few
paths were still treating child data as if parent offset were always
zero. This updates:
- struct child validation to require `offset + len` child elements
- struct equality to compare child values at `parent_offset +
logical_start`
- struct `MutableArrayData` extension to copy from the
parent-offset-adjusted child range
The regression tests cover the concrete cases from #10933: validation,
equality, and `MutableArrayData::try_extend`.
Validation run locally:
```
cargo test -p arrow-data --lib
cargo fmt --check
cargo clippy -p arrow-data --lib --tests -- -D warnings
```
---
arrow-array/src/array/struct_array.rs | 22 +++++-----
arrow-data/src/data.rs | 83 ++++++++++++++++++++++++++++++++++-
arrow-data/src/equal/structure.rs | 3 ++
arrow-data/src/transform/structure.rs | 15 ++++++-
4 files changed, 109 insertions(+), 14 deletions(-)
diff --git a/arrow-array/src/array/struct_array.rs
b/arrow-array/src/array/struct_array.rs
index 0086cae8ed..592126318a 100644
--- a/arrow-array/src/array/struct_array.rs
+++ b/arrow-array/src/array/struct_array.rs
@@ -864,22 +864,24 @@ mod tests {
}
#[test]
- #[should_panic(expected = "assertion failed: end <= self.len()")]
fn test_struct_array_from_data_with_offset_and_length_error() {
let int_arr = Int32Array::from(vec![1, 2, 3, 4, 5]);
let int_field = Field::new("x", DataType::Int32, false);
let struct_nulls = NullBuffer::new(BooleanBuffer::from(vec![true,
true, false]));
let int_data = int_arr.to_data();
// If parent offset is 3 and len is 3 then child must have 6 items
- let struct_data =
-
ArrayData::builder(DataType::Struct(Fields::from(vec![int_field.clone()])))
- .len(3)
- .offset(3)
- .nulls(Some(struct_nulls))
- .add_child_data(int_data)
- .build()
- .unwrap();
- let _ = StructArray::from(struct_data);
+ let err =
ArrayData::builder(DataType::Struct(Fields::from(vec![int_field.clone()])))
+ .len(3)
+ .offset(3)
+ .nulls(Some(struct_nulls))
+ .add_child_data(int_data)
+ .build()
+ .unwrap_err()
+ .to_string();
+
+ assert!(err.contains(
+ "child array #0 for field x has length smaller than expected for
struct array (5 < 6)"
+ ));
}
/// validates that struct can be accessed using `column_name` as index
i.e. `struct_array["column_name"]`.
diff --git a/arrow-data/src/data.rs b/arrow-data/src/data.rs
index 0d46dbee62..8078c7cfb5 100644
--- a/arrow-data/src/data.rs
+++ b/arrow-data/src/data.rs
@@ -1298,18 +1298,20 @@ impl ArrayData {
}
DataType::Struct(fields) => {
self.validate_num_child_data(fields.len())?;
+ let len_plus_offset =
+ checked_len_plus_offset(&self.data_type, self.len,
self.offset)?;
for (i, field) in fields.iter().enumerate() {
let field_data = self.get_valid_child_data(i,
field.data_type())?;
// Ensure child field has sufficient size
- if field_data.len < self.len {
+ if field_data.len < len_plus_offset {
return Err(ArrowError::InvalidArgumentError(format!(
"{} child array #{} for field {} has length
smaller than expected for struct array ({} < {})",
self.data_type,
i,
field.name(),
field_data.len,
- self.len
+ len_plus_offset
)));
}
}
@@ -2444,6 +2446,7 @@ pub(crate) fn get_fixed_size_binary_width(data_type:
&DataType) -> usize {
mod tests {
use super::*;
use crate::ByteView;
+ use crate::transform::MutableArrayData;
use arrow_buffer::{OffsetBuffer, ScalarBuffer};
use arrow_schema::{Field, Fields};
@@ -2511,6 +2514,82 @@ mod tests {
assert_eq!(child_arr_data, arr_data.child_data()[0]);
}
+ #[test]
+ fn test_struct_validation_accounts_for_parent_offset() {
+ let data_type =
+ DataType::Struct(Fields::from(vec![Field::new("x",
DataType::Int32, false)]));
+ let child = ArrayData::builder(DataType::Int32)
+ .len(5)
+ .add_buffer(Buffer::from_slice_ref([0, 1, 2, 3, 4]))
+ .build()
+ .unwrap();
+
+ // The parent needs child elements 1..6, but the child only has five.
+ let err = ArrayData::builder(data_type)
+ .len(5)
+ .offset(1)
+ .add_child_data(child)
+ .build()
+ .unwrap_err()
+ .to_string();
+
+ assert!(err.contains(
+ "child array #0 for field x has length smaller than expected for
struct array (5 < 6)"
+ ));
+ }
+
+ #[test]
+ fn test_struct_equal_accounts_for_parent_offset() {
+ let data_type =
+ DataType::Struct(Fields::from(vec![Field::new("x",
DataType::Int32, false)]));
+
+ let child1 = ArrayData::builder(DataType::Int32)
+ .len(5)
+ .add_buffer(Buffer::from_slice_ref([0, 1, 2, 3, 4]))
+ .build()
+ .unwrap();
+ let child2 = child1.slice(1, 4);
+
+ // data1 has offset at parent level; data2 has offset at child level.
+ let data1 = ArrayData::builder(data_type.clone())
+ .len(4)
+ .offset(1)
+ .add_child_data(child1)
+ .build()
+ .unwrap();
+ let data2 = ArrayData::builder(data_type)
+ .len(4)
+ .add_child_data(child2)
+ .build()
+ .unwrap();
+
+ assert_eq!(data1, data2);
+ }
+
+ #[test]
+ fn test_extend_struct_accounts_for_parent_offset() {
+ let data_type =
+ DataType::Struct(Fields::from(vec![Field::new("x",
DataType::Int32, false)]));
+ let child = ArrayData::builder(DataType::Int32)
+ .len(5)
+ .add_buffer(Buffer::from_slice_ref([0, 1, 2, 3, 4]))
+ .build()
+ .unwrap();
+
+ let data = ArrayData::builder(data_type)
+ .len(4)
+ .offset(1)
+ .add_child_data(child)
+ .build()
+ .unwrap();
+
+ let mut mutable = MutableArrayData::new(vec![&data], false,
data.len());
+ mutable.try_extend(0, 0, data.len()).unwrap();
+ let output = mutable.freeze();
+
+ assert_eq!(output.child_data()[0].buffer::<i32>(0), &[1, 2, 3, 4]);
+ }
+
#[test]
fn test_null_count() {
let mut bit_v: [u8; 2] = [0; 2];
diff --git a/arrow-data/src/equal/structure.rs
b/arrow-data/src/equal/structure.rs
index d6efaff9e4..b527380a12 100644
--- a/arrow-data/src/equal/structure.rs
+++ b/arrow-data/src/equal/structure.rs
@@ -28,6 +28,9 @@ fn equal_child_values(
rhs_start: usize,
len: usize,
) -> bool {
+ let lhs_start = lhs_start + lhs.offset();
+ let rhs_start = rhs_start + rhs.offset();
+
lhs.child_data()
.iter()
.zip(rhs.child_data())
diff --git a/arrow-data/src/transform/structure.rs
b/arrow-data/src/transform/structure.rs
index 9e48f6759c..78ab19768a 100644
--- a/arrow-data/src/transform/structure.rs
+++ b/arrow-data/src/transform/structure.rs
@@ -19,12 +19,23 @@ use super::{_MutableArrayData, Extend};
use crate::ArrayData;
use arrow_schema::{ArrowError, DataType};
-pub(super) fn build_extend(_: &ArrayData) -> Extend<'_> {
+pub(super) fn build_extend(array: &ArrayData) -> Extend<'_> {
+ let offset = array.offset();
Box::new(
move |mutable: &mut _MutableArrayData, index: usize, start: usize,
len: usize| {
+ let start = start.checked_add(offset).ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "struct offset {offset} with start {start} overflows usize"
+ ))
+ })?;
+ let end = start.checked_add(len).ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "struct start {start} with length {len} overflows usize"
+ ))
+ })?;
for (col_idx, child) in mutable.child_data.iter_mut().enumerate() {
child
- .try_extend(index, start, start + len)
+ .try_extend(index, start, end)
.map_err(|e| wrap_column_error(e, col_idx,
&mutable.data_type))?
}
Ok(())
