alamb commented on code in PR #9692:
URL: https://github.com/apache/arrow-rs/pull/9692#discussion_r3075244295
##########
arrow-buffer/src/buffer/null.rs:
##########
@@ -84,6 +84,20 @@ impl NullBuffer {
}
}
+ /// Computes the union of the nulls in multiple optional [`NullBuffer`]s
+ ///
+ /// See [`union`](Self::union)
+ pub fn union_many(nulls: &[Option<&NullBuffer>]) -> Option<NullBuffer> {
+ // Unwrap to BooleanBuffer because BitAndAssign is not implemented for
NullBuffer
+ let mut buffers = nulls.iter().filter_map(|nb|
nb.map(NullBuffer::inner));
+ let first = buffers.next()?;
Review Comment:
As written I think this signature will require at least one copy --
```rust
pub fn union_many(nulls: &[Option<&NullBuffer>]) -> Option<NullBuffer>
{
```
Because the first null buffer must be cloned. Is there some better
signature? Like perhaps someting like
```rust
pub fn union_in_place(mut self, other: &NullBuffer) -> Self {
self.buffer &= other.inner();
self.null_count = self.buffer.len() - self.buffer.count_set_bits();
self
}
```
I can propose adding the `union_in_place` maybe as a follow on PR
--
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]
