Clarify docs in session accessors

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

Author: gatesn

vortex-array/src/arrays/filter/kernel.rs

@@ -30,10 +30,6 @@ impl Kernel for FilterKernel {
         Ok(Filter::filter(&self.child.execute()?, &self.mask))
     }
 
-    fn cost_estimate(&self, selection: &Mask) -> f64 {
-        cost_for_dtype(&self.dtype, selection)
-    }
-
     fn push_down_filter(self: Box<Self>, selection: &Mask) -> VortexResult<PushDownResult> {
         let new_mask = self.mask.intersect_by_rank(selection);
         Ok(match self.child.push_down_filter(&new_mask)? {
@@ -46,23 +42,3 @@ impl Kernel for FilterKernel {
         })
     }
 }
-
-fn cost_for_dtype(dtype: &DType, selection: &Mask) -> f64 {
-    if selection.true_count() == 0 {
-        return 0.0;
-    }
-
-    match dtype {
-        DType::Null => 0.0,
-        DType::Primitive(ptype, _) => {
-            // Some estimate based on PType width and true-count or number of slices??
-            // Maybe nullable types are more expensive?
-            // ... No idea how to tune this yet.
-            let width = ptype.byte_width() as f64;
-            let selectivity = selection.true_count() as f64 / selection.len() as f64;
-            width * selectivity
-        }
-        DType::Extension(ext) => cost_for_dtype(ext.storage_dtype(), selection),
-        _ => f64::INFINITY,
-    }
-}

vortex-array/src/arrays/filter/vtable.rs

@@ -7,15 +7,24 @@ use std::ops::Range;
 use vortex_buffer::BufferHandle;
 use vortex_compute::filter::Filter;
 use vortex_dtype::DType;
-use vortex_error::vortex_bail;
 use vortex_error::VortexExpect;
 use vortex_error::VortexResult;
+use vortex_error::vortex_bail;
 use vortex_mask::Mask;
 use vortex_scalar::Scalar;
 
+use crate::Array;
+use crate::ArrayBufferVisitor;
+use crate::ArrayChildVisitor;
+use crate::ArrayEq;
+use crate::ArrayHash;
+use crate::ArrayRef;
+use crate::Canonical;
+use crate::IntoArray;
+use crate::Precision;
+use crate::arrays::LEGACY_SESSION;
 use crate::arrays::filter::array::FilterArray;
 use crate::arrays::filter::kernel::FilterKernel;
-use crate::arrays::LEGACY_SESSION;
 use crate::kernel::BindCtx;
 use crate::kernel::KernelRef;
 use crate::kernel::PushDownResult;
@@ -33,15 +42,6 @@ use crate::vtable::OperationsVTable;
 use crate::vtable::VTable;
 use crate::vtable::ValidityVTable;
 use crate::vtable::VisitorVTable;
-use crate::Array;
-use crate::ArrayBufferVisitor;
-use crate::ArrayChildVisitor;
-use crate::ArrayEq;
-use crate::ArrayHash;
-use crate::ArrayRef;
-use crate::Canonical;
-use crate::IntoArray;
-use crate::Precision;
 
 vtable!(Filter);
 
@@ -100,17 +100,9 @@ impl VTable for FilterVTable {
         let mut child = array.child.bind_kernel(ctx)?;
         let mask = array.mask.clone();
 
-        let full_cost = child.cost_estimate(&Mask::new_true(array.child.len()));
-        let pushdown_cost = child.cost_estimate(&mask);
-        log::debug!(
-            "Filter kernel cost estimate: full={}, pushdown={}",
-            full_cost,
-            pushdown_cost
-        );
-
         // NOTE(ngates): for now we keep the same behavior as develop where we push-down any
         //  query with <20% true values.
-        let pushdown = (pushdown_cost < full_cost) || (array.mask.density() < 0.2);
+        let pushdown = array.mask.density() < 0.2;
 
         if pushdown {
             // Try to push down the filter to the child if it's cheaper.

vortex-array/src/arrays/scalar_fn/kernel.rs

@@ -61,20 +61,6 @@ impl Kernel for ScalarFnKernel {
         Ok(self.scalar_fn.execute(args)?.ensure_vector(self.row_count))
     }
 
-    fn cost_estimate(&self, selection: &Mask) -> f64 {
-        let self_cost = self.scalar_fn.cost_estimate(selection);
-        let child_cost = self
-            .inputs
-            .iter()
-            .map(|input| match input {
-                KernelInput::Scalar(_) => 0.0,
-                KernelInput::Vector(k) => k.cost_estimate(selection),
-            })
-            .sum::<f64>();
-
-        self_cost + child_cost
-    }
-
     fn push_down_filter(self: Box<Self>, selection: &Mask) -> VortexResult<PushDownResult> {
         let mut new_inputs = Vec::with_capacity(self.inputs.len());
         for (input, dtype) in self.inputs.into_iter().zip(&self.input_dtypes) {

vortex-array/src/arrays/scalar_fn/rules.rs

@@ -3,19 +3,19 @@
 
 use vortex_error::VortexResult;
 use vortex_scalar::Scalar;
-use vortex_vector::scalar_matches_dtype;
 use vortex_vector::Datum;
 use vortex_vector::VectorOps;
+use vortex_vector::scalar_matches_dtype;
 
+use crate::Array;
+use crate::ArrayRef;
+use crate::IntoArray;
 use crate::arrays::AnyScalarFn;
 use crate::arrays::ConstantArray;
 use crate::arrays::ConstantVTable;
 use crate::arrays::ScalarFnArray;
 use crate::expr::ExecutionArgs;
 use crate::optimizer::rules::ArrayReduceRule;
-use crate::Array;
-use crate::ArrayRef;
-use crate::IntoArray;
 
 #[derive(Debug)]
 pub(crate) struct ScalarFnConstantRule;

vortex-array/src/arrays/scalar_fn/vtable/canonical.rs

@@ -3,15 +3,15 @@
 
 use vortex_error::VortexExpect;
 
+use crate::Array;
+use crate::Canonical;
+use crate::arrays::LEGACY_SESSION;
 use crate::arrays::scalar_fn::array::ScalarFnArray;
 use crate::arrays::scalar_fn::vtable::ScalarFnVTable;
-use crate::arrays::LEGACY_SESSION;
 use crate::executor::VectorExecutor;
 use crate::expr::ExecutionArgs;
 use crate::vectors::VectorIntoArray;
 use crate::vtable::CanonicalVTable;
-use crate::Array;
-use crate::Canonical;
 
 impl CanonicalVTable<ScalarFnVTable> for ScalarFnVTable {
     fn canonicalize(array: &ScalarFnArray) -> Canonical {

vortex-array/src/executor.rs

@@ -1,18 +1,18 @@
 // SPDX-License-Identifier: Apache-2.0
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
-use vortex_error::vortex_ensure;
 use vortex_error::VortexResult;
+use vortex_error::vortex_ensure;
 use vortex_session::VortexSession;
 use vortex_vector::Datum;
 use vortex_vector::Vector;
 use vortex_vector::VectorOps;
 
+use crate::Array;
+use crate::ArrayRef;
 use crate::arrays::ConstantVTable;
 use crate::kernel::BindCtx;
 use crate::session::ArraySessionExt;
-use crate::Array;
-use crate::ArrayRef;
 
 /// Executor for exporting a Vortex [`Vector`] or [`Datum`] from an [`ArrayRef`].
 pub trait VectorExecutor {

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

@@ -9,18 +9,17 @@ use vortex_dtype::DType;
 use vortex_dtype::FieldName;
 use vortex_dtype::FieldPath;
 use vortex_dtype::Nullability;
-use vortex_error::vortex_err;
 use vortex_error::VortexExpect;
 use vortex_error::VortexResult;
-use vortex_mask::Mask;
+use vortex_error::vortex_err;
 use vortex_proto::expr as pb;
 use vortex_vector::Datum;
 use vortex_vector::ScalarOps;
 use vortex_vector::VectorOps;
 
+use crate::ArrayRef;
+use crate::ToCanonical;
 use crate::compute::mask;
-use crate::expr::exprs::root::root;
-use crate::expr::stats::Stat;
 use crate::expr::Arity;
 use crate::expr::ChildName;
 use crate::expr::ExecutionArgs;
@@ -31,9 +30,10 @@ use crate::expr::SimplifyCtx;
 use crate::expr::StatsCatalog;
 use crate::expr::VTable;
 use crate::expr::VTableExt;
+use crate::expr::exprs::root::root;
+use crate::expr::lit;
+use crate::expr::stats::Stat;
 use crate::scalar_fns::ExprBuiltins;
-use crate::ArrayRef;
-use crate::ToCanonical;
 
 pub struct GetItem;
 
@@ -173,6 +173,44 @@ impl VTable for GetItem {
         Ok(None)
     }
 
+    fn simplify_untyped(
+        &self,
+        field_name: &FieldName,
+        expr: &Expression,
+    ) -> VortexResult<Option<Expression>> {
+        let child = expr.child(0);
+
+        // If the child is a Pack expression, we can directly return the corresponding child.
+        if let Some(pack) = child.as_opt::<Pack>() {
+            let idx = pack
+                .names
+                .iter()
+                .position(|name| name == field_name)
+                .ok_or_else(|| {
+                    vortex_err!(
+                        "Cannot find field {} in pack fields {:?}",
+                        field_name,
+                        pack.names
+                    )
+                })?;
+
+            let mut field = child.child(idx).clone();
+
+            // It's useful to simplify this node without type info, but we need to make sure
+            // the nullability is correct. We cannot cast since we don't have the dtype info here,
+            // so instead we insert a Mask expression that we know converts a child's dtype to
+            // nullable.
+            if pack.nullability.is_nullable() {
+                // Mask with an all-true array to ensure the field DType is nullable.
+                field = field.mask(lit(true))?;
+            }
+
+            return Ok(Some(field));
+        }
+
+        Ok(None)
+    }
+
     fn stat_expression(
         &self,
         field_name: &FieldName,
@@ -200,11 +238,6 @@ impl VTable for GetItem {
         // If this type-checks its infallible.
         false
     }
-
-    fn cost_estimate(&self, _options: &Self::Options, _selection: &Mask) -> f64 {
-        // This is largely a metadata-only operation.
-        0.0
-    }
 }
 
 /// Creates an expression that accesses a field from the root array.
@@ -242,15 +275,15 @@ mod tests {
     use vortex_dtype::StructFields;
     use vortex_scalar::Scalar;
 
+    use crate::Array;
+    use crate::IntoArray;
     use crate::arrays::StructArray;
     use crate::expr::exprs::binary::checked_add;
     use crate::expr::exprs::get_item::get_item;
     use crate::expr::exprs::literal::lit;
     use crate::expr::exprs::pack::pack;
     use crate::expr::exprs::root::root;
     use crate::validity::Validity;
-    use crate::Array;
-    use crate::IntoArray;
 
     fn test_array() -> StructArray {
         StructArray::from_fields(&[

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

@@ -2,6 +2,7 @@
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
 use std::fmt::Formatter;
+use std::ops::Not;
 
 use vortex_dtype::DType;
 use vortex_dtype::Nullability;
@@ -15,6 +16,7 @@ use vortex_vector::ScalarOps;
 use vortex_vector::VectorMutOps;
 use vortex_vector::VectorOps;
 
+use crate::Array;
 use crate::ArrayRef;
 use crate::expr::Arity;
 use crate::expr::ChildName;
@@ -75,10 +77,15 @@ impl VTable for Mask {
     fn evaluate(
         &self,
         _options: &Self::Options,
-        _expr: &Expression,
-        _scope: &ArrayRef,
+        expr: &Expression,
+        scope: &ArrayRef,
     ) -> VortexResult<ArrayRef> {
-        todo!()
+        let child = expr.child(0).evaluate(scope)?;
+
+        // Invert the validity mask - we want to set values to null where validity is false.
+        let inverted_mask = child.validity_mask().not();
+
+        crate::compute::mask(&child, &inverted_mask)
     }
 
     fn execute(&self, _options: &Self::Options, args: ExecutionArgs) -> VortexResult<Datum> {

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

@@ -14,7 +14,6 @@ use vortex_dtype::StructFields;
 use vortex_error::VortexExpect;
 use vortex_error::VortexResult;
 use vortex_error::vortex_bail;
-use vortex_mask::Mask;
 use vortex_utils::aliases::hash_set::HashSet;
 use vortex_vector::Datum;
 
@@ -186,11 +185,6 @@ impl VTable for Merge {
         todo!()
     }
 
-    fn cost_estimate(&self, _options: &Self::Options, _selection: &Mask) -> f64 {
-        // This is largely a metadata-only operation.
-        0.0
-    }
-
     fn simplify(
         &self,
         options: &Self::Options,