alamb commented on code in PR #7013:
URL: https://github.com/apache/arrow-rs/pull/7013#discussion_r1929170949
##########
arrow-buffer/src/builder/null.rs:
##########
@@ -134,6 +133,42 @@ impl NullBufferBuilder {
}
}
+ /// Returns the capacity of the buffer
+ #[inline]
+ pub fn capacity(&self) -> usize {
+ if let Some(buf) = self.bitmap_builder.as_ref() {
+ buf.capacity()
+ } else {
+ 0
Review Comment:
I agree it is a bit strange, but I think the PR as written makes the most
sense.
Specifically, downstream in DataFusion (and other places) we use the
`capacity` as a way to calculate how much memory has been allocated -- if there
is no bitmap_builder there is no memory allocated.
I think we can make this less confusing with some comments. I left some
suggestions
Another thing that might help might be to rename it to `fn
allocated_capacity()` to make the difference more explicit 🤔
##########
arrow-buffer/src/builder/null.rs:
##########
@@ -134,6 +133,42 @@ impl NullBufferBuilder {
}
}
+ /// Returns the capacity of the buffer
Review Comment:
```suggestion
/// Returns the allocated capacity of the buffer, if any
///
/// Note: if no nulls have been added, no buffer has been materialized
/// and this function will return 0, regardless of the capacity passed
to `Self::new`
```
##########
arrow-buffer/src/builder/null.rs:
##########
@@ -134,6 +133,42 @@ impl NullBufferBuilder {
}
}
+ /// Returns the capacity of the buffer
+ #[inline]
+ pub fn capacity(&self) -> usize {
+ if let Some(buf) = self.bitmap_builder.as_ref() {
+ buf.capacity()
+ } else {
+ 0
+ }
+ }
+
+ /// Gets a bit in the buffer at `index`
+ #[inline]
+ pub fn is_valid(&mut self, index: usize) -> bool {
+ if let Some(buf) = self.bitmap_builder.as_mut() {
+ buf.get_bit(index)
+ } else {
+ true
+ }
+ }
+
+ /// Truncates the builder to the given length
+ ///
+ /// If `len` is greater than the buffer's current length, this has no
effect
+ #[inline]
+ pub fn truncate(&mut self, len: usize) {
+ if len > self.len {
+ return;
+ }
Review Comment:
I am not sure what you mean -- in this case the user requested to truncate
to a value *larger* than the current length which has no effect.
This behavior seems to be consistent with `Vec::truncate` so it makes sense
to me: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.truncate
--
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]
