Merge branch 'develop' into jc/ipc-stats

spiraldb · May 8, 2024 · 2bfcb46 · 2bfcb46
2 parents 19ec4fb + c09d2d6
commit 2bfcb46
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 104 deletions.
diff --git a/vortex-array/src/data.rs b/vortex-array/src/data.rs
@@ -2,7 +2,7 @@ use std::sync::{Arc, RwLock};
 
 use vortex_buffer::Buffer;
 use vortex_dtype::DType;
-use vortex_error::{vortex_err, VortexResult};
+use vortex_error::VortexResult;
 use vortex_scalar::Scalar;
 
 use crate::encoding::EncodingRef;
@@ -162,35 +162,4 @@ impl Statistics for ArrayData {
         );
         self.get(stat)
     }
-
-    #[inline]
-    fn with_stat_value<'a>(
-        &self,
-        stat: Stat,
-        f: &'a mut dyn FnMut(&Scalar) -> VortexResult<()>,
-    ) -> VortexResult<()> {
-        self.stats_map
-            .read()
-            .unwrap()
-            .get(stat)
-            .ok_or_else(|| vortex_err!(ComputeError: "statistic {} missing", stat))
-            .and_then(f)
-    }
-
-    #[inline]
-    fn with_computed_stat_value<'a>(
-        &self,
-        stat: Stat,
-        f: &'a mut dyn FnMut(&Scalar) -> VortexResult<()>,
-    ) -> VortexResult<()> {
-        if let Some(s) = self.stats_map.read().unwrap().get(stat) {
-            return f(s);
-        }
-
-        self.stats_map
-            .write()
-            .unwrap()
-            .extend(self.to_array().with_dyn(|a| a.compute_statistics(stat))?);
-        self.with_stat_value(stat, f)
-    }
 }
diff --git a/vortex-array/src/stats/mod.rs b/vortex-array/src/stats/mod.rs
@@ -3,8 +3,9 @@ use std::hash::Hash;
 
 use enum_iterator::Sequence;
 pub use statsset::*;
+use vortex_dtype::Nullability::NonNullable;
 use vortex_dtype::{DType, NativePType};
-use vortex_error::{VortexError, VortexResult};
+use vortex_error::{vortex_err, VortexError, VortexResult};
 use vortex_scalar::Scalar;
 
 mod statsset;
@@ -51,20 +52,6 @@ pub trait Statistics {
 
     /// Computes the value of the stat if it's not present
     fn compute(&self, stat: Stat) -> Option<Scalar>;
-
-    /// Applies the given function to the statistic if it's present
-    fn with_stat_value<'a>(
-        &self,
-        stat: Stat,
-        f: &'a mut dyn FnMut(&Scalar) -> VortexResult<()>,
-    ) -> VortexResult<()>;
-
-    /// Applies the given function to the statistic, computing it if it's not already present
-    fn with_computed_stat_value<'a>(
-        &self,
-        stat: Stat,
-        f: &'a mut dyn FnMut(&Scalar) -> VortexResult<()>,
-    ) -> VortexResult<()>;
 }
 
 pub struct EmptyStatistics;
@@ -83,24 +70,6 @@ impl Statistics for EmptyStatistics {
     fn compute(&self, _stat: Stat) -> Option<Scalar> {
         None
     }
-
-    #[inline]
-    fn with_stat_value<'a>(
-        &self,
-        _stat: Stat,
-        _f: &'a mut dyn FnMut(&Scalar) -> VortexResult<()>,
-    ) -> VortexResult<()> {
-        Ok(())
-    }
-
-    #[inline]
-    fn with_computed_stat_value<'a>(
-        &self,
-        _stat: Stat,
-        _f: &'a mut dyn FnMut(&Scalar) -> VortexResult<()>,
-    ) -> VortexResult<()> {
-        Ok(())
-    }
 }
 
 pub trait ArrayStatistics {
@@ -119,36 +88,28 @@ impl dyn Statistics + '_ {
         &self,
         stat: Stat,
     ) -> VortexResult<U> {
-        let mut res: Option<U> = None;
-        self.with_stat_value(stat, &mut |s| {
-            res = Some(U::try_from(s)?);
-            Ok(())
-        })?;
-        Ok(res.expect("Result should have been populated by previous call"))
+        self.get(stat)
+            .ok_or_else(|| vortex_err!(ComputeError: "statistic {} missing", stat))
+            .and_then(|s| U::try_from(&s))
     }
 
     pub fn compute_as<U: for<'a> TryFrom<&'a Scalar, Error = VortexError>>(
         &self,
         stat: Stat,
     ) -> VortexResult<U> {
-        let mut res: Option<U> = None;
-        self.with_computed_stat_value(stat, &mut |s| {
-            res = Some(U::try_from(s)?);
-            Ok(())
-        })?;
-        Ok(res.expect("Result should have been populated by previous call"))
+        self.compute(stat)
+            .ok_or_else(|| vortex_err!(ComputeError: "statistic {} missing", stat))
+            .and_then(|s| U::try_from(&s))
     }
 
     pub fn compute_as_cast<U: NativePType + for<'a> TryFrom<&'a Scalar, Error = VortexError>>(
         &self,
         stat: Stat,
     ) -> VortexResult<U> {
-        let mut res: Option<U> = None;
-        self.with_computed_stat_value(stat, &mut |s| {
-            res = Some(U::try_from(s.cast(&DType::from(U::PTYPE))?.as_ref())?);
-            Ok(())
-        })?;
-        Ok(res.expect("Result should have been populated by previous call"))
+        self.compute(stat)
+            .ok_or_else(|| vortex_err!(ComputeError: "statistic {} missing", stat))
+            .and_then(|s| s.cast(&DType::Primitive(U::PTYPE, NonNullable)))
+            .and_then(|s| U::try_from(s.as_ref()))
     }
 
     pub fn compute_min<U: for<'a> TryFrom<&'a Scalar, Error = VortexError>>(

diff --git a/vortex-array/src/view.rs b/vortex-array/src/view.rs
@@ -216,27 +216,6 @@ impl Statistics for ArrayView<'_> {
 
         self.get(stat)
     }
-
-    fn with_stat_value<'a>(
-        &self,
-        stat: Stat,
-        f: &'a mut dyn FnMut(&Scalar) -> VortexResult<()>,
-    ) -> VortexResult<()> {
-        if let Some(existing) = self.get(stat) {
-            return f(&existing);
-        }
-        vortex_bail!(ComputeError: "statistic {} missing", stat);
-    }
-
-    fn with_computed_stat_value<'a>(
-        &self,
-        stat: Stat,
-        f: &'a mut dyn FnMut(&Scalar) -> VortexResult<()>,
-    ) -> VortexResult<()> {
-        self.compute(stat)
-            .map(|s| f(&s))
-            .unwrap_or_else(|| vortex_bail!(ComputeError: "statistic {} missing", stat))
-    }
 }
 
 impl ToArray for ArrayView<'_> {