Indices: house keeping (privatize stuff + fix minor bug in insert_index)

crates/datastore/src/locking_tx_datastore/datastore.rs

@@ -3560,7 +3560,7 @@ mod tests {
                 .unwrap()
                 .indexes
                 .values()
-                .map(|i| i.key_type.clone())
+                .map(|i| i.key_type().clone())
                 .collect::<Vec<_>>()
         };
         assert_eq!(index_key_types(&tx), [AlgebraicType::U64, sum_original]);

crates/datastore/src/locking_tx_datastore/mut_tx.rs

@@ -332,8 +332,8 @@ impl MutTxId {
         };
 
         let idx = idx.index();
-        let cols = idx.indexed_columns.clone();
-        let point = point.into_algebraic_value(&idx.key_type);
+        let cols = idx.indexed_columns().clone();
+        let point = idx.key_into_algebraic_value(point);
         self.read_sets.insert_index_scan(table_id, cols, point, view.clone());
     }
 
@@ -1299,10 +1299,8 @@ impl MutTxId {
         let map_violation = |violation, index: &TableIndex, table: &Table, bs: &dyn BlobStore| {
             let violation = table
                 .get_row_ref(bs, violation)
-                .expect("row came from scanning the table")
-                .project(&index.indexed_columns)
-                .expect("`cols` should consist of valid columns for this table");
-
+                .expect("row came from scanning the table");
+            let violation = index.project_row(violation);
             let schema = table.get_schema();
             let violation = UniqueConstraintViolation::build_with_index_schema(schema, index, &index_schema, violation);
             IndexError::from(violation).into()
@@ -3054,9 +3052,7 @@ impl MutTxId {
 
                 tx_row_ptr
             } else {
-                let index_key = tx_row_ref
-                    .project(&commit_index.indexed_columns)
-                    .expect("`tx_row_ref` should be compatible with `commit_index`");
+                let index_key = commit_index.project_row(tx_row_ref);
                 throw!(IndexError::KeyNotFound(index_id, index_key));
             };
 

crates/table/src/table.rs

@@ -26,6 +26,7 @@ use core::{
 use core::{mem, ops::RangeBounds};
 use derive_more::{Add, AddAssign, From, Sub, SubAssign};
 use enum_as_inner::EnumAsInner;
+use itertools::Itertools;
 use smallvec::SmallVec;
 use spacetimedb_primitives::{ColId, ColList, IndexId, SequenceId, TableId};
 use spacetimedb_sats::{
@@ -45,7 +46,7 @@ use spacetimedb_sats::{
 };
 use spacetimedb_sats::{memory_usage::MemoryUsage, raw_identifier::RawIdentifier};
 use spacetimedb_schema::{
-    def::{BTreeAlgorithm, IndexAlgorithm},
+    def::IndexAlgorithm,
     identifier::Identifier,
     schema::{columns_to_row_type, ColumnSchema, IndexSchema, TableSchema},
     table_name::TableName,
@@ -526,9 +527,7 @@ impl Table {
         let schema = self.get_schema().clone();
         let row_type = schema.get_row_type();
         for index in self.indexes.values_mut() {
-            index.key_type = row_type
-                .project(&index.indexed_columns)
-                .expect("new row type should have as many columns as before")
+            index.recompute_key_type(row_type);
         }
     }
 
@@ -1429,23 +1428,25 @@ impl Table {
             let row = unsafe { self.get_row_ref_unchecked(blob_store, ptr) }.to_product_value();
 
             if let Some(index_schema) = self.schema.indexes.iter().find(|index_schema| index_schema.index_id == index_id) {
-                let indexed_column = if let IndexAlgorithm::BTree(BTreeAlgorithm { columns }) = &index_schema.index_algorithm {
-                    Some(columns)
-                } else {
-                    None
-                };
-                let indexed_column = indexed_column.and_then(|columns| columns.as_singleton());
-                let indexed_column_info = indexed_column.and_then(|column| self.schema.get_column(column.idx()));
+                let cols = index_schema.index_algorithm.columns().to_owned();
+                let cols_infos = cols
+                    .iter()
+                    .map(|col|
+                        self.schema.get_column(col.idx())
+                            .map(|c| format!("`{}`", &*c.col_name))
+                            .unwrap_or_else(|| "<unknown>".into())
+                    )
+                    .join(",");
 
                 format!(
-                    "Adding index `{}` {:?} to table `{}` {:?} on column `{}` {:?} should cause no unique constraint violations.\
+                    "Adding index `{}` {:?} to table `{}` {:?} on columns `{}` {:?} should cause no unique constraint violations.\
                     Found violation at pointer {ptr:?} to row {:?}.",
                     index_schema.index_name,
                     index_schema.index_id,
                     self.schema.table_name,
                     self.schema.table_id,
-                    indexed_column_info.map(|column| &column.col_name[..]).unwrap_or("unknown column"),
-                    indexed_column,
+                    cols_infos,
+                    cols,
                     row,
                 )
             } else {
@@ -1454,8 +1455,8 @@ impl Table {
                     Found violation at pointer {ptr:?} to row {:?}.",
                     self.schema.table_name,
                     self.schema.table_id,
-                    index.indexed_columns,
-                    index.key_type,
+                    index.indexed_columns(),
+                    index.key_type(),
                     row,
                 )
             }
@@ -1555,7 +1556,7 @@ impl Table {
     pub fn get_index_by_cols(&self, cols: &ColList) -> Option<(IndexId, &TableIndex)> {
         self.indexes
             .iter()
-            .find(|(_, index)| &index.indexed_columns == cols)
+            .find(|(_, index)| index.indexed_columns() == cols)
             .map(|(id, idx)| (*id, idx))
     }
 
@@ -2277,7 +2278,7 @@ impl UniqueConstraintViolation {
 
         // Fetch the names of the columns used in the index.
         let cols = schema
-            .get_columns(&index.indexed_columns)
+            .get_columns(index.indexed_columns())
             .map(|(_, cs)| cs.unwrap().col_name.clone())
             .collect();
 
@@ -2304,7 +2305,7 @@ impl Table {
         index_id: IndexId,
         row: RowRef<'_>,
     ) -> UniqueConstraintViolation {
-        let value = row.project(&index.indexed_columns).unwrap();
+        let value = index.project_row(row);
         let schema = self.get_schema();
         UniqueConstraintViolation::build(schema, index, index_id, value)
     }
@@ -2428,6 +2429,7 @@ pub(crate) mod test {
     use super::*;
     use crate::blob_store::{HashMapBlobStore, NullBlobStore};
     use crate::page::tests::hash_unmodified_save_get;
+    use crate::table_index::KeySize;
     use crate::var_len::VarLenGranule;
     use proptest::prelude::*;
     use proptest::test_runner::TestCaseResult;
@@ -2560,8 +2562,7 @@ pub(crate) mod test {
             .iter()
             .map(|row_ptr| {
                 let row_ref = table.get_row_ref(blob_store, row_ptr).unwrap();
-                let key = row_ref.project(&index.indexed_columns).unwrap();
-                crate::table_index::KeySize::key_size_in_bytes(&key) as u64
+                index.project_row(row_ref).key_size_in_bytes() as u64
             })
             .sum()
     }

crates/table/src/table_index/mod.rs

@@ -1778,13 +1778,6 @@ pub struct IndexKey<'a> {
     key: TypedIndexKey<'a>,
 }
 
-impl IndexKey<'_> {
-    /// Converts the key into an [`AlgebraicValue`].
-    pub fn into_algebraic_value(self, key_type: &AlgebraicType) -> AlgebraicValue {
-        self.key.into_algebraic_value(key_type)
-    }
-}
-
 /// A decoded range scan bound, which may be a point or a range.
 #[derive(Debug, EnumAsInner)]
 pub enum PointOrRange<'a> {
@@ -1802,12 +1795,12 @@ pub struct TableIndex {
     /// The key type of this index.
     /// This is the projection of the row type to the types of the columns indexed.
     // NOTE(centril): This is accessed in index scan ABIs for decoding, so don't `Box<_>` it.
-    pub key_type: AlgebraicType,
+    key_type: AlgebraicType,
 
     /// Given a full row, typed at some `ty: ProductType`,
     /// these columns are the ones that this index indexes.
     /// Projecting the `ty` to `self.indexed_columns` yields the index's type `self.key_type`.
-    pub indexed_columns: ColList,
+    indexed_columns: ColList,
 }
 
 impl MemoryUsage for TableIndex {
@@ -1858,6 +1851,32 @@ impl TableIndex {
         self.idx.is_unique()
     }
 
+    /// Returns the indexed columns of this index.
+    pub fn indexed_columns(&self) -> &ColList {
+        &self.indexed_columns
+    }
+
+    /// Returns the key type of this index.
+    pub fn key_type(&self) -> &AlgebraicType {
+        &self.key_type
+    }
+
+    /// Recomputes the key type of the index.
+    ///
+    /// Assumes that `row_type` is layout compatible with the row type
+    /// that `self` was constructed with.
+    /// The method may panic otherwise.
+    pub fn recompute_key_type(&mut self, row_type: &ProductType) {
+        self.key_type = row_type
+            .project(&self.indexed_columns)
+            .expect("new row type should have as many columns as before")
+    }
+
+    /// Converts `key` into an [`AlgebraicValue`]
+    pub fn key_into_algebraic_value(&self, key: IndexKey<'_>) -> AlgebraicValue {
+        key.key.into_algebraic_value(&self.key_type)
+    }
+
     /// Derives a key for this index from `value`.
     ///
     /// Panics if `value` is not consistent with this index's key type.
@@ -2122,6 +2141,15 @@ impl TableIndex {
         unsafe { TypedIndexKey::from_row_ref(&self.key_type, &self.idx, &self.indexed_columns, row_ref) }.into()
     }
 
+    /// Projects `row_ref` to the columns of `self`.
+    ///
+    /// May panic if `row_ref` doesn't belong to the same table as this inex.
+    pub fn project_row(&self, row_ref: RowRef<'_>) -> AlgebraicValue {
+        row_ref
+            .project(&self.indexed_columns)
+            .expect("`row_ref` should belong to the same table as this index")
+    }
+
     /// Inserts `ptr` with the value `row` to this index.
     /// This index will extract the necessary values from `row` based on `self.indexed_columns`.
     ///