Expressions

Signed-off-by: Nicholas Gates <[email protected]>

Author: gatesn

vortex-array/src/compute/between.rs

@@ -2,6 +2,8 @@
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
 use std::any::Any;
+use std::fmt::Display;
+use std::fmt::Formatter;
 use std::sync::LazyLock;
 
 use arcref::ArcRef;
@@ -253,6 +255,22 @@ pub struct BetweenOptions {
     pub upper_strict: StrictComparison,
 }
 
+impl Display for BetweenOptions {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let lower_op = if self.lower_strict.is_strict() {
+            "<"
+        } else {
+            "<="
+        };
+        let upper_op = if self.upper_strict.is_strict() {
+            "<"
+        } else {
+            "<="
+        };
+        write!(f, "lower_strict: {}, upper_strict: {}", lower_op, upper_op)
+    }
+}
+
 impl Options for BetweenOptions {
     fn as_any(&self) -> &dyn Any {
         self

vortex-array/src/expr/bound.rs

@@ -12,11 +12,11 @@ use vortex_dtype::DType;
 use vortex_error::VortexResult;
 use vortex_utils::debug_with::DebugWith;
 
-use crate::expr::options::ExpressionOptions;
-use crate::expr::signature::ExpressionSignature;
 use crate::expr::ExprId;
 use crate::expr::ExprVTable;
 use crate::expr::VTable;
+use crate::expr::options::ExpressionOptions;
+use crate::expr::signature::ExpressionSignature;
 
 /// An instance of an expression bound to some invocation options.
 pub struct BoundExpression {

vortex-array/src/expr/expression.rs

@@ -11,15 +11,15 @@ use std::sync::Arc;
 
 use itertools::Itertools;
 use vortex_dtype::DType;
-use vortex_error::vortex_ensure;
 use vortex_error::VortexResult;
+use vortex_error::vortex_ensure;
 
+use crate::ArrayRef;
+use crate::expr::StatsCatalog;
+use crate::expr::VTable;
 use crate::expr::bound::BoundExpression;
 use crate::expr::display::DisplayTreeExpr;
 use crate::expr::stats::Stat;
-use crate::expr::StatsCatalog;
-use crate::expr::VTable;
-use crate::ArrayRef;
 
 /// A node in a Vortex expression tree.
 ///
@@ -142,13 +142,11 @@ impl Expression {
     }
 
     /// Returns an expression representing the zoned maximum statistic, if available.
-    #[deprecated(note = "Use `stat_expression` instead")]
     pub fn stat_min(&self, catalog: &dyn StatsCatalog) -> Option<Expression> {
         self.stat_expression(Stat::Min, catalog)
     }
 
     /// Returns an expression representing the zoned maximum statistic, if available.
-    #[deprecated(note = "Use `stat_expression` instead")]
     pub fn stat_max(&self, catalog: &dyn StatsCatalog) -> Option<Expression> {
         self.stat_expression(Stat::Max, catalog)
     }

vortex-array/src/expr/exprs/between.rs

@@ -6,23 +6,25 @@ use std::fmt::Formatter;
 use prost::Message;
 use vortex_dtype::DType;
 use vortex_dtype::DType::Bool;
-use vortex_error::vortex_bail;
 use vortex_error::VortexExpect;
 use vortex_error::VortexResult;
+use vortex_error::vortex_bail;
 use vortex_proto::expr as pb;
+use vortex_vector::Datum;
 
-use crate::compute::between as between_compute;
+use crate::ArrayRef;
 use crate::compute::BetweenOptions;
-use crate::expr::expression::Expression;
-use crate::expr::exprs::binary::Binary;
-use crate::expr::exprs::operators::Operator;
+use crate::compute::between as between_compute;
 use crate::expr::Arity;
 use crate::expr::ChildName;
+use crate::expr::ExecutionArgs;
 use crate::expr::ExprId;
 use crate::expr::StatsCatalog;
 use crate::expr::VTable;
 use crate::expr::VTableExt;
-use crate::ArrayRef;
+use crate::expr::expression::Expression;
+use crate::expr::exprs::binary::Binary;
+use crate::expr::exprs::operators::Operator;
 
 /// An optimized scalar expression to compute whether values fall between two bounds.
 ///
@@ -54,9 +56,9 @@ impl VTable for Between {
         ))
     }
 
-    fn deserialize(&self, metadata: &[u8]) -> VortexResult<Option<Self::Options>> {
+    fn deserialize(&self, metadata: &[u8]) -> VortexResult<Self::Options> {
         let opts = pb::BetweenOpts::decode(metadata)?;
-        Ok(Some(BetweenOptions {
+        Ok(BetweenOptions {
             lower_strict: if opts.lower_strict {
                 crate::compute::StrictComparison::Strict
             } else {
@@ -67,17 +69,11 @@ impl VTable for Between {
             } else {
                 crate::compute::StrictComparison::NonStrict
             },
-        }))
+        })
     }
 
-    fn validate(&self, expr: &ExpressionView<Self>) -> VortexResult<()> {
-        if expr.children().len() != 3 {
-            vortex_bail!(
-                "Between expression requires exactly 3 children, got {}",
-                expr.children().len()
-            );
-        }
-        Ok(())
+    fn arity(&self, _options: &Self::Options) -> Arity {
+        Arity::Exact(3)
     }
 
     fn child_name(&self, _instance: &Self::Options, child_idx: usize) -> ChildName {
@@ -89,8 +85,12 @@ impl VTable for Between {
         }
     }
 
-    fn fmt_sql(&self, expr: &ExpressionView<Self>, f: &mut Formatter<'_>) -> std::fmt::Result {
-        let options = expr.data();
+    fn fmt_sql(
+        &self,
+        options: &Self::Options,
+        expr: &Expression,
+        f: &mut Formatter<'_>,
+    ) -> std::fmt::Result {
         let lower_op = if options.lower_strict.is_strict() {
             "<"
         } else {
@@ -104,18 +104,18 @@ impl VTable for Between {
         write!(
             f,
             "({} {} {} {} {})",
-            expr.lower(),
+            expr.child(1),
             lower_op,
-            expr.child(),
+            expr.child(0),
             upper_op,
-            expr.upper()
+            expr.child(2)
         )
     }
 
-    fn return_dtype(&self, expr: &ExpressionView<Self>, scope: &DType) -> VortexResult<DType> {
-        let arr_dt = expr.child().return_dtype(scope)?;
-        let lower_dt = expr.lower().return_dtype(scope)?;
-        let upper_dt = expr.upper().return_dtype(scope)?;
+    fn return_dtype(&self, options: &Self::Options, arg_dtypes: &[DType]) -> VortexResult<DType> {
+        let arr_dt = &arg_dtypes[0];
+        let lower_dt = &arg_dtypes[1];
+        let upper_dt = &arg_dtypes[2];
 
         if !arr_dt.eq_ignore_nullability(&lower_dt) {
             vortex_bail!(
@@ -137,56 +137,45 @@ impl VTable for Between {
         ))
     }
 
-    fn evaluate(&self, expr: &ExpressionView<Self>, scope: &ArrayRef) -> VortexResult<ArrayRef> {
-        let arr = expr.child().evaluate(scope)?;
-        let lower = expr.lower().evaluate(scope)?;
-        let upper = expr.upper().evaluate(scope)?;
-        between_compute(&arr, &lower, &upper, expr.data())
+    fn evaluate(
+        &self,
+        options: &Self::Options,
+        expr: &Expression,
+        scope: &ArrayRef,
+    ) -> VortexResult<ArrayRef> {
+        let arr = expr.child(0).evaluate(scope)?;
+        let lower = expr.child(1).evaluate(scope)?;
+        let upper = expr.child(2).evaluate(scope)?;
+        between_compute(&arr, &lower, &upper, options)
+    }
+
+    fn execute(&self, data: &Self::Options, args: ExecutionArgs) -> VortexResult<Datum> {
+        todo!()
     }
 
     fn stat_falsification(
         &self,
-        _options: &Self::Options,
+        options: &Self::Options,
         expr: &Expression,
         catalog: &dyn StatsCatalog,
     ) -> Option<Expression> {
-        expr.to_binary_expr().stat_falsification(catalog)
-    }
-
-    fn is_null_sensitive(&self, _instance: &Self::Options) -> bool {
-        false
-    }
-
-    fn arity(&self, _options: &Self::Options) -> Arity {
-        Arity::Exact(3)
-    }
-}
-
-impl ExpressionView<'_, Between> {
-    pub fn child(&self) -> &Expression {
-        &self.children()[0]
-    }
-
-    pub fn lower(&self) -> &Expression {
-        &self.children()[1]
-    }
-
-    pub fn upper(&self) -> &Expression {
-        &self.children()[2]
-    }
-
-    pub fn to_binary_expr(&self) -> Expression {
-        let options = self.data();
-        let arr = self.children()[0].clone();
-        let lower = self.children()[1].clone();
-        let upper = self.children()[2].clone();
+        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().into(),
             [lower, arr.clone()],
         );
         let rhs = Binary.new_expr(options.upper_strict.to_operator().into(), [arr, upper]);
-        Binary.new_expr(Operator::And, [lhs, rhs])
+
+        Binary
+            .new_expr(Operator::And, [lhs, rhs])
+            .stat_falsification(catalog)
+    }
+
+    fn is_null_sensitive(&self, _instance: &Self::Options) -> bool {
+        false
     }
 }