Use more stats in the btrblocks compressor (#2720)

taken out of #2718

Author: AdamGS

vortex-btrblocks/src/float/stats.rs

@@ -135,11 +135,12 @@ where
         };
     }
 
-    let validity = array.validity_mask().vortex_expect("logical_validity");
-    let null_count = validity.false_count();
-    let value_count = validity.true_count();
-    let mut min = T::max_value();
-    let mut max = T::min_value();
+    let null_count = array
+        .statistics()
+        .compute_null_count()
+        .vortex_expect("null count");
+    let value_count = array.len() - null_count;
+
     // Keep a HashMap of T, then convert the keys into PValue afterward since value is
     // so much more efficient to hash and search for.
     let mut distinct_values = if count_distinct_values {
@@ -148,6 +149,8 @@ where
         HashMap::with_hasher(FxBuildHasher)
     };
 
+    let validity = array.validity_mask().vortex_expect("logical_validity");
+
     let mut runs = 1;
     let head_idx = validity
         .first()
@@ -159,9 +162,6 @@ where
     match validity.boolean_buffer() {
         AllOr::All => {
             for value in first_valid_buff {
-                min = min.min(value);
-                max = max.max(value);
-
                 if count_distinct_values {
                     *distinct_values.entry(value.to_bits()).or_insert(0) += 1;
                 }
@@ -179,9 +179,6 @@ where
                 .zip_eq(v.slice(head_idx, array.len() - head_idx).iter())
             {
                 if valid {
-                    min = min.min(value);
-                    max = max.max(value);
-
                     if count_distinct_values {
                         *distinct_values.entry(value.to_bits()).or_insert(0) += 1;
                     }

vortex-btrblocks/src/integer/stats.rs

@@ -5,12 +5,13 @@ use num_traits::PrimInt;
 use rustc_hash::FxBuildHasher;
 use vortex_array::aliases::hash_map::HashMap;
 use vortex_array::arrays::PrimitiveArray;
+use vortex_array::stats::Stat;
 use vortex_array::variants::PrimitiveArrayTrait;
 use vortex_array::{Array, ToCanonical};
 use vortex_dtype::{NativePType, match_each_integer_ptype};
-use vortex_error::{VortexExpect, VortexUnwrap};
+use vortex_error::{VortexError, VortexExpect, VortexUnwrap};
 use vortex_mask::AllOr;
-use vortex_scalar::PValue;
+use vortex_scalar::{PValue, ScalarValue};
 
 use crate::sample::sample;
 use crate::{CompressorStats, GenerateStatsOptions};
@@ -150,11 +151,9 @@ impl CompressorStats for IntegerStats {
     }
 }
 
-fn typed_int_stats<T: NativePType + Hash + PrimInt>(
-    array: &PrimitiveArray,
-    count_distinct_values: bool,
-) -> IntegerStats
+fn typed_int_stats<T>(array: &PrimitiveArray, count_distinct_values: bool) -> IntegerStats
 where
+    T: NativePType + Hash + PrimInt + for<'a> TryFrom<&'a ScalarValue, Error = VortexError>,
     TypedStats<T>: Into<ErasedStats>,
 {
     // Special case: empty array
@@ -204,8 +203,6 @@ where
     let head = buffer[head_idx];
 
     let mut loop_state = LoopState {
-        min: head,
-        max: head,
         distinct_values: if count_distinct_values {
             HashMap::with_capacity_and_hasher(array.len() / 2, FxBuildHasher)
         } else {
@@ -281,9 +278,19 @@ where
         u32::MAX
     };
 
+    let min = array
+        .statistics()
+        .compute_as::<T>(Stat::Min)
+        .vortex_expect("min should be computed");
+
+    let max = array
+        .statistics()
+        .compute_as::<T>(Stat::Max)
+        .vortex_expect("max should be computed");
+
     let typed = TypedStats {
-        min: loop_state.min,
-        max: loop_state.max,
+        min,
+        max,
         distinct_values: loop_state.distinct_values,
         top_value,
         top_count,
@@ -307,8 +314,6 @@ where
 }
 
 struct LoopState<T> {
-    min: T,
-    max: T,
     prev: T,
     runs: u32,
     distinct_values: HashMap<T, u32, FxBuildHasher>,
@@ -321,9 +326,6 @@ fn inner_loop_nonnull<T: PrimInt + Hash>(
     state: &mut LoopState<T>,
 ) {
     for &value in values {
-        state.min = state.min.min(value);
-        state.max = state.max.max(value);
-
         if count_distinct_values {
             *state.distinct_values.entry(value).or_insert(0) += 1;
         }
@@ -344,9 +346,6 @@ fn inner_loop_nullable<T: PrimInt + Hash>(
 ) {
     for (idx, &value) in values.iter().enumerate() {
         if is_valid.value(idx) {
-            state.min = state.min.min(value);
-            state.max = state.max.max(value);
-
             if count_distinct_values {
                 *state.distinct_values.entry(value).or_insert(0) += 1;
             }
@@ -368,9 +367,6 @@ fn inner_loop_naive<T: PrimInt + Hash>(
 ) {
     for (idx, &value) in values.iter().enumerate() {
         if is_valid.value(idx) {
-            state.min = state.min.min(value);
-            state.max = state.max.max(value);
-
             if count_distinct_values {
                 *state.distinct_values.entry(value).or_insert(0) += 1;
             }

vortex-btrblocks/src/string.rs

@@ -29,15 +29,15 @@ fn estimate_distinct_count(strings: &VarBinViewArray) -> u32 {
     // Iterate the views. Two strings which are equal must have the same first 8-bytes.
     // NOTE: there are cases where this performs pessimally, e.g. when we have strings that all
     // share a 4-byte prefix and have the same length.
-    let mut disinct = HashSet::with_capacity(views.len() / 2);
+    let mut distinct = HashSet::with_capacity(views.len() / 2);
     views.iter().for_each(|&view| {
         // SAFETY: we're doing bitwise manipulations. Did you expect that to be safe??
         let view_u128: u128 = unsafe { std::mem::transmute(view) };
         let len_and_prefix = view_u128 as u64;
-        disinct.insert(len_and_prefix);
+        distinct.insert(len_and_prefix);
     });
 
-    disinct
+    distinct
         .len()
         .try_into()
         .vortex_expect("distinct count must fit in u32")
@@ -48,9 +48,9 @@ impl CompressorStats for StringStats {
 
     fn generate_opts(input: &Self::ArrayType, opts: GenerateStatsOptions) -> Self {
         let null_count = input
-            .validity()
-            .null_count(input.len())
-            .vortex_expect("null_count");
+            .statistics()
+            .compute_null_count()
+            .vortex_expect("null count");
         let value_count = input.len() - null_count;
         let estimated_distinct = if opts.count_distinct_values {
             estimate_distinct_count(input)