Remove var expression (#3829)

Fixes #3671 

* Removes the Var expression, leaving instead a `root()` expression for
resolving the scope root.
* The expression scope can hold context variables, useful for passing in
auth tokens for example, but not variables that would impact the
return_dtype of the expression.
* ScopeDType has therefore been removed, because the dtype of the scope
_is_ just the dtype of the root array.
* Simplifies some transformation / partitioning logic where vars no
longer need to be considered.

Signed-off-by: Nicholas Gates <[email protected]>
Co-authored-by: Will Manning <[email protected]>

Author: gatesn

java/vortex-jni/src/main/java/dev/vortex/api/Expression.java

@@ -34,7 +34,7 @@ default <T> T accept(Visitor<T> visitor) {
     interface Visitor<T> {
         T visitLiteral(Literal<?> literal);
 
-        T visitIdentity(Identity identity);
+        T visitRoot(Root root);
 
         T visitBinary(Binary binary);
 

java/vortex-jni/src/main/java/dev/vortex/api/expressions/Identity.java

@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: Apache-2.0
+// SPDX-FileCopyrightText: Copyright the Vortex contributors
+
+package dev.vortex.api.expressions;
+
+import dev.vortex.api.Expression;
+
+import java.util.List;
+import java.util.Optional;
+
+public final class Root implements Expression {
+    public static final Root INSTANCE = new Root();
+
+    private Root() {}
+
+    public static Root parse(byte[] _metadata, List<Expression> children) {
+        if (!children.isEmpty()) {
+            throw new IllegalArgumentException("Root expression must have no children, found: " + children.size());
+        }
+        return INSTANCE;
+    }
+
+    @Override
+    public String id() {
+        return "root";
+    }
+
+    @Override
+    public List<Expression> children() {
+        return List.of();
+    }
+
+    @Override
+    public Optional<byte[]> metadata() {
+        return Optional.of(new byte[] {});
+    }
+
+    @Override
+    public String toString() {
+        return "$";
+    }
+
+    // equals and hashCode depend on address equality to INSTANCE.
+
+    @Override
+    public <T> T accept(Visitor<T> visitor) {
+        return visitor.visitRoot(this);
+    }
+}

java/vortex-jni/src/main/java/dev/vortex/api/expressions/Root.java

@@ -42,8 +42,8 @@ public static Expression deserialize(ExprProtos.Expr expr) {
                 return Binary.parse(metadata, children);
             case "get_item":
                 return GetItem.parse(metadata, children);
-            case "var":
-                return Identity.parse(metadata, children);
+            case "root":
+                return Root.parse(metadata, children);
             case "literal":
                 return Literal.parse(metadata, children);
             case "not":

java/vortex-jni/src/main/java/dev/vortex/api/proto/Expressions.java

@@ -7,13 +7,14 @@
 
 import dev.vortex.api.expressions.Binary;
 import dev.vortex.api.expressions.GetItem;
-import dev.vortex.api.expressions.Identity;
+import dev.vortex.api.expressions.Root;
 import dev.vortex.api.expressions.Literal;
 import java.math.BigDecimal;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+
 import org.junit.jupiter.api.Test;
 
 public final class TestMinimal {
@@ -159,7 +160,7 @@ public Person readRow(Array[] fields, int idx) {
     public void testProjectedScanWithFilter() {
         var filterOptions = ScanOptions.builder()
                 .addColumns("State", "Salary", "Name")
-                .predicate(Binary.eq(GetItem.of(Identity.INSTANCE, "State"), Literal.string("VA")))
+                .predicate(Binary.eq(GetItem.of(Root.INSTANCE, "State"), Literal.string("VA")))
                 .build();
 
         try (var file = Files.open(MINIMAL_PATH);

java/vortex-jni/src/test/java/dev/vortex/api/TestMinimal.java

@@ -15,8 +15,8 @@ public final class TestExpressionProtos {
     @Test
     public void testRoundTrip() {
         Expression expression = Binary.and(
-                GetItem.of(Identity.INSTANCE, "a.b.c"),
-                Binary.or(Identity.INSTANCE, Not.of(Literal.bool(null)), Literal.bool(false)),
+                GetItem.of(Root.INSTANCE, "a.b.c"),
+                Binary.or(Root.INSTANCE, Not.of(Literal.bool(null)), Literal.bool(false)),
                 Binary.eq(Literal.bool(true), Not.of(Literal.bool(false))));
         ExprProtos.Expr proto = Expressions.serialize(expression);
         Expression deserialized = Expressions.deserialize(proto);

java/vortex-jni/src/test/java/dev/vortex/api/expressions/proto/TestExpressionProtos.java

@@ -91,6 +91,17 @@ impl StructArray {
         struct_dtype
     }
 
+    /// Create a new `StructArray` with the given length, but without any fields.
+    pub fn new_with_len(len: usize) -> Self {
+        Self::try_new(
+            FieldNames::default(),
+            Vec::new(),
+            len,
+            Validity::NonNullable,
+        )
+        .vortex_expect("StructArray::new_with_len should not fail")
+    }
+
     pub fn try_new(
         names: FieldNames,
         fields: Vec<ArrayRef>,

vortex-array/src/arrays/struct_/mod.rs

@@ -2,13 +2,18 @@
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
 use vortex_array::stats::Stat;
+use vortex_dtype::FieldPath;
 
-use crate::{AccessPath, ExprRef};
+use crate::ExprRef;
 
 pub trait StatsCatalog {
-    /// Given an id, field and stat return an expression that when evaluated will return that stat
-    /// this would be a column reference or a literal value, if the value is known at planning time.
-    fn stats_ref(&mut self, _access_path: &AccessPath, _stat: Stat) -> Option<ExprRef> {
+    /// Given a field path and statist, return an expression that when evaluated over the catalog
+    /// will return that stat for the referenced field.
+    ///
+    /// This is likely to be a column expression, or a literal.
+    ///
+    /// Returns `None` if the stat is not available for the field path.
+    fn stats_ref(&mut self, _field_path: &FieldPath, _stat: Stat) -> Option<ExprRef> {
         None
     }
 }
@@ -56,7 +61,7 @@ pub trait AnalysisExpr {
         None
     }
 
-    fn field_path(&self) -> Option<AccessPath> {
+    fn field_path(&self) -> Option<FieldPath> {
         None
     }
 

vortex-expr/src/analysis.rs

@@ -11,8 +11,7 @@ use vortex_error::{VortexResult, vortex_bail};
 use vortex_proto::expr as pb;
 
 use crate::{
-    AnalysisExpr, BinaryExpr, ExprEncodingRef, ExprId, ExprRef, IntoExpr, Scope, ScopeDType,
-    VTable, vtable,
+    AnalysisExpr, BinaryExpr, ExprEncodingRef, ExprId, ExprRef, IntoExpr, Scope, VTable, vtable,
 };
 
 vtable!(Between);
@@ -102,7 +101,7 @@ impl VTable for BetweenVTable {
         between_compute(&arr_val, &lower_arr_val, &upper_arr_val, &expr.options)
     }
 
-    fn return_dtype(expr: &Self::Expr, scope: &ScopeDType) -> VortexResult<DType> {
+    fn return_dtype(expr: &Self::Expr, scope: &DType) -> VortexResult<DType> {
         let arr_dt = expr.arr.return_dtype(scope)?;
         let lower_dt = expr.lower.return_dtype(scope)?;
         let upper_dt = expr.upper.return_dtype(scope)?;

vortex-expr/src/exprs/between.rs

@@ -11,8 +11,8 @@ use vortex_error::{VortexResult, vortex_bail};
 use vortex_proto::expr as pb;
 
 use crate::{
-    AnalysisExpr, ExprEncodingRef, ExprId, ExprRef, IntoExpr, Operator, Scope, ScopeDType,
-    StatsCatalog, VTable, vtable,
+    AnalysisExpr, ExprEncodingRef, ExprId, ExprRef, IntoExpr, Operator, Scope, StatsCatalog,
+    VTable, vtable,
 };
 
 vtable!(Binary);
@@ -93,7 +93,7 @@ impl VTable for BinaryVTable {
         }
     }
 
-    fn return_dtype(expr: &Self::Expr, scope: &ScopeDType) -> VortexResult<DType> {
+    fn return_dtype(expr: &Self::Expr, scope: &DType) -> VortexResult<DType> {
         let lhs = expr.lhs.return_dtype(scope)?;
         let rhs = expr.rhs.return_dtype(scope)?;
 
@@ -426,8 +426,8 @@ mod tests {
     use vortex_dtype::{DType, Nullability};
 
     use crate::{
-        ScopeDType, VortexExpr, and, and_collect, and_collect_right, col, eq, gt, gt_eq, lit, lt,
-        lt_eq, not_eq, or, test_harness,
+        VortexExpr, and, and_collect, and_collect_right, col, eq, gt, gt_eq, lit, lt, lt_eq,
+        not_eq, or, test_harness,
     };
 
     #[test]
@@ -455,13 +455,13 @@ mod tests {
         let bool2: Arc<dyn VortexExpr> = col("bool2");
         assert_eq!(
             and(bool1.clone(), bool2.clone())
-                .return_dtype(&ScopeDType::new(dtype.clone()))
+                .return_dtype(&dtype)
                 .unwrap(),
             DType::Bool(Nullability::NonNullable)
         );
         assert_eq!(
             or(bool1.clone(), bool2.clone())
-                .return_dtype(&ScopeDType::new(dtype.clone()))
+                .return_dtype(&dtype)
                 .unwrap(),
             DType::Bool(Nullability::NonNullable)
         );
@@ -470,38 +470,32 @@ mod tests {
         let col2: Arc<dyn VortexExpr> = col("col2");
 
         assert_eq!(
-            eq(col1.clone(), col2.clone())
-                .return_dtype(&ScopeDType::new(dtype.clone()))
-                .unwrap(),
+            eq(col1.clone(), col2.clone()).return_dtype(&dtype).unwrap(),
             DType::Bool(Nullability::Nullable)
         );
         assert_eq!(
             not_eq(col1.clone(), col2.clone())
-                .return_dtype(&ScopeDType::new(dtype.clone()))
+                .return_dtype(&dtype)
                 .unwrap(),
             DType::Bool(Nullability::Nullable)
         );
         assert_eq!(
-            gt(col1.clone(), col2.clone())
-                .return_dtype(&ScopeDType::new(dtype.clone()))
-                .unwrap(),
+            gt(col1.clone(), col2.clone()).return_dtype(&dtype).unwrap(),
             DType::Bool(Nullability::Nullable)
         );
         assert_eq!(
             gt_eq(col1.clone(), col2.clone())
-                .return_dtype(&ScopeDType::new(dtype.clone()))
+                .return_dtype(&dtype)
                 .unwrap(),
             DType::Bool(Nullability::Nullable)
         );
         assert_eq!(
-            lt(col1.clone(), col2.clone())
-                .return_dtype(&ScopeDType::new(dtype.clone()))
-                .unwrap(),
+            lt(col1.clone(), col2.clone()).return_dtype(&dtype).unwrap(),
             DType::Bool(Nullability::Nullable)
         );
         assert_eq!(
             lt_eq(col1.clone(), col2.clone())
-                .return_dtype(&ScopeDType::new(dtype.clone()))
+                .return_dtype(&dtype)
                 .unwrap(),
             DType::Bool(Nullability::Nullable)
         );
@@ -511,7 +505,7 @@ mod tests {
                 lt(col1.clone(), col2.clone()),
                 not_eq(col1.clone(), col2.clone())
             )
-            .return_dtype(&ScopeDType::new(dtype))
+            .return_dtype(&dtype)
             .unwrap(),
             DType::Bool(Nullability::Nullable)
         );