alamb commented on a change in pull request #8598:
URL: https://github.com/apache/arrow/pull/8598#discussion_r518961426
##########
File path: rust/arrow/src/buffer.rs
##########
@@ -259,16 +260,20 @@ impl Buffer {
/// Returns a slice of this buffer starting at a certain bit offset.
/// If the offset is byte-aligned the returned buffer is a shallow clone,
/// otherwise a new buffer is allocated and filled with a copy of the bits
in the range.
- pub fn bit_slice(&self, offset: usize, len: usize) -> Self {
- if offset % 8 == 0 && len % 8 == 0 {
- return self.slice(offset / 8);
+ pub fn bit_view(&self, offset_in_bits: usize, len_in_bits: usize) -> Self {
+ if offset_in_bits % 8 == 0 && len_in_bits % 8 == 0 {
+ self.slice(offset_in_bits / 8)
Review comment:
> If you give the whole buffer's length in bits and start offset as 0
then it will cover the whole buffer
Right, what I don't understand is how the test for `len_in_bits % 8 == 0` is
checking for the whole buffer length. It seems like it is checking that
`len_in_bits` is a multiple of 8 (aka represents whole bytes)
Maybe there is some assumption here like `self.len_in_bits < 8`?
##########
File path: rust/arrow/src/util/bit_slice_iterator.rs
##########
@@ -63,7 +63,7 @@ impl<'a> BufferBitSlice<'a> {
where
T: BitMemory,
{
- let offset_size_in_bits = 8 * std::mem::size_of::<u64>();
Review comment:
👍
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
