Remove legacy stat falsification hooks

Signed-off-by: Nicholas Gates <nick@nickgates.com>

Author: gatesn

vortex-array/src/expr/expression.rs

@@ -114,28 +114,6 @@ impl Expression {
         self.scalar_fn.validity(self)
     }
 
-    /// An expression over zone-statistics which implies all records in the zone evaluate to false.
-    ///
-    /// Given an expression, `e`, if `e.stat_falsification(..)` evaluates to true, it is guaranteed
-    /// that `e` evaluates to false on all records in the zone. However, the inverse is not
-    /// necessarily true: even if the falsification evaluates to false, `e` need not evaluate to
-    /// true on all records.
-    ///
-    /// The [`StatsCatalog`] can be used to constrain or rename stats used in the final expr.
-    ///
-    /// # Examples
-    ///
-    /// - An expression over one variable: `x > 0` is false for all records in a zone if the maximum
-    ///   value of the column `x` in that zone is less than or equal to zero: `max(x) <= 0`.
-    /// - An expression over two variables: `x > y` becomes `max(x) <= min(y)`.
-    /// - A conjunctive expression: `x > y AND z < x` becomes `max(x) <= min(y) OR min(z) >= max(x).
-    ///
-    /// Some expressions, in theory, have falsifications but this function does not support them
-    /// such as `x < (y < z)` or `x LIKE "needle%"`.
-    pub fn stat_falsification(&self, catalog: &dyn StatsCatalog) -> Option<Expression> {
-        self.scalar_fn().stat_falsification(self, catalog)
-    }
-
     /// Returns an expression that proves this predicate is definitely false from stats.
     ///
     /// `scope` is the dtype of the row this expression evaluates over.

vortex-array/src/expr/pruning/pruning_expr.rs

@@ -1,15 +1,11 @@
 // SPDX-License-Identifier: Apache-2.0
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
-#[cfg(test)]
-use std::cell::RefCell;
 use std::iter;
 
 use itertools::Itertools;
 use vortex_error::VortexResult;
 use vortex_session::VortexSession;
-#[cfg(test)]
-use vortex_utils::aliases::hash_map::HashMap;
 use vortex_utils::aliases::hash_set::HashSet;
 
 use super::relation::Relation;
@@ -19,8 +15,6 @@ use crate::dtype::FieldName;
 use crate::dtype::FieldPath;
 use crate::dtype::FieldPathSet;
 use crate::expr::Expression;
-#[cfg(test)]
-use crate::expr::StatsCatalog;
 use crate::expr::analysis::referenced_field_paths;
 use crate::expr::get_item;
 use crate::expr::is_root;
@@ -34,27 +28,6 @@ use crate::stats::bind::bind_stats;
 
 pub type RequiredStats = Relation<FieldPath, Stat>;
 
-// A catalog that returns a stat column whenever it is required, tracking all accessed
-// stats and returning them later.
-#[cfg(test)]
-#[derive(Default)]
-pub(crate) struct TrackingStatsCatalog {
-    usage: RefCell<HashMap<(FieldPath, Stat), Expression>>,
-}
-
-#[cfg(test)]
-impl StatsCatalog for TrackingStatsCatalog {
-    fn stats_ref(&self, field_path: &FieldPath, stat: Stat) -> Option<Expression> {
-        let mut expr = root();
-        let name = field_path_stat_field_name(field_path, stat);
-        expr = get_item(name, expr);
-        self.usage
-            .borrow_mut()
-            .insert((field_path.clone(), stat), expr.clone());
-        Some(expr)
-    }
-}
-
 #[doc(hidden)]
 pub fn field_path_stat_field_name(field_path: &FieldPath, stat: Stat) -> FieldName {
     field_path

vortex-array/src/scalar_fn/erased.rs

@@ -181,15 +181,6 @@ impl ScalarFnRef {
         self.0.simplify_untyped(expr)
     }
 
-    /// Compute stat falsification expression.
-    pub(crate) fn stat_falsification(
-        &self,
-        expr: &Expression,
-        catalog: &dyn StatsCatalog,
-    ) -> Option<Expression> {
-        self.0.stat_falsification(expr, catalog)
-    }
-
     /// Compute stat expression.
     pub(crate) fn stat_expression(
         &self,

vortex-array/src/scalar_fn/fns/between/mod.rs

@@ -25,16 +25,13 @@ use crate::arrays::Primitive;
 use crate::builtins::ArrayBuiltins;
 use crate::dtype::DType;
 use crate::dtype::DType::Bool;
-use crate::expr::StatsCatalog;
 use crate::expr::expression::Expression;
 use crate::scalar::Scalar;
 use crate::scalar_fn::Arity;
 use crate::scalar_fn::ChildName;
 use crate::scalar_fn::ExecutionArgs;
 use crate::scalar_fn::ScalarFnId;
 use crate::scalar_fn::ScalarFnVTable;
-use crate::scalar_fn::ScalarFnVTableExt;
-use crate::scalar_fn::fns::binary::Binary;
 use crate::scalar_fn::fns::binary::execute_boolean;
 use crate::scalar_fn::fns::operators::CompareOperator;
 use crate::scalar_fn::fns::operators::Operator;
@@ -298,22 +295,6 @@ impl ScalarFnVTable for Between {
         between_canonical(&arr, &lower, &upper, options, ctx)
     }
 
-    fn stat_falsification(
-        &self,
-        options: &Self::Options,
-        expr: &Expression,
-        catalog: &dyn StatsCatalog,
-    ) -> Option<Expression> {
-        let arr = expr.child(0).clone();
-        let lower = expr.child(1).clone();
-        let upper = expr.child(2).clone();
-
-        let lhs = Binary.new_expr(options.lower_strict.to_operator(), [lower, arr.clone()]);
-        let rhs = Binary.new_expr(options.upper_strict.to_operator(), [arr, upper]);
-
-        and(lhs, rhs).stat_falsification(catalog)
-    }
-
     fn validity(
         &self,
         _options: &Self::Options,

vortex-array/src/scalar_fn/fns/binary/mod.rs

@@ -18,18 +18,9 @@ use crate::ArrayRef;
 use crate::ExecutionCtx;
 use crate::dtype::DType;
 use crate::dtype::Nullability;
-use crate::expr::StatsCatalog;
 use crate::expr::and;
-use crate::expr::and_collect;
-use crate::expr::eq;
 use crate::expr::expression::Expression;
-use crate::expr::gt;
-use crate::expr::gt_eq;
 use crate::expr::lit;
-use crate::expr::lt;
-use crate::expr::lt_eq;
-use crate::expr::or_collect;
-use crate::expr::stats::Stat;
 use crate::scalar_fn::Arity;
 use crate::scalar_fn::ChildName;
 use crate::scalar_fn::ExecutionArgs;
@@ -221,110 +212,6 @@ impl ScalarFnVTable for Binary {
         })
     }
 
-    fn stat_falsification(
-        &self,
-        operator: &Operator,
-        expr: &Expression,
-        catalog: &dyn StatsCatalog,
-    ) -> Option<Expression> {
-        // Wrap another predicate with an optional NaNCount check, if the stat is available.
-        //
-        // For example, regular pruning conversion for `A >= B` would be
-        //
-        //      A.max < B.min
-        //
-        // With NaN predicate introduction, we'd conjunct it with a check for NaNCount, resulting
-        // in:
-        //
-        //      (A.nan_count = 0) AND (B.nan_count = 0) AND A.max < B.min
-        //
-        // Non-floating point column and literal expressions should be unaffected as they do not
-        // have a nan_count statistic defined.
-        fn with_nan_predicate(
-            lhs: &Expression,
-            rhs: &Expression,
-            value_predicate: Expression,
-            catalog: &dyn StatsCatalog,
-        ) -> Expression {
-            let nan_predicate = and_collect(
-                lhs.stat_expression(Stat::NaNCount, catalog)
-                    .into_iter()
-                    .chain(rhs.stat_expression(Stat::NaNCount, catalog))
-                    .map(|nans| eq(nans, lit(0u64))),
-            );
-
-            if let Some(nan_check) = nan_predicate {
-                and(nan_check, value_predicate)
-            } else {
-                value_predicate
-            }
-        }
-
-        let lhs = expr.child(0);
-        let rhs = expr.child(1);
-        match operator {
-            Operator::Eq => {
-                let min_lhs = lhs.stat_min(catalog);
-                let max_lhs = lhs.stat_max(catalog);
-
-                let min_rhs = rhs.stat_min(catalog);
-                let max_rhs = rhs.stat_max(catalog);
-
-                let left = min_lhs.zip(max_rhs).map(|(a, b)| gt(a, b));
-                let right = min_rhs.zip(max_lhs).map(|(a, b)| gt(a, b));
-
-                let min_max_check = or_collect(left.into_iter().chain(right))?;
-
-                // NaN is not captured by the min/max stat, so we must check NaNCount before pruning
-                Some(with_nan_predicate(lhs, rhs, min_max_check, catalog))
-            }
-            Operator::NotEq => {
-                let min_lhs = lhs.stat_min(catalog)?;
-                let max_lhs = lhs.stat_max(catalog)?;
-
-                let min_rhs = rhs.stat_min(catalog)?;
-                let max_rhs = rhs.stat_max(catalog)?;
-
-                let min_max_check = and(eq(min_lhs, max_rhs), eq(max_lhs, min_rhs));
-
-                Some(with_nan_predicate(lhs, rhs, min_max_check, catalog))
-            }
-            Operator::Gt => {
-                let min_max_check = lt_eq(lhs.stat_max(catalog)?, rhs.stat_min(catalog)?);
-
-                Some(with_nan_predicate(lhs, rhs, min_max_check, catalog))
-            }
-            Operator::Gte => {
-                // NaN is not captured by the min/max stat, so we must check NaNCount before pruning
-                let min_max_check = lt(lhs.stat_max(catalog)?, rhs.stat_min(catalog)?);
-
-                Some(with_nan_predicate(lhs, rhs, min_max_check, catalog))
-            }
-            Operator::Lt => {
-                // NaN is not captured by the min/max stat, so we must check NaNCount before pruning
-                let min_max_check = gt_eq(lhs.stat_min(catalog)?, rhs.stat_max(catalog)?);
-
-                Some(with_nan_predicate(lhs, rhs, min_max_check, catalog))
-            }
-            Operator::Lte => {
-                // NaN is not captured by the min/max stat, so we must check NaNCount before pruning
-                let min_max_check = gt(lhs.stat_min(catalog)?, rhs.stat_max(catalog)?);
-
-                Some(with_nan_predicate(lhs, rhs, min_max_check, catalog))
-            }
-            Operator::And => or_collect(
-                lhs.stat_falsification(catalog)
-                    .into_iter()
-                    .chain(rhs.stat_falsification(catalog)),
-            ),
-            Operator::Or => Some(and(
-                lhs.stat_falsification(catalog)?,
-                rhs.stat_falsification(catalog)?,
-            )),
-            Operator::Add | Operator::Sub | Operator::Mul | Operator::Div => None,
-        }
-    }
-
     fn validity(
         &self,
         operator: &Operator,
@@ -381,8 +268,12 @@ mod tests {
     use crate::expr::Expression;
     use crate::expr::and_collect;
     use crate::expr::col;
+    use crate::expr::eq;
+    use crate::expr::gt;
+    use crate::expr::gt_eq;
     use crate::expr::lit;
     use crate::expr::lt;
+    use crate::expr::lt_eq;
     use crate::expr::not_eq;
     use crate::expr::or;
     use crate::expr::or_collect;

vortex-array/src/scalar_fn/fns/dynamic.rs

@@ -20,7 +20,6 @@ use crate::IntoArray;
 use crate::arrays::ConstantArray;
 use crate::dtype::DType;
 use crate::expr::Expression;
-use crate::expr::StatsCatalog;
 use crate::expr::traversal::NodeExt;
 use crate::expr::traversal::NodeVisitor;
 use crate::expr::traversal::TraversalOrder;
@@ -120,50 +119,6 @@ impl ScalarFnVTable for DynamicComparison {
         .into_array())
     }
 
-    fn stat_falsification(
-        &self,
-        dynamic: &DynamicComparisonExpr,
-        expr: &Expression,
-        catalog: &dyn StatsCatalog,
-    ) -> Option<Expression> {
-        let lhs = expr.child(0);
-        match dynamic.operator {
-            CompareOperator::Eq | CompareOperator::NotEq => None,
-            CompareOperator::Gt => Some(DynamicComparison.new_expr(
-                DynamicComparisonExpr {
-                    operator: CompareOperator::Lte,
-                    rhs: Arc::clone(&dynamic.rhs),
-                    default: !dynamic.default,
-                },
-                vec![lhs.stat_max(catalog)?],
-            )),
-            CompareOperator::Gte => Some(DynamicComparison.new_expr(
-                DynamicComparisonExpr {
-                    operator: CompareOperator::Lt,
-                    rhs: Arc::clone(&dynamic.rhs),
-                    default: !dynamic.default,
-                },
-                vec![lhs.stat_max(catalog)?],
-            )),
-            CompareOperator::Lt => Some(DynamicComparison.new_expr(
-                DynamicComparisonExpr {
-                    operator: CompareOperator::Gte,
-                    rhs: Arc::clone(&dynamic.rhs),
-                    default: !dynamic.default,
-                },
-                vec![lhs.stat_min(catalog)?],
-            )),
-            CompareOperator::Lte => Some(DynamicComparison.new_expr(
-                DynamicComparisonExpr {
-                    operator: CompareOperator::Gt,
-                    rhs: Arc::clone(&dynamic.rhs),
-                    default: !dynamic.default,
-                },
-                vec![lhs.stat_min(catalog)?],
-            )),
-        }
-    }
-
     // Defer to the child
     fn is_null_sensitive(&self, _instance: &Self::Options) -> bool {
         false

vortex-array/src/scalar_fn/fns/is_not_null.rs

@@ -3,7 +3,6 @@
 
 use std::fmt::Formatter;
 
-use vortex_array::scalar_fn::internal::row_count::RowCount;
 use vortex_error::VortexResult;
 use vortex_session::VortexSession;
 use vortex_session::registry::CachedId;
@@ -15,16 +14,12 @@ use crate::arrays::ConstantArray;
 use crate::dtype::DType;
 use crate::dtype::Nullability;
 use crate::expr::Expression;
-use crate::expr::StatsCatalog;
-use crate::expr::eq;
-use crate::expr::stats::Stat;
 use crate::scalar_fn::Arity;
 use crate::scalar_fn::ChildName;
 use crate::scalar_fn::EmptyOptions;
 use crate::scalar_fn::ExecutionArgs;
 use crate::scalar_fn::ScalarFnId;
 use crate::scalar_fn::ScalarFnVTable;
-use crate::scalar_fn::ScalarFnVTableExt;
 use crate::validity::Validity;
 
 /// Expression that checks for non-null values.
@@ -100,18 +95,6 @@ impl ScalarFnVTable for IsNotNull {
     fn is_fallible(&self, _instance: &Self::Options) -> bool {
         false
     }
-
-    fn stat_falsification(
-        &self,
-        _options: &Self::Options,
-        expr: &Expression,
-        catalog: &dyn StatsCatalog,
-    ) -> Option<Expression> {
-        // is_not_null is falsified when ALL values are null, i.e. null_count == row_count.
-        let child = expr.child(0);
-        let null_count_expr = child.stat_expression(Stat::NullCount, catalog)?;
-        Some(eq(null_count_expr, RowCount.new_expr(EmptyOptions, [])))
-    }
 }
 
 #[cfg(test)]