-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add BooleanArray::binary/unary helpers backed by buffer bitwise APIs #8876
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Add BooleanArray::binary and BooleanArray::unary in arrow-array, implemented in terms of Buffer::bitwise_binary / Buffer::bitwise_unary Update boolean kernels in arrow-arith to use these helpers instead of open-coded bitmap logic Add randomized equivalence tests in arrow-arith to validate the new APIs against existing behavior Add rand as a dev-dependency in arrow-arith for the new tests All behavior should be unchanged; this is an internal refactor toward bitwise API consolidation Tests run locally: cargo test -p arrow-array --lib cargo test -p arrow-arith --lib cargo test -p arrow-select --lib
alamb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @joe-ucp -- this looks like a good start.
| /// * `op` - The unary operation to apply. | ||
| pub fn unary<F>(&self, op: F) -> Self | ||
| where | ||
| F: Fn(u64) -> u64 + Copy, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we please change the bound to follow the bitwise unary signature ?
Namely
F: FnMut(u64) -> u64There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same thing applies below to binary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thanks! I’ll update BooleanArray::unary to use FnMut(u64) -> u64 to match the bitwise helper signatures.
| /// Returns `Err` if this is shared or its allocation is from an external source or | ||
| /// it is not allocated with alignment [`ALIGNMENT`] | ||
| /// | ||
| /// # Example: Creating a [`MutableBuffer`] from a [`Buffer`] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oversight from PR shuffle I did this morning, splitting these up. It got dropped when I pulled immutable.rs over from my exploratory branch.
I’ll restore the example doc comment in this PR; no behavior change was intended here.
Align BooleanArray helper bounds with Buffer bitwise APIs and restore the removed doc example in immutable.rs.
What issue does this PR close?
This PR extracts the BooleanArray bitwise changes from a larger exploratory PR to enable focused and independent review.
Rationale
This change consolidates BooleanArray bitwise logic by leveraging new buffer-level bitwise primitives. By doing so:
Summary of Changes
BooleanArray::binaryandBooleanArray::unarymethods to operate on the value bitmap, built on top ofBuffer::bitwise_binaryandBuffer::bitwise_unary.arrow-arith/src/boolean.rsto use these new helpers instead of manual bitmap logic.arrow-arithto validate the new helpers against existing logic across a range of offsets and lengths.randas a dev-dependency toarrow-arithfor randomized testing.nullifor other higher-level APIs in this PR.Testing
cargo test -p arrow-array --libcargo test -p arrow-arith --libcargo test -p arrow-select --libAll tests pass.
User-Facing Changes
None.
This is an internal refactor: new helpers and tests only. No changes to existing public APIs or observable behavior.