From 6a41ce8911bbb9220770946e30ae592894b4cf22 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 10 Apr 2026 10:37:14 +0000 Subject: [PATCH 1/2] indices: make key_type private & fix bug in Table::insert_index --- .../src/locking_tx_datastore/datastore.rs | 2 +- .../src/locking_tx_datastore/mut_tx.rs | 2 +- crates/table/src/table.rs | 31 ++++++++++--------- crates/table/src/table_index/mod.rs | 30 +++++++++++++----- 4 files changed, 40 insertions(+), 25 deletions(-) diff --git a/crates/datastore/src/locking_tx_datastore/datastore.rs b/crates/datastore/src/locking_tx_datastore/datastore.rs index 53327b55c8b..a4b197b8eb5 100644 --- a/crates/datastore/src/locking_tx_datastore/datastore.rs +++ b/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::>() }; assert_eq!(index_key_types(&tx), [AlgebraicType::U64, sum_original]); diff --git a/crates/datastore/src/locking_tx_datastore/mut_tx.rs b/crates/datastore/src/locking_tx_datastore/mut_tx.rs index f152f3306af..69e71f93ce3 100644 --- a/crates/datastore/src/locking_tx_datastore/mut_tx.rs +++ b/crates/datastore/src/locking_tx_datastore/mut_tx.rs @@ -333,7 +333,7 @@ impl MutTxId { let idx = idx.index(); let cols = idx.indexed_columns.clone(); - let point = point.into_algebraic_value(&idx.key_type); + let point = idx.key_into_algebraic_value(point); self.read_sets.insert_index_scan(table_id, cols, point, view.clone()); } diff --git a/crates/table/src/table.rs b/crates/table/src/table.rs index a130e1fa1fb..e50ae283548 100644 --- a/crates/table/src/table.rs +++ b/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(|| "".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 { @@ -1455,7 +1456,7 @@ impl Table { self.schema.table_name, self.schema.table_id, index.indexed_columns, - index.key_type, + index.key_type(), row, ) } diff --git a/crates/table/src/table_index/mod.rs b/crates/table/src/table_index/mod.rs index 839479b6010..c0055c967d5 100644 --- a/crates/table/src/table_index/mod.rs +++ b/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,7 +1795,7 @@ 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. @@ -1858,6 +1851,27 @@ impl TableIndex { self.idx.is_unique() } + /// 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. From a1e6b6e3a8bf4a2e5c41e12a6dcd04a97d3159d8 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 10 Apr 2026 10:57:37 +0000 Subject: [PATCH 2/2] TableIndex: privatize indexed_columns --- .../datastore/src/locking_tx_datastore/mut_tx.rs | 12 ++++-------- crates/table/src/table.rs | 12 ++++++------ crates/table/src/table_index/mod.rs | 16 +++++++++++++++- 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/crates/datastore/src/locking_tx_datastore/mut_tx.rs b/crates/datastore/src/locking_tx_datastore/mut_tx.rs index 69e71f93ce3..5da97c57dc8 100644 --- a/crates/datastore/src/locking_tx_datastore/mut_tx.rs +++ b/crates/datastore/src/locking_tx_datastore/mut_tx.rs @@ -332,7 +332,7 @@ impl MutTxId { }; let idx = idx.index(); - let cols = idx.indexed_columns.clone(); + 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)); }; diff --git a/crates/table/src/table.rs b/crates/table/src/table.rs index e50ae283548..338104c573c 100644 --- a/crates/table/src/table.rs +++ b/crates/table/src/table.rs @@ -1455,7 +1455,7 @@ impl Table { Found violation at pointer {ptr:?} to row {:?}.", self.schema.table_name, self.schema.table_id, - index.indexed_columns, + index.indexed_columns(), index.key_type(), row, ) @@ -1556,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)) } @@ -2278,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(); @@ -2305,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) } @@ -2429,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; @@ -2561,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() } diff --git a/crates/table/src/table_index/mod.rs b/crates/table/src/table_index/mod.rs index c0055c967d5..346630dbeff 100644 --- a/crates/table/src/table_index/mod.rs +++ b/crates/table/src/table_index/mod.rs @@ -1800,7 +1800,7 @@ pub struct TableIndex { /// 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 { @@ -1851,6 +1851,11 @@ 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 @@ -2136,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`. ///