review fixes part 1

spiraldb · Apr 22, 2024 · 588a015 · 588a015
1 parent 86a0a0f
commit 588a015
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 26 deletions.
diff --git a/vortex-alp/src/array.rs b/vortex-alp/src/array.rs
@@ -16,7 +16,7 @@ impl_encoding!("vortex.alp", ALP);
 pub struct ALPMetadata {
     exponents: Exponents,
     has_patches: bool,
-    dtype: DType,
+    encoded_dtype: DType,
 }
 
 impl ALPArray<'_> {
@@ -47,7 +47,7 @@ impl ALPArray<'_> {
             ALPMetadata {
                 exponents,
                 has_patches: patches.is_some(),
-                dtype: d_type,
+                encoded_dtype: d_type,
             },
             children.into(),
             Default::default(),
@@ -64,7 +64,7 @@ impl ALPArray<'_> {
 
     pub fn encoded(&self) -> Array {
         self.array()
-            .child(0, &self.metadata().dtype)
+            .child(0, &self.metadata().encoded_dtype)
             .expect("Missing encoded array")
     }
 
@@ -103,7 +103,7 @@ impl ArrayFlatten for ALPArray<'_> {
 impl AcceptArrayVisitor for ALPArray<'_> {
     fn accept(&self, visitor: &mut dyn ArrayVisitor) -> VortexResult<()> {
         visitor.visit_child("encoded", &self.encoded())?;
-        if self.metadata().has_patches {
+        if self.patches().is_some() {
             visitor.visit_child(
                 "patches",
                 &self.patches().expect("Expected patches to be present "),
@@ -117,6 +117,6 @@ impl ArrayStatisticsCompute for ALPArray<'_> {}
 
 impl ArrayTrait for ALPArray<'_> {
     fn len(&self) -> usize {
-        self.array().len()
+        self.encoded().len()
     }
 }
diff --git a/vortex-alp/src/compress.rs b/vortex-alp/src/compress.rs
@@ -9,7 +9,7 @@ use vortex_error::{vortex_bail, vortex_err, VortexResult};
 
 use crate::alp::ALPFloat;
 use crate::array::{ALPArray, ALPEncoding};
-use crate::Exponents;
+use crate::{Exponents, OwnedALPArray};
 
 #[macro_export]
 macro_rules! match_each_alp_float_ptype {
@@ -33,7 +33,7 @@ impl EncodingCompression for ALPEncoding {
         _config: &CompressConfig,
     ) -> Option<&dyn EncodingCompression> {
         // Only support primitive arrays
-        let parray = array.clone().flatten_primitive().unwrap();
+        let parray = PrimitiveArray::try_from(array).ok()?;
 
         // Only supports f32 and f64
         if !matches!(parray.ptype(), PType::F32 | PType::F64) {
@@ -50,17 +50,16 @@ impl EncodingCompression for ALPEncoding {
         ctx: CompressCtx,
     ) -> VortexResult<Array<'static>> {
         let like_alp = like.map(|like_array| like_array.as_array_ref());
-        let exponents_cloned = like
+        let like_exponents = like
             .map(|like_array| ALPArray::try_from(like_array).unwrap())
-            .as_ref()
             .map(|a| a.exponents().clone());
 
         // TODO(ngates): fill forward nulls
-        let parray = array.clone().flatten_primitive()?;
+        let parray = PrimitiveArray::try_from(array)?;
 
         let (exponents, encoded, patches) = match_each_alp_float_ptype!(
             parray.ptype(), |$T| {
-            encode_to_array::<$T>(&parray, exponents_cloned.as_ref())
+            encode_to_array::<$T>(&parray, like_exponents.as_ref())
         })?;
 
         let compressed_encoded = ctx
@@ -105,7 +104,7 @@ where
     )
 }
 
-pub(crate) fn alp_encode(parray: &PrimitiveArray) -> VortexResult<ALPArray<'static>> {
+pub(crate) fn alp_encode(parray: &PrimitiveArray) -> VortexResult<OwnedALPArray> {
     let (exponents, encoded, patches) = match parray.ptype() {
         PType::F32 => encode_to_array::<f32>(parray, None),
         PType::F64 => encode_to_array::<f64>(parray, None),
@@ -169,10 +168,7 @@ mod tests {
         let encoded = alp_encode(&array).unwrap();
         assert!(encoded.patches().is_none());
         assert_eq!(
-            encoded
-                .encoded()
-                .clone()
-                .flatten_primitive()
+            PrimitiveArray::try_from(encoded.encoded())
                 .unwrap()
                 .typed_data::<i32>(),
             vec![1234; 1025]
@@ -190,10 +186,7 @@ mod tests {
         println!("Encoded {:?}", encoded);
         assert!(encoded.patches().is_none());
         assert_eq!(
-            encoded
-                .encoded()
-                .clone()
-                .flatten_primitive()
+            PrimitiveArray::try_from(encoded.encoded())
                 .unwrap()
                 .typed_data::<i32>(),
             vec![0, 1234, 0]

diff --git a/vortex-array/src/array/primitive/mod.rs b/vortex-array/src/array/primitive/mod.rs
@@ -8,7 +8,7 @@ use crate::buffer::Buffer;
 use crate::ptype::{NativePType, PType};
 use crate::validity::{ArrayValidity, LogicalValidity, Validity, ValidityMetadata};
 use crate::visitor::{AcceptArrayVisitor, ArrayVisitor};
-use crate::{impl_encoding, ArrayDType};
+use crate::{impl_encoding, ArrayDType, ToStatic};
 use crate::{match_each_native_ptype, ArrayFlatten};
 
 mod accessor;
@@ -22,7 +22,7 @@ pub struct PrimitiveMetadata {
     validity: ValidityMetadata,
 }
 
-impl<'a> PrimitiveArray<'a> {
+impl PrimitiveArray<'_> {
     pub fn try_new<T: NativePType + ArrowNativeType>(
         buffer: ScalarBuffer<T>,
         validity: Validity,
@@ -115,8 +115,8 @@ impl<'a> PrimitiveArray<'a> {
         Self::try_new(ScalarBuffer::from(own_values), validity)
     }
 
-    pub fn into_buffer(self) -> Buffer<'a> {
-        self.into_array().into_buffer().unwrap()
+    pub fn into_buffer(self) -> Buffer<'static> {
+        self.into_array().into_buffer().unwrap().to_static()
     }
 }
 

diff --git a/vortex-array/src/flatten.rs b/vortex-array/src/flatten.rs
@@ -37,8 +37,7 @@ impl<'a> Array<'a> {
     }
 
     pub fn flatten_primitive(self) -> VortexResult<PrimitiveArray<'a>> {
-        let result = self.clone().flatten()?;
-        PrimitiveArray::try_from(result.into_array())
+        PrimitiveArray::try_from(self.flatten()?.into_array())
     }
 
     pub fn flatten_varbin(self) -> VortexResult<VarBinArray<'a>> {