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. There is an AI disclosure
section at the bottom of the PR description per `CONTRIBUTING.md`, but I should
have repeated it here given the length and shape of that reply rather than
making you ask.
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]
