Flatten ColumnOp so that we box less

clockworklabs · May 14, 2024 · 8944135 · 8944135
1 parent 24686b5
commit 8944135
Show file tree

Hide file tree

Showing 4 changed files with 208 additions and 263 deletions.
diff --git a/crates/core/src/sql/compiler.rs b/crates/core/src/sql/compiler.rs
@@ -250,7 +250,6 @@ mod tests {
     use crate::sql::execute::tests::run_for_testing;
     use crate::vm::tests::create_table_with_rows;
     use spacetimedb_lib::error::{ResultTest, TestError};
-    use spacetimedb_lib::operator::OpQuery;
     use spacetimedb_lib::{Address, Identity};
     use spacetimedb_primitives::{col_list, ColList, TableId};
     use spacetimedb_sats::{product, AlgebraicType, AlgebraicValue, ProductType};
@@ -648,25 +647,16 @@ mod tests {
         assert_eq!(source_lhs.table_id().unwrap(), lhs_id);
         assert_eq!(query.len(), 2);
 
-        // The first operation in the pipeline should be a selection
-        let Query::Select(ColumnOp::Cmp {
-            op: OpQuery::Cmp(OpCmp::Eq),
-            ref lhs,
-            ref rhs,
+        // The first operation in the pipeline should be a selection with `col#0 = 3`
+        let Query::Select(ColumnOp::ColCmpVal {
+            cmp: OpCmp::Eq,
+            lhs: ColId(0),
+            rhs: AlgebraicValue::U64(3),
         }) = query[0]
         else {
             panic!("unexpected operator {:#?}", query[0]);
         };
 
-        let ColumnOp::Col(ColExpr::Col(col)) = **lhs else {
-            panic!("unexpected left hand side {:#?}", **lhs);
-        };
-        assert_eq!(col, 0.into());
-
-        let ColumnOp::Col(ColExpr::Value(AlgebraicValue::U64(3))) = **rhs else {
-            panic!("unexpected right hand side {:#?}", **rhs);
-        };
-
         // The join should follow the selection
         let Query::JoinInner(JoinExpr {
             ref rhs,
@@ -732,23 +722,14 @@ mod tests {
         assert_eq!(&**inner_header, &source_lhs.head().extend(rhs.source.head()));
 
         // The selection should be pushed onto the rhs of the join
-        let Query::Select(ColumnOp::Cmp {
-            op: OpQuery::Cmp(OpCmp::Eq),
-            ref lhs,
-            ref rhs,
+        let Query::Select(ColumnOp::ColCmpVal {
+            cmp: OpCmp::Eq,
+            lhs: ColId(1),
+            rhs: AlgebraicValue::U64(3),
         }) = rhs.query[0]
         else {
             panic!("unexpected operator {:#?}", rhs.query[0]);
         };
-
-        let ColumnOp::Col(ColExpr::Col(col)) = **lhs else {
-            panic!("unexpected left hand side {:#?}", **lhs);
-        };
-        assert_eq!(col, 1.into());
-
-        let ColumnOp::Col(ColExpr::Value(AlgebraicValue::U64(3))) = **rhs else {
-            panic!("unexpected right hand side {:#?}", **rhs);
-        };
         Ok(())
     }
 
@@ -855,6 +836,8 @@ mod tests {
         assert_eq!(table_id, lhs_id);
         assert_eq!(query.len(), 1);
 
+        dbg!(&query[0]);
+
         let Query::IndexJoin(IndexJoin {
             probe_side:
                 QueryExpr {
@@ -890,23 +873,14 @@ mod tests {
         assert_eq!(table_id, rhs_id);
 
         // Followed by a selection
-        let Query::Select(ColumnOp::Cmp {
-            op: OpQuery::Cmp(OpCmp::Eq),
-            lhs: ref field,
-            rhs: ref value,
+        let Query::Select(ColumnOp::ColCmpVal {
+            cmp: OpCmp::Eq,
+            lhs: ColId(2),
+            rhs: AlgebraicValue::U64(3),
         }) = rhs[1]
         else {
             panic!("unexpected operator {:#?}", rhs[0]);
         };
-
-        let ColumnOp::Col(ColExpr::Col(col)) = **field else {
-            panic!("unexpected left hand side {:#?}", field);
-        };
-        assert_eq!(col, 2.into());
-
-        let ColumnOp::Col(ColExpr::Value(AlgebraicValue::U64(3))) = **value else {
-            panic!("unexpected right hand side {:#?}", value);
-        };
         Ok(())
     }
 
@@ -976,23 +950,14 @@ mod tests {
         assert_eq!(table_id, rhs_id);
 
         // Followed by a selection
-        let Query::Select(ColumnOp::Cmp {
-            op: OpQuery::Cmp(OpCmp::Eq),
-            lhs: ref field,
-            rhs: ref value,
+        let Query::Select(ColumnOp::ColCmpVal {
+            cmp: OpCmp::Eq,
+            lhs: ColId(2),
+            rhs: AlgebraicValue::U64(3),
         }) = rhs[1]
         else {
             panic!("unexpected operator {:#?}", rhs[0]);
         };
-
-        let ColumnOp::Col(ColExpr::Col(col)) = **field else {
-            panic!("unexpected left hand side {:#?}", field);
-        };
-        assert_eq!(col, 2.into());
-
-        let ColumnOp::Col(ColExpr::Value(AlgebraicValue::U64(3))) = **value else {
-            panic!("unexpected right hand side {:#?}", value);
-        };
         Ok(())
     }
 

diff --git a/crates/core/src/vm.rs b/crates/core/src/vm.rs
@@ -291,7 +291,7 @@ static_assert_size!(IndexSemiJoinLeft<Box<IterRows<'static>>>, 312);
 
 impl<'a, Rhs: RelOps<'a>> IndexSemiJoinLeft<'a, '_, Rhs> {
     fn filter(&self, index_row: &RelValue<'_>) -> bool {
-        self.index_select.as_ref().map_or(true, |op| op.compare(index_row))
+        self.index_select.as_ref().map_or(true, |op| op.eval_bool(index_row))
     }
 }
 
@@ -327,7 +327,7 @@ impl<'a, Rhs: RelOps<'a>> RelOps<'a> for IndexSemiJoinLeft<'a, '_, Rhs> {
     }
 }
 
-/// An index join operator that returns matching rows from the index side.
+/// An index join operator that returns matching rows from the probe side.
 pub struct IndexSemiJoinRight<'a, 'c, Rhs: RelOps<'a>> {
     /// An iterator for the probe side.
     /// The values returned will be used to probe the index.
@@ -352,7 +352,7 @@ static_assert_size!(IndexSemiJoinRight<Box<IterRows<'static>>>, 64);
 
 impl<'a, Rhs: RelOps<'a>> IndexSemiJoinRight<'a, '_, Rhs> {
     fn filter(&self, index_row: &RelValue<'_>) -> bool {
-        self.index_select.as_ref().map_or(true, |op| op.compare(index_row))
+        self.index_select.as_ref().map_or(true, |op| op.eval_bool(index_row))
     }
 }
 

diff --git a/crates/vm/src/eval.rs b/crates/vm/src/eval.rs
@@ -8,7 +8,7 @@ use spacetimedb_sats::ProductValue;
 pub type IterRows<'a> = dyn RelOps<'a> + 'a;
 
 pub fn build_select<'a>(base: impl RelOps<'a> + 'a, cmp: &'a ColumnOp) -> Box<IterRows<'a>> {
-    Box::new(base.select(move |row| cmp.compare(row)))
+    Box::new(base.select(move |row| cmp.eval_bool(row)))
 }
 
 pub fn build_project<'a>(base: impl RelOps<'a> + 'a, proj: &'a ProjectExpr) -> Box<IterRows<'a>> {