Fixes for performance drop related to missing index selection (#647)

* Fix test bootstrap

Merge

Merge

* Fix test & refactor FieldDef

* Added test to check schema validations

crates/core/src/db/datastore/locking_tx_datastore/mod.rs

@@ -52,6 +52,7 @@ use spacetimedb_lib::{metrics::METRICS, Address};
 use spacetimedb_primitives::*;
 use spacetimedb_sats::data_key::{DataKey, ToDataKey};
 use spacetimedb_sats::db::def::*;
+use spacetimedb_sats::db::error::SchemaErrors;
 use spacetimedb_sats::hash::Hash;
 use spacetimedb_sats::relation::RelValue;
 use spacetimedb_sats::{
@@ -499,7 +500,7 @@ impl CommittedState {
         let mut constraints = Vec::new();
         for data_ref in self.iter_by_col_eq(
             &ST_CONSTRAINTS_ID,
-            &NonEmpty::new(StIndexFields::TableId.col_id()),
+            &NonEmpty::new(StConstraintFields::TableId.col_id()),
             &table_id.into(),
         )? {
             let row = data_ref.view();
@@ -993,7 +994,11 @@ impl MutTxId {
             return Err(TableError::System(table_schema.table_name.clone()).into());
         }
 
-        table_schema.clone().into_schema(0.into()).validated()?;
+        table_schema
+            .clone()
+            .into_schema(0.into())
+            .validated()
+            .map_err(|err| DBError::Schema(SchemaErrors(err)))?;
 
         Ok(())
     }
@@ -1137,7 +1142,7 @@ impl MutTxId {
 
         // Look up the constraints for the table in question.
         let mut constraints = Vec::new();
-        for data_ref in self.iter_by_col_eq(&ctx, &ST_CONSTRAINTS_ID, StIndexFields::TableId, table_id.into())? {
+        for data_ref in self.iter_by_col_eq(&ctx, &ST_CONSTRAINTS_ID, StConstraintFields::TableId, table_id.into())? {
             let row = data_ref.view();
 
             let el = StConstraintRow::try_from(row)?;
@@ -2973,7 +2978,6 @@ mod tests {
                 },
             ])
             .with_column_sequence(ColId(0))
-            .unwrap()
     }
 
     #[rustfmt::skip]
@@ -2986,10 +2990,9 @@ mod tests {
                 IdxSchema { id: 6, table: 6, col: 0, name: "id_idx", unique: true },
                 IdxSchema { id: 7, table: 6, col: 1, name: "name_idx", unique: true },
             ]),
-
             map_array([
-                ConstraintRow { constraint_id: 6, table_id: 6, columns: col(0), constraints: Constraints::indexed(), constraint_name: "ct_Foo_id_idx_indexed" },
-                ConstraintRow { constraint_id: 7, table_id: 6, columns: col(1), constraints: Constraints::indexed(), constraint_name: "ct_Foo_name_idx_indexed" }
+                ConstraintRow { constraint_id: 6, table_id: 6, columns: col(0), constraints: Constraints::unique(), constraint_name: "ct_Foo_id_idx_unique" },
+                ConstraintRow { constraint_id: 7, table_id: 6, columns: col(1), constraints: Constraints::unique(), constraint_name: "ct_Foo_name_idx_unique" }
             ]),
              map_array([
                 SequenceRow { id: 4, table: 6, col_pos: 0, name: "seq_Foo_id", start: 1 }
@@ -3073,7 +3076,7 @@ mod tests {
         assert_eq!(query.scan_st_indexes()?, map_array([
             IndexRow { id: 0, table: 0, col: col(0), name: "idx_st_table_table_id_primary_key_auto_unique", unique: true },
             IndexRow { id: 1, table: 0, col: col(1), name: "idx_st_table_table_name_unique", unique: true },
-            IndexRow { id: 2, table: 1, col: cols(0, vec![1]), name: "idx_st_columns_table_id_col_pos_unique_unique", unique: true },
+            IndexRow { id: 2, table: 1, col: cols(0, vec![1]), name: "idx_st_columns_table_id_col_pos_unique", unique: true },
             IndexRow { id: 3, table: 2, col: col(0), name: "idx_st_sequence_sequence_id_primary_key_auto_unique", unique: true },
             IndexRow { id: 4, table: 3, col: col(0), name: "idx_st_indexes_index_id_primary_key_auto_unique", unique: true },
             IndexRow { id: 5, table: 4, col: col(0), name: "idx_st_constraints_constraint_id_primary_key_auto_unique", unique: true },
@@ -3088,7 +3091,7 @@ mod tests {
         #[rustfmt::skip]
         assert_eq!(query.scan_st_constraints()?, map_array([
             ConstraintRow { constraint_id: 0, table_id: 0, columns: col(0), constraints: Constraints::primary_key_auto(), constraint_name: "ct_st_table_table_id_primary_key_auto" },
-            ConstraintRow { constraint_id: 1, table_id: 0, columns: col(1), constraints: Constraints::indexed(), constraint_name: "ct_st_table_table_name_unique_indexed" },
+            ConstraintRow { constraint_id: 1, table_id: 0, columns: col(1), constraints: Constraints::unique(), constraint_name: "ct_st_table_table_name_unique" },
             ConstraintRow { constraint_id: 2, table_id: 1, columns: cols(0, vec![1]), constraints: Constraints::unique(), constraint_name: "ct_st_columns_table_id_col_pos_unique" },
             ConstraintRow { constraint_id: 3, table_id: 2, columns: col(0), constraints: Constraints::primary_key_auto(), constraint_name: "ct_st_sequence_sequence_id_primary_key_auto" },
             ConstraintRow { constraint_id: 4, table_id: 3, columns: col(0), constraints: Constraints::primary_key_auto(), constraint_name: "ct_st_indexes_index_id_primary_key_auto" },
@@ -3394,7 +3397,7 @@ mod tests {
         assert_eq!(index_rows, [
             IndexRow { id: 0, table: 0, col: col(0), name: "idx_st_table_table_id_primary_key_auto_unique", unique: true },
             IndexRow { id: 1, table: 0, col: col(1), name: "idx_st_table_table_name_unique", unique: true },
-            IndexRow { id: 2, table: 1, col: cols(0, vec![1]), name: "idx_st_columns_table_id_col_pos_unique_unique", unique: true },
+            IndexRow { id: 2, table: 1, col: cols(0, vec![1]), name: "idx_st_columns_table_id_col_pos_unique", unique: true },
             IndexRow { id: 3, table: 2, col: col(0), name: "idx_st_sequence_sequence_id_primary_key_auto_unique", unique: true },
             IndexRow { id: 4, table: 3, col: col(0), name: "idx_st_indexes_index_id_primary_key_auto_unique", unique: true },
             IndexRow { id: 5, table: 4, col: col(0), name: "idx_st_constraints_constraint_id_primary_key_auto_unique", unique: true },
@@ -3437,7 +3440,7 @@ mod tests {
         assert_eq!(index_rows, [
             IndexRow { id: 0, table: 0, col: col(0), name: "idx_st_table_table_id_primary_key_auto_unique", unique: true },
             IndexRow { id: 1, table: 0, col: col(1), name: "idx_st_table_table_name_unique", unique: true },
-            IndexRow { id: 2, table: 1, col: cols(0, vec![1]), name: "idx_st_columns_table_id_col_pos_unique_unique", unique: true },
+            IndexRow { id: 2, table: 1, col: cols(0, vec![1]), name: "idx_st_columns_table_id_col_pos_unique", unique: true },
             IndexRow { id: 3, table: 2, col: col(0), name: "idx_st_sequence_sequence_id_primary_key_auto_unique", unique: true },
             IndexRow { id: 4, table: 3, col: col(0), name: "idx_st_indexes_index_id_primary_key_auto_unique", unique: true },
             IndexRow { id: 5, table: 4, col: col(0), name: "idx_st_constraints_constraint_id_primary_key_auto_unique", unique: true },
@@ -3480,7 +3483,7 @@ mod tests {
         assert_eq!(index_rows, [
             IndexRow { id: 0, table: 0, col: col(0), name: "idx_st_table_table_id_primary_key_auto_unique", unique: true },
             IndexRow { id: 1, table: 0, col: col(1), name: "idx_st_table_table_name_unique", unique: true },
-            IndexRow { id: 2, table: 1, col: cols(0, vec![1]), name: "idx_st_columns_table_id_col_pos_unique_unique", unique: true },
+            IndexRow { id: 2, table: 1, col: cols(0, vec![1]), name: "idx_st_columns_table_id_col_pos_unique", unique: true },
             IndexRow { id: 3, table: 2, col: col(0), name: "idx_st_sequence_sequence_id_primary_key_auto_unique", unique: true },
             IndexRow { id: 4, table: 3, col: col(0), name: "idx_st_indexes_index_id_primary_key_auto_unique", unique: true },
             IndexRow { id: 5, table: 4, col: col(0), name: "idx_st_constraints_constraint_id_primary_key_auto_unique", unique: true },

crates/core/src/db/datastore/system_tables.rs

@@ -164,9 +164,7 @@ pub fn st_table_schema() -> TableSchema {
     )
     .with_type(StTableType::System)
     .with_column_constraint(Constraints::primary_key_auto(), StTableFields::TableId.col_id())
-    .unwrap()
     .with_column_index(StTableFields::TableName.col_id(), true)
-    .unwrap()
     .into_schema(ST_TABLES_ID)
 }
 
@@ -190,7 +188,6 @@ pub fn st_columns_schema() -> TableSchema {
         Constraints::unique(),
         (StColumnFields::TableId.col_id(), vec![StColumnFields::ColPos.col_id()]),
     )
-    .unwrap()
     .into_schema(ST_COLUMNS_ID)
 }
 
@@ -214,7 +211,6 @@ pub fn st_indexes_schema() -> TableSchema {
     .with_type(StTableType::System)
     // TODO: Unique constraint on index name?
     .with_column_constraint(Constraints::primary_key_auto(), StIndexFields::IndexId.col_id())
-    .unwrap()
     .into_schema(ST_INDEXES_ID)
 }
 
@@ -241,7 +237,6 @@ pub(crate) fn st_sequences_schema() -> TableSchema {
     .with_type(StTableType::System)
     // TODO: Unique constraint on sequence name?
     .with_column_constraint(Constraints::primary_key_auto(), StSequenceFields::SequenceId.col_id())
-    .unwrap()
     .into_schema(ST_SEQUENCES_ID)
 }
 
@@ -269,7 +264,6 @@ pub(crate) fn st_constraints_schema() -> TableSchema {
         Constraints::primary_key_auto(),
         StConstraintFields::ConstraintId.col_id(),
     )
-    .unwrap()
     .into_schema(ST_CONSTRAINTS_ID)
 }
 

crates/core/src/db/relational_db.rs

@@ -946,7 +946,6 @@ mod tests {
             }],
         )
         .with_column_constraint(Constraints::primary_key_auto(), ColId(0))
-        .unwrap()
     }
 
     #[test]
@@ -1039,8 +1038,7 @@ mod tests {
                 col_type: AlgebraicType::I64,
             }],
         )
-        .with_column_sequence(ColId(0))
-        .unwrap();
+        .with_column_sequence(ColId(0));
 
         let table_id = stdb.create_table(&mut tx, schema)?;
 
@@ -1174,8 +1172,7 @@ mod tests {
             is_unique: true,
             index_type: IndexType::BTree,
         }])
-        .with_column_constraint(Constraints::identity(), ColId(0))
-        .unwrap();
+        .with_column_constraint(Constraints::identity(), ColId(0));
 
         let table_id = stdb.create_table(&mut tx, schema)?;
 

crates/core/src/db/update.rs

@@ -323,7 +323,6 @@ mod tests {
                 ],
             )
             .with_column_constraint(Constraints::identity(), ColId(0))
-            .unwrap()
             .with_indexes(vec![IndexDef::btree(
                 "Person_id_unique".into(),
                 (ColId(0), vec![ColId(1)]),
@@ -346,7 +345,6 @@ mod tests {
             ],
         )
         .with_column_constraint(Constraints::identity(), ColId(0))
-        .unwrap()
         .with_indexes(vec![IndexDef::btree(
             "Person_id_and_name".into(),
             (ColId(0), vec![ColId(1)]),
@@ -401,8 +399,7 @@ mod tests {
                 },
             ],
         )
-        .with_column_constraint(Constraints::identity(), ColId(0))
-        .unwrap()];
+        .with_column_constraint(Constraints::identity(), ColId(0))];
 
         match schema_updates(current, proposed)? {
             SchemaUpdates::Tainted(tainted) => {

crates/core/src/error.rs

@@ -13,7 +13,7 @@ use spacetimedb_lib::buffer::DecodeError;
 use spacetimedb_lib::{PrimaryKey, ProductValue};
 use spacetimedb_primitives::*;
 use spacetimedb_sats::db::def::IndexDef;
-use spacetimedb_sats::db::error::{LibError, RelationError, SchemaError};
+use spacetimedb_sats::db::error::{LibError, RelationError, SchemaErrors};
 use spacetimedb_sats::hash::Hash;
 use spacetimedb_sats::product_value::InvalidFieldError;
 use spacetimedb_sats::relation::FieldName;
@@ -146,7 +146,7 @@ pub enum DBError {
     #[error("IndexError: {0}")]
     Index(#[from] IndexError),
     #[error("SchemaError: {0}")]
-    Schema(#[from] SchemaError),
+    Schema(SchemaErrors),
     #[error("IOError: {0}.")]
     IoError(#[from] std::io::Error),
     #[error("ParseIntError: {0}.")]

crates/core/src/sql/ast.rs

@@ -175,14 +175,14 @@ impl From {
 
     /// Returns all the fields matching `f` as a `Vec<FromField>`,
     /// including the ones inside the joins.
-    pub fn find_field(&self, f: &str) -> Result<Vec<FieldDef>, RelationError> {
+    pub fn find_field<'a>(&'a self, f: &'a str) -> Result<Vec<FieldDef>, RelationError> {
         let field = extract_table_field(f)?;
         let fields = self.iter_tables().flat_map(|t| {
             t.columns().iter().filter_map(|column| {
                 if column.col_name == field.field {
                     Some(FieldDef {
-                        column: column.clone(),
-                        table_name: field.table.unwrap_or(&t.table_name).to_string(),
+                        column,
+                        table_name: field.table.unwrap_or(&t.table_name),
                     })
                 } else {
                     None
@@ -195,7 +195,7 @@ impl From {
 
     /// Checks if the field `named` matches exactly once in all the tables
     /// including the ones inside the joins
-    pub fn resolve_field(&self, named: &str) -> Result<FieldDef, PlanError> {
+    pub fn resolve_field<'a>(&'a self, named: &'a str) -> Result<FieldDef, PlanError> {
         let fields = self.find_field(named)?;
 
         match fields.len() {