From 526fd1d8db5aca78655ddfb8d75d527af228de7f Mon Sep 17 00:00:00 2001
From: Jorge Leitao <jorgecarleitao@gmail.com>
Date: Fri, 30 Jul 2021 07:51:17 +0200
Subject: [PATCH] Fixed error in slicing bitmap. (#237)

---
 src/bitmap/immutable.rs            |  7 +------
 src/bitmap/mutable.rs              |  2 +-
 src/bitmap/utils/slice_iterator.rs | 14 +++++++++++++-
 src/compute/aggregate/memory.rs    |  4 ++--
 4 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/src/bitmap/immutable.rs b/src/bitmap/immutable.rs
index 1ac94755b42..5630e8b4bd8 100644
--- a/src/bitmap/immutable.rs
+++ b/src/bitmap/immutable.rs
@@ -166,11 +166,6 @@ impl Bitmap {
     pub fn chunks<T: BitChunk>(&self) -> BitChunks<T> {
         BitChunks::new(&self.bytes, self.offset, self.length)
     }
-
-    #[inline]
-    pub(crate) fn bytes(&self) -> &[u8] {
-        &self.bytes
-    }
 }
 
 impl Bitmap {
@@ -217,9 +212,9 @@ impl Bitmap {
         self.offset % 8
     }
 
+    /// Returns the byte slice of this Bitmap.
     #[inline]
     pub(crate) fn as_slice(&self) -> &[u8] {
-        assert_eq!(self.offset % 8, 0); // slices only make sense when there is no offset
         let start = self.offset / 8;
         let len = self.length.saturating_add(7) / 8;
         &self.bytes[start..start + len]
diff --git a/src/bitmap/mutable.rs b/src/bitmap/mutable.rs
index f2b94e2709c..6255f3a0cb3 100644
--- a/src/bitmap/mutable.rs
+++ b/src/bitmap/mutable.rs
@@ -434,7 +434,7 @@ impl MutableBitmap {
     /// Extends the [`MutableBitmap`] from a [`Bitmap`].
     #[inline]
     pub fn extend_from_bitmap(&mut self, bitmap: &Bitmap) {
-        self.extend_from_slice(bitmap.bytes(), bitmap.offset(), bitmap.len());
+        self.extend_from_slice(bitmap.as_slice(), bitmap.offset(), bitmap.len());
     }
 }
 
diff --git a/src/bitmap/utils/slice_iterator.rs b/src/bitmap/utils/slice_iterator.rs
index eaa48000ab5..2dfbe111eef 100644
--- a/src/bitmap/utils/slice_iterator.rs
+++ b/src/bitmap/utils/slice_iterator.rs
@@ -29,7 +29,7 @@ pub struct SlicesIterator<'a> {
 impl<'a> SlicesIterator<'a> {
     pub fn new(values: &'a Bitmap) -> Self {
         let offset = values.offset();
-        let buffer = &values.bytes()[offset / 8..];
+        let buffer = values.as_slice();
         let mut iter = buffer.iter();
 
         let (current_byte, state) = match iter.next() {
@@ -272,4 +272,16 @@ mod tests {
 
         assert_eq!(count, total);
     }
+
+    #[test]
+    fn sliced() {
+        let values = Bitmap::from_u8_slice(&[0b11111010, 0b11111011], 16);
+        let values = values.slice(8, 2);
+        let iter = SlicesIterator::new(&values);
+
+        let chunks = iter.collect::<Vec<_>>();
+
+        // the first "11" in the second byte
+        assert_eq!(chunks, vec![(0, 2)]);
+    }
 }
diff --git a/src/compute/aggregate/memory.rs b/src/compute/aggregate/memory.rs
index e5713b73eb9..7b1eb239d00 100644
--- a/src/compute/aggregate/memory.rs
+++ b/src/compute/aggregate/memory.rs
@@ -4,7 +4,7 @@ use crate::datatypes::{DataType, IntervalUnit};
 use crate::types::days_ms;
 
 fn validity_size(validity: &Option<Bitmap>) -> usize {
-    validity.as_ref().map(|b| b.bytes().len()).unwrap_or(0)
+    validity.as_ref().map(|b| b.as_slice().len()).unwrap_or(0)
 }
 
 macro_rules! dyn_primitive {
@@ -55,7 +55,7 @@ pub fn estimated_bytes_size(array: &dyn Array) -> usize {
         Null => 0,
         Boolean => {
             let array = array.as_any().downcast_ref::<BooleanArray>().unwrap();
-            array.values().bytes().len() + validity_size(array.validity())
+            array.values().as_slice().len() + validity_size(array.validity())
         }
         Int8 => dyn_primitive!(array, i8),
         Int16 => dyn_primitive!(array, i16),