alliasgher commented on code in PR #10709:
URL: https://github.com/apache/arrow-rs/pull/10709#discussion_r3886138145
##########
arrow-data/src/data.rs:
##########
@@ -965,7 +965,8 @@ impl ArrayData {
}
let actual_len = nulls.validity().len();
- let needed_len = bit_util::ceil(len_plus_offset, 8);
+ // ArrayData::offset does not apply to the null buffer, which
carries its own offset
+ let needed_len = bit_util::ceil(nulls.offset() + nulls.len(), 8);
if actual_len < needed_len {
Review Comment:
Fair question, and yes, I use an AI assistant. I should have said so up
front given the length and shape of that reply, and I have added a disclosure
to the PR description per `CONTRIBUTING.md`'s AI Generated Submissions section.
The findings in it are mine and I checked each one rather than pasting
output:
- `BooleanBuffer::new` asserting `bit_offset + bit_len <= buffer.len() * 8`
is at `arrow-buffer/src/buffer/boolean.rs:124-131`.
- `NullBuffer::new_unchecked` takes an already-built `BooleanBuffer`, so its
safety contract is only about `null_count`.
- The FFI path reaches `try_new` with a raw `Buffer`, which is why that
sibling check is the reachable one.
- I ran the removal locally before suggesting it: clean build, `clippy
--all-targets -- -D warnings` clean, 44 unit plus 13 doc tests, and
`array_validation` 58 including `test_bitmap_too_small`.
I will keep replies here shorter. Still happy to push either the removal or
the current fix, whichever you prefer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
