Remove length of patches from ALP and BitPacked array (#688)

spiraldb · Aug 27, 2024 · 960048e · 960048e
1 parent 48bdbd6
commit 960048e
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 21 deletions.
diff --git a/encodings/alp/src/array.rs b/encodings/alp/src/array.rs
@@ -22,7 +22,6 @@ pub struct ALPMetadata {
     exponents: Exponents,
     encoded_dtype: DType,
     patches_dtype: Option<DType>,
-    patches_len: usize,
 }
 
 impl ALPArray {
@@ -39,9 +38,19 @@ impl ALPArray {
         };
 
         let length = encoded.len();
+        if let Some(parray) = patches.as_ref() {
+            if parray.len() != length {
+                vortex_bail!(
+                    "Mismatched length in ALPArray between encoded({}) {} and it's patches({}) {}",
+                    encoded.encoding().id(),
+                    encoded.len(),
+                    parray.encoding().id(),
+                    parray.len()
+                )
+            }
+        }
 
         let patches_dtype = patches.as_ref().map(|a| a.dtype().as_nullable());
-        let patches_len = patches.as_ref().map(|a| a.len()).unwrap_or(0);
         let mut children = Vec::with_capacity(2);
         children.push(encoded);
         if let Some(patch) = patches {
@@ -55,7 +64,6 @@ impl ALPArray {
                 exponents,
                 encoded_dtype,
                 patches_dtype,
-                patches_len,
             },
             children.into(),
             Default::default(),
@@ -83,15 +91,13 @@ impl ALPArray {
 
     pub fn patches(&self) -> Option<Array> {
         self.metadata().patches_dtype.as_ref().map(|dt| {
-            self.array()
-                .child(1, dt, self.metadata().patches_len)
-                .unwrap_or_else(|| {
-                    panic!(
-                        "Missing patches with present metadata flag; dtype: {}, patches_len: {}",
-                        dt,
-                        self.metadata().patches_len
-                    )
-                })
+            self.array().child(1, dt, self.len()).unwrap_or_else(|| {
+                panic!(
+                    "Missing patches with present metadata flag; dtype: {}, patches_len: {}",
+                    dt,
+                    self.len()
+                )
+            })
         })
     }
 

diff --git a/encodings/fastlanes/src/bitpacking/mod.rs b/encodings/fastlanes/src/bitpacking/mod.rs
@@ -18,10 +18,10 @@ impl_encoding!("fastlanes.bitpacked", 14u16, BitPacked);
 pub struct BitPackedMetadata {
     // TODO(ngates): serialize into compact form
     validity: ValidityMetadata,
-    patches_len: usize,
     bit_width: usize,
     offset: usize, // Know to be <1024
     length: usize, // Store end padding instead <1024
+    has_patches: bool,
 }
 
 /// NB: All non-null values in the patches array are considered patches
@@ -69,12 +69,23 @@ impl BitPackedArray {
             ));
         }
 
+        if let Some(parray) = patches.as_ref() {
+            if parray.len() != length {
+                vortex_bail!(
+                    "Mismatched length in BitPackedArray between encoded {} and it's patches({}) {}",
+                    length,
+                    parray.encoding().id(),
+                    parray.len()
+                )
+            }
+        }
+
         let metadata = BitPackedMetadata {
             validity: validity.to_metadata(length)?,
-            patches_len: patches.as_ref().map(|a| a.len()).unwrap_or_default(),
             offset,
             length,
             bit_width,
+            has_patches: patches.is_some(),
         };
 
         let mut children = Vec::with_capacity(3);
@@ -112,12 +123,12 @@ impl BitPackedArray {
 
     #[inline]
     pub fn patches(&self) -> Option<Array> {
-        (self.metadata().patches_len > 0)
+        (self.metadata().has_patches)
             .then(|| {
                 self.array().child(
                     1,
                     &self.dtype().with_nullability(Nullability::Nullable),
-                    self.metadata().patches_len,
+                    self.len(),
                 )
             })
             .flatten()
@@ -129,11 +140,7 @@ impl BitPackedArray {
     }
 
     pub fn validity(&self) -> Validity {
-        let validity_child_idx = if self.metadata().patches_len > 0 {
-            2
-        } else {
-            1
-        };
+        let validity_child_idx = if self.metadata().has_patches { 2 } else { 1 };
 
         self.metadata().validity.to_validity(self.array().child(
             validity_child_idx,