This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-11362-40699a23ff7bf65a70a707a726ef3ffe50b178df in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
commit d4edc6846e1b7176fe22ee5f309358f7330f4fcc Author: Liang-Chi Hsieh <[email protected]> AuthorDate: Sun Oct 4 07:53:19 2026 +0000 fix(arrow-data): account for parent offset in FixedSizeList null validation (#11362) # Which issue does this PR close? Closes #11363. # Rationale for this change FixedSizeList nullability validation compares expanded parent validity with the unsliced child validity, ignoring the parent offset. This can both reject valid arrays and accept nulls in non-nullable items. For example, with list size 2, parent offset 1 and length 2, and child values `[0, 1, null, null, 4, 5]`, parent validity `[false, true]` should be accepted but is rejected; `[true, false]` should be rejected but is accepted. When the parent has no null buffer, child nulls outside the visible range can also cause an incorrect rejection. # What changes are included in this PR? - Slice child validity to `offset * list_size .. (offset + len) * list_size` before checking non-nullable items. - Include the parent offset in the required child length check so validation rejects undersized children before slicing. - Add regression coverage for masked and visible child nulls, child offsets, absent parent null buffers, nulls outside the visible range, empty slices, zero-sized lists, and undersized children. # Are these changes tested? All three new regression tests failed before the fix and pass afterward. - `cargo test -p arrow-buffer -p arrow-data -p arrow-array -p arrow-select -p arrow-ipc --lib`: 1,690 passed, 1 ignored. - `cargo clippy -p arrow-data --all-targets -- -D warnings`: passed. - `cargo fmt --all -- --check`: passed. - `git diff --check`: passed. # Are there any user-facing changes? Validation now correctly accepts child nulls masked by a sliced FixedSizeList parent or outside its visible range, and rejects visible nulls in non-nullable items. Undersized child arrays are rejected when the parent offset requires additional elements. No public API changes. # AI assistance AI assistance was used to investigate the validation semantics, implement the fix and regression tests, review the diff, run the checks above, and prepare this PR description. --- arrow-data/src/data.rs | 104 +++++++++++++++++++++++++++++++++++----- arrow/tests/array_validation.rs | 2 +- 2 files changed, 94 insertions(+), 12 deletions(-) diff --git a/arrow-data/src/data.rs b/arrow-data/src/data.rs index c2f32ef9d0..a8a6970125 100644 --- a/arrow-data/src/data.rs +++ b/arrow-data/src/data.rs @@ -1283,14 +1283,16 @@ impl ArrayData { )) })?; - let expected_values_len = self.len + let len_plus_offset = + checked_len_plus_offset(&self.data_type, self.len, self.offset)?; + let expected_values_len = len_plus_offset .checked_mul(list_size) .expect("integer overflow computing expected number of expected values in FixedListSize"); if values_data.len < expected_values_len { return Err(ArrowError::InvalidArgumentError(format!( - "Values length {} is less than the length ({}) multiplied by the value size ({}) for {}", - values_data.len, self.len, list_size, self.data_type + "Values length {} is less than the length + offset ({}) multiplied by the value size ({}) for {}", + values_data.len, len_plus_offset, list_size, self.data_type ))); } } @@ -1526,14 +1528,12 @@ impl ArrayData { DataType::FixedSizeList(field, len) => { let child = &self.child_data[0]; if !field.is_nullable() { - match &self.nulls { - Some(nulls) => { - let element_len = *len as usize; - let expanded = nulls.expand(element_len); - self.validate_non_nullable(Some(&expanded), child, child.nulls())?; - } - None => self.validate_non_nullable(None, child, child.nulls())?, - } + let element_len = *len as usize; + let child_nulls = child.nulls().map(|nulls| { + nulls.slice(self.offset * element_len, self.len * element_len) + }); + let expanded = self.nulls.as_ref().map(|nulls| nulls.expand(element_len)); + self.validate_non_nullable(expanded.as_ref(), child, child_nulls.as_ref())?; } } DataType::Struct(fields) => { @@ -2587,6 +2587,88 @@ mod tests { assert!(build(vec![true, true, false, true]).is_err()); } + #[test] + fn test_fixed_size_list_non_nullable_child_nulls_account_for_parent_offset() { + for child_offset in [0, 1] { + let child = ArrayData::builder(DataType::Int32) + .len(6) + .offset(child_offset) + .add_buffer(Buffer::from_slice_ref([0i32; 7])) + .nulls(Some(NullBuffer::from(vec![ + true, true, false, false, true, true, + ]))) + .build() + .unwrap(); + let build = |parent_nulls| { + ArrayData::builder(DataType::FixedSizeList( + Arc::new(Field::new("item", DataType::Int32, false)), + 2, + )) + .offset(1) + .len(2) + .nulls(Some(NullBuffer::from(parent_nulls))) + .add_child_data(child.clone()) + .build() + }; + + // The first visible list contains both child nulls. + assert!(build(vec![false, true]).is_ok()); + assert!(build(vec![true, false]).is_err()); + } + } + + #[test] + fn test_fixed_size_list_non_nullable_child_nulls_without_parent_nulls() { + let child = ArrayData::builder(DataType::Int32) + .len(6) + .add_buffer(Buffer::from_slice_ref([0i32; 6])) + .nulls(Some(NullBuffer::from(vec![ + false, false, true, true, false, false, + ]))) + .build() + .unwrap(); + let build = |offset, len, size| { + ArrayData::builder(DataType::FixedSizeList( + Arc::new(Field::new("item", DataType::Int32, false)), + size, + )) + .offset(offset) + .len(len) + .add_child_data(child.clone()) + .build() + }; + + // Nulls before and after the visible child range are irrelevant. + assert!(build(1, 1, 2).is_ok()); + assert!(build(0, 1, 2).is_err()); + assert!(build(2, 1, 2).is_err()); + assert!(build(3, 0, 2).is_ok()); + assert!(build(1, 2, 0).is_ok()); + } + + #[test] + fn test_fixed_size_list_validation_accounts_for_parent_offset() { + for nulls in [None, Some(NullBuffer::new_null(4))] { + let child = ArrayData::builder(DataType::Int32) + .len(4) + .add_buffer(Buffer::from_slice_ref([0i32; 4])) + .nulls(nulls) + .build() + .unwrap(); + let err = ArrayData::builder(DataType::FixedSizeList( + Arc::new(Field::new("item", DataType::Int32, false)), + 2, + )) + .offset(1) + .len(2) + .add_child_data(child) + .build() + .unwrap_err() + .to_string(); + assert!(err.contains("Values length 4 is less than")); + } + } + #[test] fn test_struct_equal_accounts_for_parent_offset() { let data_type = diff --git a/arrow/tests/array_validation.rs b/arrow/tests/array_validation.rs index 6455a15b3e..70c1cf34fb 100644 --- a/arrow/tests/array_validation.rs +++ b/arrow/tests/array_validation.rs @@ -432,7 +432,7 @@ fn test_validate_large_list_view_negative_sizes() { #[test] #[should_panic( - expected = "Values length 4 is less than the length (3) multiplied by the value size (2) for FixedSizeList" + expected = "Values length 4 is less than the length + offset (3) multiplied by the value size (2) for FixedSizeList" )] fn test_validate_fixed_size_list() { // child has 4 elements,
