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 b372d1f14c fix(arrow-data): align struct null validation with parent
offset (#10970)
b372d1f14c is described below
commit b372d1f14cfe8974d8b71849e83bfa85f2a0494c
Author: hylin <[email protected]>
AuthorDate: Sat Sep 5 08:33:06 2026 +0800
fix(arrow-data): align struct null validation with parent offset (#10970)
# Which issue does this PR close?
- Closes #10951.
# Rationale for this change
For a struct with a non-zero parent offset, visible parent row `i` maps
to child row `offset + i`. Nullability validation compared the visible
parent null buffer against the unsliced child null buffer, so it could
reject masked child nulls and accept visible child nulls.
# What changes are included in this PR?
- Slice each struct child to the parent-visible window before validating
non-nullable fields.
- Add a regression test covering both the correctly masked and
incorrectly unmasked cases.
# Are these changes tested?
Yes. TDD evidence on `main` at `c134baf8f`:
- Before the production change, the focused regression failed because
the correctly masked child null was rejected.
- After the change, the focused regression passes.
- `cargo test -p arrow-data --lib`: 56 passed.
- `cargo fmt --all -- --check`: passed.
- `cargo clippy -p arrow-data --all-targets -- -D warnings`: passed.
- `git diff --check`: passed.
# Are there any user-facing changes?
Yes. Validation now correctly accepts child nulls masked by a sliced
struct parent and rejects child nulls visible through a non-null parent
row. There are no API changes.
# AI assistance
I used AI assistance to investigate the validation path, implement the
focused test and fix, and prepare this description. I reviewed the
complete diff and verified the behavior with the commands above.
---
arrow-data/src/data.rs | 45 +++++++++++++++++++++++++++++++++++++++------
1 file changed, 39 insertions(+), 6 deletions(-)
diff --git a/arrow-data/src/data.rs b/arrow-data/src/data.rs
index 8078c7cfb5..4b9acca77a 100644
--- a/arrow-data/src/data.rs
+++ b/arrow-data/src/data.rs
@@ -1525,7 +1525,8 @@ impl ArrayData {
match &self.data_type {
DataType::List(f) | DataType::LargeList(f) | DataType::Map(f, _)
=> {
if !f.is_nullable() {
- self.validate_non_nullable(None, &self.child_data[0])?
+ let child = &self.child_data[0];
+ self.validate_non_nullable(None, child, child.nulls())?
}
}
DataType::FixedSizeList(field, len) => {
@@ -1535,16 +1536,19 @@ impl ArrayData {
Some(nulls) => {
let element_len = *len as usize;
let expanded = nulls.expand(element_len);
- self.validate_non_nullable(Some(&expanded),
child)?;
+ self.validate_non_nullable(Some(&expanded), child,
child.nulls())?;
}
- None => self.validate_non_nullable(None, child)?,
+ None => self.validate_non_nullable(None, child,
child.nulls())?,
}
}
}
DataType::Struct(fields) => {
for (field, child) in fields.iter().zip(&self.child_data) {
if !field.is_nullable() {
- self.validate_non_nullable(self.nulls(), child)?
+ let child_nulls = child
+ .nulls()
+ .map(|nulls| nulls.slice(self.offset, self.len));
+ self.validate_non_nullable(self.nulls(), child,
child_nulls.as_ref())?
}
}
}
@@ -1559,9 +1563,10 @@ impl ArrayData {
&self,
mask: Option<&NullBuffer>,
child: &ArrayData,
+ child_nulls: Option<&NullBuffer>,
) -> Result<(), ArrowError> {
let Some(mask) = mask else {
- return match child.null_count() {
+ return match
child_nulls.map(NullBuffer::null_count).unwrap_or_default() {
0 => Ok(()),
_ => Err(ArrowError::InvalidArgumentError(format!(
"non-nullable child of type {} contains nulls not present
in parent {}",
@@ -1570,7 +1575,7 @@ impl ArrayData {
};
};
- match child.nulls() {
+ match child_nulls {
Some(nulls) if !mask.contains(nulls) =>
Err(ArrowError::InvalidArgumentError(format!(
"non-nullable child of type {} contains nulls not present in
parent",
child.data_type
@@ -2538,6 +2543,34 @@ mod tests {
));
}
+ #[test]
+ fn test_struct_non_nullable_child_nulls_account_for_parent_offset() {
+ let build = |parent_nulls| {
+ let child = ArrayData::builder(DataType::Int32)
+ .len(5)
+ .add_buffer(Buffer::from_slice_ref([0, 1, 2, 3, 4]))
+ .nulls(Some(NullBuffer::new(BooleanBuffer::from(vec![
+ true, true, false, true, true,
+ ]))))
+ .build()
+ .unwrap();
+
+ ArrayData::builder(DataType::Struct(Fields::from(vec![Field::new(
+ "x",
+ DataType::Int32,
+ false,
+ )])))
+ .len(4)
+ .offset(1)
+ .nulls(Some(NullBuffer::new(BooleanBuffer::from(parent_nulls))))
+ .add_child_data(child)
+ .build()
+ };
+
+ assert!(build(vec![true, false, true, true]).is_ok());
+ assert!(build(vec![true, true, false, true]).is_err());
+ }
+
#[test]
fn test_struct_equal_accounts_for_parent_offset() {
let data_type =
