[GitHub] [arrow] alamb commented on a change in pull request #8571: ARROW-10461: [Rust] Fix offset bug in remainder bits

[GitBox <gi...@apache.org>]

alamb commented on a change in pull request #8571:
URL: https://github.com/apache/arrow/pull/8571#discussion_r518757983



##########
File path: rust/arrow-flight/Cargo.toml
##########
@@ -27,7 +27,10 @@ license = "Apache-2.0"
 
 [dependencies]
 arrow = { path = "../arrow", version = "3.0.0-SNAPSHOT" }
-tonic = "0.3"
+tonic = "0.3.1"

Review comment:
       I wonder if there is some reason to upgrade tonic in this same PR?

##########
File path: rust/arrow/src/util/bit_chunk_iterator.rs
##########
@@ -55,48 +58,52 @@ impl<'a> BitChunks<'a> {
 pub struct BitChunkIterator<'a> {
     buffer: &'a Buffer,
     raw_data: *const u8,
-    offset: usize,
+    bit_offset: usize,
     chunk_len: usize,
     index: usize,
 }
 
 impl<'a> BitChunks<'a> {
+    /// Returns the number of remaining bits, guaranteed to be between 0 and 63 (inclusive)
     #[inline]
     pub const fn remainder_len(&self) -> usize {
         self.remainder_len
     }
 
+    /// Returns the bitmask of remaining bits
     #[inline]
     pub fn remainder_bits(&self) -> u64 {
         let bit_len = self.remainder_len;
         if bit_len == 0 {
             0
         } else {
-            let byte_len = ceil(bit_len, 8);

Review comment:
       Just to confirm my understanding, the bug is that the previous code assumed the remainder bits always started at a byte boundary like:
   
   ```
   +--------+--------+--------+----+
   |        |        |        |    |
   +--------+--------+--------+----+
            ^
            (remainder starts)
   ```
   
   But produces incorrect behavior if the remainder starts in the middle of a byte
   ```
   +--------+--------+--------+----+
   |        |        |        |    |
   +--------+--------+--------+----+
               ^
               (remainder starts)
   ```
   




----------------------------------------------------------------
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:
users@infra.apache.org