Add AlgebraicValue::take + move test-code in btree_index to tests (…

…#1028)

* add AlgebraicValue::take for a neater interface

* btree_index: move test-only code to tests

crates/sats/src/algebraic_value.rs

@@ -2,9 +2,10 @@ pub mod de;
 pub mod ser;
 
 use crate::{AlgebraicType, ArrayValue, MapValue, ProductValue, SumValue};
+use core::mem;
+use core::ops::{Bound, RangeBounds};
 use derive_more::From;
 use enum_as_inner::EnumAsInner;
-use std::ops::{Bound, RangeBounds};
 
 /// Totally ordered [`f32`] allowing all IEEE-754 floating point values.
 pub type F32 = decorum::Total<f32>;
@@ -112,6 +113,11 @@ pub enum AlgebraicValue {
 
 #[allow(non_snake_case)]
 impl AlgebraicValue {
+    /// Extract the value and replace it with a dummy one that is cheap to make.
+    pub fn take(&mut self) -> Self {
+        mem::replace(self, Self::U8(0))
+    }
+
     /// Interpret the value as a byte slice or `None` if it isn't a byte slice.
     #[inline]
     pub fn as_bytes(&self) -> Option<&[u8]> {

crates/table/src/btree_index.rs

@@ -31,7 +31,7 @@ use crate::{
 use core::ops::RangeBounds;
 use multimap::{MultiMap, MultiMapRangeIter};
 use spacetimedb_primitives::{ColList, IndexId};
-use spacetimedb_sats::{product_value::InvalidFieldError, AlgebraicValue, ProductValue};
+use spacetimedb_sats::{product_value::InvalidFieldError, AlgebraicValue};
 
 mod multimap;
 
@@ -375,11 +375,6 @@ impl BTreeIndex {
         })
     }
 
-    /// Extracts from `row` the relevant column values according to what columns are indexed.
-    pub fn get_fields(&self, cols: &ColList, row: &ProductValue) -> Result<AlgebraicValue, InvalidFieldError> {
-        row.project_not_empty(cols)
-    }
-
     /// Inserts `ptr` with the value `row` to this index.
     /// This index will extract the necessary values from `row` based on `self.cols`.
     ///
@@ -395,15 +390,6 @@ impl BTreeIndex {
         self.idx.delete(cols, row_ref)
     }
 
-    /// Returns whether indexing `row` again would violate a unique constraint, if any.
-    pub fn violates_unique_constraint(&self, cols: &ColList, row: &ProductValue) -> bool {
-        if self.is_unique {
-            let col_value = self.get_fields(cols, row).unwrap();
-            return self.contains_any(&col_value);
-        }
-        false
-    }
-
     /// Returns an iterator over the rows that would violate the unique constraint of this index,
     /// if `row` were inserted,
     /// or `None`, if this index doesn't have a unique constraint.
@@ -467,7 +453,7 @@ mod test {
     use spacetimedb_primitives::ColListBuilder;
     use spacetimedb_sats::{
         db::def::{TableDef, TableSchema},
-        product, AlgebraicType, ProductType,
+        product, AlgebraicType, ProductType, ProductValue,
     };
     use std::collections::HashMap;
 
@@ -497,6 +483,16 @@ mod test {
         Table::new(schema, SquashedOffset::COMMITTED_STATE)
     }
 
+    /// Extracts from `row` the relevant column values according to what columns are indexed.
+    fn get_fields(cols: &ColList, row: &ProductValue) -> AlgebraicValue {
+        row.project_not_empty(cols).unwrap()
+    }
+
+    /// Returns whether indexing `row` again would violate a unique constraint, if any.
+    fn violates_unique_constraint(index: &BTreeIndex, cols: &ColList, row: &ProductValue) -> bool {
+        !index.is_unique || index.contains_any(&get_fields(cols, row))
+    }
+
     proptest! {
         #[test]
         fn remove_nonexistent_noop(((ty, cols, pv), is_unique) in (gen_row_and_cols(), any::<bool>())) {
@@ -516,7 +512,7 @@ mod test {
             let mut blob_store = HashMapBlobStore::default();
             let ptr = table.insert(&mut blob_store, &pv).unwrap().1;
             let row_ref = table.get_row_ref(&blob_store, ptr).unwrap();
-            let value = index.get_fields(&cols, &pv).unwrap();
+            let value = get_fields(&cols, &pv);
 
             prop_assert_eq!(index.idx.len(), 0);
             prop_assert_eq!(index.contains_any(&value), false);
@@ -541,11 +537,11 @@ mod test {
             let mut blob_store = HashMapBlobStore::default();
             let ptr = table.insert(&mut blob_store, &pv).unwrap().1;
             let row_ref = table.get_row_ref(&blob_store, ptr).unwrap();
-            let value = index.get_fields(&cols, &pv).unwrap();
+            let value = get_fields(&cols, &pv);
 
             // Nothing in the index yet.
             prop_assert_eq!(index.idx.len(), 0);
-            prop_assert_eq!(index.violates_unique_constraint(&cols, &pv), false);
+            prop_assert_eq!(violates_unique_constraint(&index, &cols, &pv), false);
             prop_assert_eq!(
                 index.get_rows_that_violate_unique_constraint(&value).unwrap().collect::<Vec<_>>(),
                 []
@@ -556,7 +552,7 @@ mod test {
 
             // Inserting again would be a problem.
             prop_assert_eq!(index.idx.len(), 1);
-            prop_assert_eq!(index.violates_unique_constraint(&cols, &pv), true);
+            prop_assert_eq!(violates_unique_constraint(&index, &cols, &pv), true);
             prop_assert_eq!(
                 index.get_rows_that_violate_unique_constraint(&value).unwrap().collect::<Vec<_>>(),
                 [ptr]

crates/vm/src/relation.rs

@@ -11,7 +11,6 @@ use spacetimedb_table::read_column::ReadColumn;
 use spacetimedb_table::table::RowRef;
 use std::borrow::Cow;
 use std::hash::{Hash, Hasher};
-use std::mem;
 use std::sync::Arc;
 
 /// RelValue represents either a reference to a row in a table,
@@ -97,10 +96,7 @@ impl<'a> RelValue<'a> {
     fn read_or_take_column(&mut self, col: usize) -> Option<AlgebraicValue> {
         match self {
             Self::Row(row_ref) => AlgebraicValue::read_column(*row_ref, col).ok(),
-            Self::Projection(pv) => {
-                let elem = pv.elements.get_mut(col)?;
-                Some(mem::replace(elem, AlgebraicValue::U8(0)))
-            }
+            Self::Projection(pv) => pv.elements.get_mut(col).map(AlgebraicValue::take),
         }
     }