Skip to content

fix(arrow-data): align struct null validation with parent offset - #10970

Merged
Jefffrey merged 2 commits into
apache:mainfrom
linhongyu510:fix/struct-null-validation-parent-offset
Sep 5, 2026
Merged

fix(arrow-data): align struct null validation with parent offset#10970
Jefffrey merged 2 commits into
apache:mainfrom
linhongyu510:fix/struct-null-validation-parent-offset

Conversation

@linhongyu510

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

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.

@github-actions github-actions Bot added arrow Changes to the arrow crate arrow-data labels Sep 3, 2026
@linhongyu510
linhongyu510 marked this pull request as ready for review September 3, 2026 06:49
Comment thread arrow-data/src/data.rs Outdated
Comment on lines +1547 to +1548
let child = child.slice(self.offset, self.len);
self.validate_non_nullable(self.nulls(), &child)?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i feel we can modify validate_non_nullable to take in the child nulls, which we can slice here, instead of slicing the entire child which clones it

@linhongyu510

Copy link
Copy Markdown
Contributor Author

Addressed the review in 10a396f: validate_non_nullable now receives the relevant child NullBuffer, and Struct validation slices only that bitmap at the parent offset instead of cloning the full child ArrayData. cargo test -p arrow-data passed (56 unit + 13 doctests), plus workspace fmt and Clippy with -D warnings.

@Jefffrey Jefffrey added the bug label Sep 5, 2026
@Jefffrey
Jefffrey merged commit b372d1f into apache:main Sep 5, 2026
41 checks passed
@Jefffrey

Jefffrey commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

thanks @linhongyu510

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrow Changes to the arrow crate arrow-data bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Struct ArrayData non-nullable child validation ignores parent offset

2 participants