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 c44f8d4c2f fix(arrow-data): don't double-count offset when slicing
struct ArrayData (#10835)
c44f8d4c2f is described below
commit c44f8d4c2fa6da9e990b273da60fa8a4add14419
Author: Jaideep Pyne <[email protected]>
AuthorDate: Tue Sep 1 13:10:41 2026 +0530
fix(arrow-data): don't double-count offset when slicing struct ArrayData
(#10835)
# Which issue does this PR close?
- Closes #7750.
- Closes #7595.
# Rationale for this change
Slicing a `Struct` `ArrayData` pushed the new offset into the child data
*and* also added it to the parent's own `offset`. Rebuilding an array
from the sliced data (`make_array` / `From<ArrayData> for StructArray`)
then windowed the already-windowed children a second time and panicked
with `(offset + length) <= self.len()`. This is a regression from 54.3.0
(it panics on 55.0.0 through current main).
This finishes the approach from #7596, which @alamb approved pending
additional testing before it went stale.
# What changes are included in this PR?
In the `Struct` arm of `ArrayData::slice`, keep `self.offset` unchanged
and let the cumulative child offsets carry the slice (a struct's
`ArrayData` has no buffers of its own, now guarded by an assert).
# Are these changes tested?
Yes. Added `test_struct_array_data_slice` (the C data interface offset
representation) and `test_make_array_sliced_struct_data` (the exact
#7750 reproducer), both failing before the change and passing after.
Verified no regressions across arrow-data, arrow-array, arrow-select,
arrow-ord, arrow-cast, and arrow-ipc.
# Are there any user-facing changes?
Slicing a struct `ArrayData` no longer panics on rebuild. The sliced
`ArrayData` now represents the offset on its children with the parent
offset unchanged — consistent with how `From<ArrayData> for StructArray`
already interprets it.
---
arrow-array/src/array/struct_array.rs | 133 +++++++++++++++++++++++++++++++++-
arrow-data/src/data.rs | 14 ++--
2 files changed, 141 insertions(+), 6 deletions(-)
diff --git a/arrow-array/src/array/struct_array.rs
b/arrow-array/src/array/struct_array.rs
index fa2718c7fd..0086cae8ed 100644
--- a/arrow-array/src/array/struct_array.rs
+++ b/arrow-array/src/array/struct_array.rs
@@ -630,7 +630,10 @@ impl Index<&str> for StructArray {
mod tests {
use super::*;
- use crate::{BooleanArray, Float32Array, Float64Array, Int32Array,
Int64Array, StringArray};
+ use crate::{
+ BooleanArray, Float32Array, Float64Array, Int32Array, Int64Array,
StringArray,
+ cast::AsArray, types::Int32Type,
+ };
use arrow_buffer::ToByteSlice;
#[test]
@@ -732,6 +735,134 @@ mod tests {
}
}
+ #[test]
+ fn test_struct_array_data_slice() {
+ // Slicing a struct's `ArrayData` and rebuilding an array from it has
to
+ // apply the offset to the children exactly once (#7595, #7750).
+ let x = Int32Array::from(vec![Some(0), Some(1), Some(2), Some(3),
None, Some(5)]);
+ let struct_array = StructArray::new(
+ Fields::from(vec![Field::new("x", DataType::Int32, true)]),
+ vec![Arc::new(x.clone())],
+ Some(NullBuffer::from(vec![true, true, true, false, true, true])),
+ )
+ .into_data();
+ let sliced = struct_array.slice(1, 4);
+
+ let arr = make_array(sliced);
+ assert_eq!(
+ arr.as_struct().column(0).as_primitive::<Int32Type>(),
+ &x.slice(1, 4)
+ );
+
+ // A struct whose top-level `ArrayData` carries a non-zero offset over
+ // full-length children is how the C++ implementation of Arrow (and the
+ // C data interface) represents a sliced struct: the offset/length live
+ // on the struct, not on the children. arrow-rs must decode it to the
+ // same logical array its own `StructArray::slice` produces.
+ let x = Int32Array::from(vec![Some(0), Some(1), Some(2), Some(3),
None, Some(5)]);
+ let y = Int32Array::from(vec![Some(5), Some(6), None, Some(8),
Some(9), Some(10)]);
+ let struct_array = StructArray::new(
+ Fields::from(vec![
+ Field::new("x", DataType::Int32, true),
+ Field::new("y", DataType::Int32, true),
+ ]),
+ vec![Arc::new(x), Arc::new(y)],
+ Some(NullBuffer::from(vec![true, true, true, false, true, true])),
+ );
+ let struct_array = StructArray::new(
+ Fields::from(vec![Field::new(
+ "inner",
+ struct_array.data_type().clone(),
+ true,
+ )]),
+ vec![Arc::new(struct_array)],
+ Some(NullBuffer::from(vec![true, false, true, true, true, true])),
+ );
+
+ let cpp_sliced_array = make_array(
+ struct_array
+ .to_data()
+ .into_builder()
+ .offset(1)
+ .len(4)
+ .nulls(Some(NullBuffer::from(vec![false, true, true, true])))
+ .build()
+ .unwrap(),
+ );
+
+ assert_eq!(cpp_sliced_array.as_struct(), &struct_array.slice(1, 4));
+ }
+
+ #[test]
+ fn test_make_array_sliced_struct_data() {
+ // Exact reproducer from #7750: `make_array` on a sliced struct's
+ // `ArrayData`.
+ let strings: ArrayRef = Arc::new(StringArray::from(vec![
+ Some("joe"),
+ None,
+ None,
+ Some("mark"),
+ Some("doe"),
+ ]));
+ let ints: ArrayRef = Arc::new(Int32Array::from(vec![
+ Some(1),
+ Some(2),
+ Some(3),
+ Some(4),
+ Some(5),
+ ]));
+
+ let array = StructArray::try_from(vec![("f1", strings.clone()), ("f2",
ints.clone())])
+ .unwrap()
+ .into_data()
+ .slice(1, 3);
+
+ let arr = make_array(array);
+ let expected = StructArray::try_from(vec![("f1", strings), ("f2",
ints)])
+ .unwrap()
+ .slice(1, 3);
+ assert_eq!(arr.as_struct(), &expected);
+ }
+
+ #[test]
+ fn test_slice_struct_data_with_existing_offset() {
+ // Slicing a struct whose `ArrayData` already carries a non-zero offset
+ // over full-length children, which is how the C data interface hands
us
+ // a sliced struct. The cumulative offset has to end up on the children
+ // only: anything left on the parent gets applied a second time by
+ // `From<ArrayData> for StructArray`.
+ let x = Int32Array::from(vec![Some(0), Some(1), Some(2), Some(3),
None, Some(5)]);
+ let struct_array = StructArray::new(
+ Fields::from(vec![Field::new("x", DataType::Int32, true)]),
+ vec![Arc::new(x.clone())],
+ Some(NullBuffer::from(vec![true, true, true, false, true, true])),
+ );
+
+ let offset_data = struct_array
+ .to_data()
+ .into_builder()
+ .offset(1)
+ .len(5)
+ .nulls(Some(NullBuffer::from(vec![true, true, false, true, true])))
+ .build()
+ .unwrap();
+
+ let sliced = offset_data.slice(1, 3);
+ // The cumulative offset (1 + 1) lands on the child; the parent's own
+ // offset is reset to 0 so nothing re-applies it.
+ assert_eq!(sliced.offset(), 0);
+ assert_eq!(sliced.len(), 3);
+ assert_eq!(sliced.child_data()[0].offset(), 2);
+ assert_eq!(sliced.child_data()[0].len(), 3);
+
+ let arr = make_array(sliced);
+ assert_eq!(
+ arr.as_struct().column(0).as_primitive::<Int32Type>(),
+ &x.slice(2, 3)
+ );
+ assert_eq!(arr.as_struct(), &struct_array.slice(2, 3));
+ }
+
#[test]
#[should_panic(expected = "assertion failed: end <= self.len()")]
fn test_struct_array_from_data_with_offset_and_length_error() {
diff --git a/arrow-data/src/data.rs b/arrow-data/src/data.rs
index ed73bf6273..0d46dbee62 100644
--- a/arrow-data/src/data.rs
+++ b/arrow-data/src/data.rs
@@ -696,19 +696,23 @@ impl ArrayData {
assert!(end <= self.len());
if let DataType::Struct(_) = self.data_type() {
- // Slice into children
- let new_offset = self.offset + offset;
+ // A struct has no buffers of its own, and reading child element
`i`
+ // combines this array's offset with the child's own offset.
Applying
+ // the slice to both would count it twice, so the cumulative offset
+ // goes to the children and this array's offset is reset to 0.
+ let child_offset = self.offset + offset;
ArrayData {
data_type: self.data_type().clone(),
len: length,
- offset: new_offset,
+ offset: 0,
buffers: self.buffers.clone(),
- // Slice child data, to propagate offsets down to them
child_data: self
.child_data()
.iter()
- .map(|data| data.slice(offset, length))
+ .map(|data| data.slice(child_offset, length))
.collect(),
+ // `nulls` belongs to this array rather than to the children,
so
+ // it is sliced by `offset` alone.
nulls: self.nulls.as_ref().map(|x| x.slice(offset, length)),
}
} else {
