diff --git a/crates/core/src/db/datastore/locking_tx_datastore/mod.rs b/crates/core/src/db/datastore/locking_tx_datastore/mod.rs index 0f7bc0beee..5c85517d07 100644 --- a/crates/core/src/db/datastore/locking_tx_datastore/mod.rs +++ b/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::{ @@ -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(()) } @@ -2968,12 +2973,11 @@ mod tests { IndexDef { columns: NonEmpty::new(1.into()), index_name: "name_idx".into(), - is_unique: false, + is_unique: true, index_type: IndexType::BTree, }, ]) .with_column_sequence(ColId(0)) - .unwrap() } #[rustfmt::skip] @@ -2984,11 +2988,11 @@ mod tests { map_array(basic_table_schema_cols()), map_array([ IdxSchema { id: 6, table: 6, col: 0, name: "id_idx", unique: true }, - IdxSchema { id: 7, table: 6, col: 1, name: "name_idx", unique: false }, + 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::unique(), constraint_name: "ct_Foo_id_idx_unique" }, - ConstraintRow { constraint_id: 7, table_id: 6, columns: col(1), constraints: Constraints::indexed(), constraint_name: "ct_Foo_name_idx_indexed" } + 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 } @@ -3072,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 }, @@ -3393,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 }, @@ -3436,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 }, @@ -3479,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 }, diff --git a/crates/core/src/db/datastore/system_tables.rs b/crates/core/src/db/datastore/system_tables.rs index 12262b7cd9..316de59c38 100644 --- a/crates/core/src/db/datastore/system_tables.rs +++ b/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) } diff --git a/crates/core/src/db/relational_db.rs b/crates/core/src/db/relational_db.rs index 83b1749fd3..0651d2a276 100644 --- a/crates/core/src/db/relational_db.rs +++ b/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)?; diff --git a/crates/core/src/db/update.rs b/crates/core/src/db/update.rs index 5f5069c156..5290b97829 100644 --- a/crates/core/src/db/update.rs +++ b/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) => { diff --git a/crates/core/src/error.rs b/crates/core/src/error.rs index a22efa8e7a..c4675b5835 100644 --- a/crates/core/src/error.rs +++ b/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}.")] diff --git a/crates/sats/src/db/def.rs b/crates/sats/src/db/def.rs index bbede5e7c4..888b8dcece 100644 --- a/crates/sats/src/db/def.rs +++ b/crates/sats/src/db/def.rs @@ -99,7 +99,10 @@ impl SequenceDef { /// let sequence_def = SequenceDef::for_column("my_table", "my_sequence", 1.into()); /// assert_eq!(sequence_def.sequence_name, "seq_my_table_my_sequence"); /// ``` - pub fn for_column(table: &str, seq_name: &str, col_pos: ColId) -> Self { + pub fn for_column(table: &str, column_or_name: &str, col_pos: ColId) -> Self { + //removes the auto-generated suffix... + let seq_name = column_or_name.trim_start_matches(&format!("ct_{}_", table)); + SequenceDef { sequence_name: format!("seq_{}_{}", table, seq_name), col_pos, @@ -218,14 +221,19 @@ impl IndexDef { /// let index_def = IndexDef::for_column("my_table", "test", NonEmpty::new(1u32.into()), true); /// assert_eq!(index_def.index_name, "idx_my_table_test_unique"); /// ``` - pub fn for_column(table: &str, index_name: &str, columns: impl Into>, is_unique: bool) -> Self { + pub fn for_column(table: &str, index_or_name: &str, columns: impl Into>, is_unique: bool) -> Self { let unique = if is_unique { "unique" } else { "non_unique" }; // Removes the auto-generated suffix from the index name. - let name = index_name.trim_start_matches(&format!("ct_{}_", table)); + let name = index_or_name.trim_start_matches(&format!("ct_{}_", table)); // Constructs the index name using a predefined format. - Self::btree(format!("idx_{table}_{name}_{unique}"), columns, is_unique) + // No duplicate the `kind_name` that was added by an constraint + if name.ends_with(&unique) { + Self::btree(format!("idx_{table}_{name}"), columns, is_unique) + } else { + Self::btree(format!("idx_{table}_{name}_{unique}"), columns, is_unique) + } } } @@ -433,16 +441,19 @@ impl ConstraintDef { /// ``` pub fn for_column( table: &str, - column_name: &str, + column_or_name: &str, constraints: Constraints, columns: impl Into>, ) -> Self { + //removes the auto-generated suffix... + let name = column_or_name.trim_start_matches(&format!("idx_{}_", table)); + let kind_name = format!("{:?}", constraints.kind()).to_lowercase(); // No duplicate the `kind_name` that was added by an index - if column_name.ends_with(&kind_name) { - Self::new(format!("ct_{table}_{column_name}"), constraints, columns) + if name.ends_with(&kind_name) { + Self::new(format!("ct_{table}_{name}"), constraints, columns) } else { - Self::new(format!("ct_{table}_{column_name}_{kind_name}"), constraints, columns) + Self::new(format!("ct_{table}_{name}_{kind_name}"), constraints, columns) } } } @@ -584,6 +595,10 @@ impl TableSchema { } } + pub fn get_columns(&self, columns: &NonEmpty) -> Vec<(ColId, Option<&ColumnSchema>)> { + columns.iter().map(|col| (*col, self.columns.get(col.idx()))).collect() + } + /// Get a reference to a column by its position (`pos`) in the table. pub fn get_column(&self, pos: usize) -> Option<&ColumnSchema> { self.columns.get(pos) @@ -732,55 +747,155 @@ impl TableSchema { /// Verify the definitions of this schema are valid: /// - Check all names are not empty + /// - All columns exists /// - Only 1 PK /// - Only 1 sequence per column /// - Only Btree Indexes - pub fn validated(self) -> Result { - let total_pk = self + pub fn validated(self) -> Result> { + let mut errors = Vec::new(); + + let pks: Vec<_> = self .column_constraints_iter() - .filter(|(_, ct)| ct.has_primary_key()) - .count(); - if total_pk > 1 { - return Err(SchemaError::MultiplePrimaryKeys(self.table_name.clone())); + .filter_map(|(cols, ct)| { + if ct.has_primary_key() { + Some( + self.get_columns(&cols) + .iter() + .map(|(col, schema)| { + if let Some(col) = schema { + col.col_name.clone() + } else { + format!("col_{col}") + } + }) + .collect(), + ) + } else { + None + } + }) + .collect(); + if pks.len() > 1 { + errors.push(SchemaError::MultiplePrimaryKeys { + table: self.table_name.clone(), + pks, + }); } if self.table_name.is_empty() { - return Err(SchemaError::EmptyTableName { - table_id: self.table_id.0, + errors.push(SchemaError::EmptyTableName { + table_id: self.table_id, }); } - if let Some(empty) = self.columns.iter().find(|x| x.col_name.is_empty()) { - return Err(SchemaError::EmptyName { - table: self.table_name.clone(), - ty: DefType::Column, - id: empty.col_pos.0, + let columns_not_found = self + .sequences + .iter() + .map(|x| (DefType::Sequence, x.sequence_name.clone(), NonEmpty::new(x.col_pos))) + .chain( + self.indexes + .iter() + .map(|x| (DefType::Index, x.index_name.clone(), x.columns.clone())), + ) + .chain( + self.constraints + .iter() + .map(|x| (DefType::Constraint, x.constraint_name.clone(), x.columns.clone())), + ) + .filter_map(|(ty, name, cols)| { + let empty: Vec<_> = self + .get_columns(&cols) + .iter() + .filter_map(|(col, x)| if x.is_none() { Some(*col) } else { None }) + .collect(); + + if empty.is_empty() { + None + } else { + Some(SchemaError::ColumnsNotFound { + name, + table: self.table_name.clone(), + columns: empty, + ty, + }) + } }); - } - if let Some(empty) = self.indexes.iter().find(|x| x.index_name.is_empty()) { - return Err(SchemaError::EmptyName { - table: self.table_name.clone(), - ty: DefType::Index, - id: empty.index_id.0, - }); - } + errors.extend(columns_not_found); - if let Some(empty) = self.constraints.iter().find(|x| x.constraint_name.is_empty()) { - return Err(SchemaError::EmptyName { - table: self.table_name.clone(), - ty: DefType::Constraint, - id: empty.constraint_id.0, - }); - } + errors.extend(self.columns.iter().filter_map(|x| { + if x.col_name.is_empty() { + Some(SchemaError::EmptyName { + table: self.table_name.clone(), + ty: DefType::Column, + id: x.col_pos.0, + }) + } else { + None + } + })); + + errors.extend(self.indexes.iter().filter_map(|x| { + if x.index_name.is_empty() || x.index_name.contains("__") { + Some(SchemaError::EmptyName { + table: self.table_name.clone(), + ty: DefType::Index, + id: x.index_id.0, + }) + } else { + None + } + })); + + //Verify not exist 'Constraints::unset()` they are equivalent to 'None' + errors.extend(self.constraints.iter().filter_map(|x| { + if x.constraints.kind() == ConstraintKind::UNSET { + Some(SchemaError::ConstraintUnset { + table: self.table_name.clone(), + name: x.constraint_name.clone(), + columns: x.columns.clone(), + }) + } else { + None + } + })); + + errors.extend(self.constraints.iter().filter_map(|x| { + if x.constraint_name.is_empty() || x.constraint_name.contains("__") { + Some(SchemaError::EmptyName { + table: self.table_name.clone(), + ty: DefType::Constraint, + id: x.constraint_id.0, + }) + } else { + None + } + })); + + errors.extend(self.sequences.iter().filter_map(|x| { + if x.sequence_name.is_empty() || x.sequence_name.contains("__") { + Some(SchemaError::EmptyName { + table: self.table_name.clone(), + ty: DefType::Sequence, + id: x.sequence_id.0, + }) + } else { + None + } + })); - if let Some(empty) = self.sequences.iter().find(|x| x.sequence_name.is_empty()) { - return Err(SchemaError::EmptyName { - table: self.table_name.clone(), - ty: DefType::Sequence, - id: empty.sequence_id.0, - }); - } + // We only support BTree indexes + errors.extend(self.indexes.iter().filter_map(|x| { + if x.index_type != IndexType::BTree { + Some(SchemaError::OnlyBtree { + table: self.table_name.clone(), + index: x.index_name.clone(), + index_type: x.index_type, + }) + } else { + None + } + })); // Verify we don't have more than 1 auto_inc for the same column if let Some(err) = self @@ -800,19 +915,14 @@ impl TableSchema { } }) { - return Err(err); + errors.push(err); } - // We only support BTree indexes - if let Some(idx) = self.indexes.iter().find(|x| x.index_type != IndexType::BTree) { - return Err(SchemaError::OnlyBtree { - table: self.table_name.clone(), - index: idx.index_name.clone(), - index_type: idx.index_type, - }); + if errors.is_empty() { + Ok(self) + } else { + Err(errors) } - - Ok(self) } } @@ -914,35 +1024,32 @@ impl TableDef { x } - fn generate_cols_name(&self, columns: &NonEmpty) -> Result { + /// Concatenate the column names from the `columns` + /// + /// WARNING: If the `ColId` not exist, is skipped. [TableSchema::validated] will find the error + fn generate_cols_name(&self, columns: &NonEmpty) -> String { let mut column_name = Vec::with_capacity(columns.len()); for col_pos in columns { if let Some(col) = self.get_column(col_pos.idx()) { column_name.push(col.col_name.as_str()) - } else { - todo!("with_column_constraint") } } - Ok(column_name.join("_")) + column_name.join("_") } /// Generate a [ConstraintDef] using the supplied `columns`. - pub fn with_column_constraint( - self, - kind: Constraints, - columns: impl Into>, - ) -> Result { + pub fn with_column_constraint(self, kind: Constraints, columns: impl Into>) -> Self { let mut x = self; let columns = columns.into(); x.constraints.push(ConstraintDef::for_column( &x.table_name, - &x.generate_cols_name(&columns)?, + &x.generate_cols_name(&columns), kind, columns, )); - Ok(x) + x } /// Set the indexes for the table and return a new `TableDef` instance with the updated indexes. @@ -953,16 +1060,16 @@ impl TableDef { } /// Generate a [IndexDef] using the supplied `columns`. - pub fn with_column_index(self, columns: impl Into>, is_unique: bool) -> Result { + pub fn with_column_index(self, columns: impl Into>, is_unique: bool) -> Self { let mut x = self; let columns = columns.into(); x.indexes.push(IndexDef::for_column( &x.table_name, - &x.generate_cols_name(&columns)?, + &x.generate_cols_name(&columns), columns, is_unique, )); - Ok(x) + x } /// Set the sequences for the table and return a new `TableDef` instance with the updated sequences. @@ -973,25 +1080,15 @@ impl TableDef { } /// Generate a [SequenceDef] using the supplied `columns`. - pub fn with_column_sequence(self, columns: impl Into>) -> Result { + pub fn with_column_sequence(self, columns: ColId) -> Self { let mut x = self; - let columns = columns.into(); - let col_pos = match columns.split_first() { - (col_pos, &[]) => *col_pos, - _ => { - return Err(SchemaError::OneAutoInc { - table: x.table_name, - field: x.columns[columns.head.idx()].col_name.clone(), - }) - } - }; x.sequences.push(SequenceDef::for_column( &x.table_name, - &x.generate_cols_name(&columns)?, - col_pos, + &x.generate_cols_name(&NonEmpty::new(columns)), + columns, )); - Ok(x) + x } /// Create a `TableDef` from a product type and table name. @@ -1012,6 +1109,8 @@ impl TableDef { } /// Get an iterator deriving [IndexDef] from the constraints that require them like `UNIQUE`. + /// + /// It looks into [Self::constraints] for possible duplicates and remove them from the result pub fn generated_indexes(&self) -> impl Iterator + '_ { self.constraints.iter().filter_map(|x| { if x.constraints.has_indexed() { @@ -1032,15 +1131,14 @@ impl TableDef { } /// Get an iterator deriving [SequenceDef] from the constraints that require them like `IDENTITY`. + /// + /// It looks into [Self::constraints] for possible duplicates and remove them from the result pub fn generated_sequences(&self) -> impl Iterator + '_ { self.constraints.iter().filter_map(|x| { if x.constraints.has_autoinc() { let col_id = x.columns.head; - //removes the auto-generated suffix... - let name = x - .constraint_name - .trim_start_matches(&format!("ct_{}_", self.table_name)); - let seq = SequenceDef::for_column(&self.table_name, name, col_id); + + let seq = SequenceDef::for_column(&self.table_name, &x.constraint_name, col_id); if self .sequences .binary_search_by(|x| x.sequence_name.cmp(&seq.sequence_name)) @@ -1055,6 +1153,9 @@ impl TableDef { }) } + /// Get an iterator deriving [ConstraintDef] from the indexes that require them like `UNIQUE`. + /// + /// It looks into [Self::constraints] for possible duplicates and remove them from the result pub fn generated_constraints(&self) -> impl Iterator + '_ { let cols: HashSet<_> = self .constraints @@ -1070,11 +1171,9 @@ impl TableDef { self.indexes.iter().filter_map(move |idx| { if !cols.contains(&idx.columns) { - //removes the auto-generated suffix... - let name = idx.index_name.trim_start_matches(&format!("idx_{}_", self.table_name)); Some(ConstraintDef::for_column( &self.table_name, - name, + &idx.index_name, if idx.is_unique { Constraints::unique() } else { @@ -1128,3 +1227,324 @@ impl From for TableDef { } } } + +#[cfg(test)] +mod tests { + use super::*; + + fn table_def() -> TableDef { + TableDef::new( + "test".into(), + vec![ + ColumnDef::sys("id", AlgebraicType::U64), + ColumnDef::sys("name", AlgebraicType::String), + ColumnDef::sys("age", AlgebraicType::I16), + ColumnDef::sys("x", AlgebraicType::F32), + ColumnDef::sys("y", AlgebraicType::F64), + ], + ) + } + + // Verify we generate indexes from constraints + #[test] + fn test_idx_generated() { + let t = table_def() + .with_column_constraint(Constraints::unique(), ColId(0)) + .with_column_constraint(Constraints::unique(), (ColId(0), vec![ColId(1)])) + .with_column_constraint(Constraints::indexed(), ColId(1)) + .with_column_constraint(Constraints::primary_key(), ColId(2)) + //This will be ignored + .with_column_constraint(Constraints::unset(), ColId(3)); + + let mut s = t.into_schema(TableId(0)).validated().unwrap(); + s.indexes.sort_by_key(|x| x.columns.clone()); + + #[rustfmt::skip] + assert_eq!( + s.indexes, + vec![ + IndexSchema::from_def(TableId(0), IndexDef::btree("idx_test_id_unique".into(), ColId(0), true)), + IndexSchema::from_def(TableId(0), IndexDef::btree("idx_test_id_name_unique".into(), (ColId(0), vec![ColId(1)]), true)), + IndexSchema::from_def(TableId(0), IndexDef::btree("idx_test_name_indexed_non_unique".into(), ColId(1), false)), + IndexSchema::from_def(TableId(0), IndexDef::btree("idx_test_age_primary_key_unique".into(), ColId(2), true)), + ] + ); + } + + // Verify we generate sequences from constraints + #[test] + fn test_seq_generated() { + let t = table_def() + .with_column_constraint(Constraints::identity(), ColId(0)) + .with_column_constraint(Constraints::primary_key_identity(), ColId(1)); + + let mut s = t.into_schema(TableId(0)).validated().unwrap(); + s.sequences.sort_by_key(|x| x.col_pos); + + #[rustfmt::skip] + assert_eq!( + s.sequences, + vec![ + SequenceSchema::from_def( + TableId(0), + SequenceDef::for_column("test", "id_identity", ColId(0)) + ), + SequenceSchema::from_def( + TableId(0), + SequenceDef::for_column("test", "name_primary_key_auto", ColId(1)) + ), + ] + ); + } + + // Verify we generate constraints from indexes + #[test] + fn test_ct_generated() { + let t = table_def() + .with_column_index(ColId(0), true) + .with_column_index(ColId(1), false) + .with_column_index((ColId(0), vec![ColId(1)]), true); + + let mut s = t.into_schema(TableId(0)).validated().unwrap(); + s.constraints.sort_by_key(|x| x.columns.clone()); + + #[rustfmt::skip] + assert_eq!( + s.constraints, + vec![ + ConstraintSchema::from_def( + TableId(0), + ConstraintDef::new("ct_test_id_unique".into(), Constraints::unique(), ColId(0)) + ), + ConstraintSchema::from_def( + TableId(0), + ConstraintDef::new("ct_test_id_name_unique".into(), Constraints::unique(), (ColId(0), vec![ColId(1)])) + ), + ConstraintSchema::from_def( + TableId(0), + ConstraintDef::new("ct_test_name_non_unique_indexed".into(), Constraints::indexed(), ColId(1)) + ), + ] + ); + } + + // Verify that if we add a Constraint + Index for the same column, we get at the end the correct definitions + #[test] + fn test_idx_ct_clash() { + // The `Constraint::unset()` should be removed + let t = table_def().with_column_index(ColId(0), true).with_constraints( + table_def() + .columns + .iter() + .enumerate() + .map(|(pos, x)| ConstraintDef::for_column("test", &x.col_name, Constraints::unset(), ColId(pos as u32))) + .collect(), + ); + + let s = t.into_schema(TableId(0)).validated(); + assert!(s.is_ok()); + + let s = s.unwrap(); + + assert_eq!( + s.indexes, + vec![IndexSchema::from_def( + TableId(0), + IndexDef::btree("idx_test_id_unique".into(), ColId(0), true) + )] + ); + assert_eq!( + s.constraints, + vec![ConstraintSchema::from_def( + TableId(0), + ConstraintDef::new("ct_test_id_unique".into(), Constraints::unique(), ColId(0)) + )] + ); + + // We got a duplication, both means 'UNIQUE' + let t = table_def() + .with_column_index(ColId(0), true) + .with_column_constraint(Constraints::unique(), ColId(0)); + + let s = t.into_schema(TableId(0)).validated(); + assert!(s.is_ok()); + + let s = s.unwrap(); + + assert_eq!( + s.indexes, + vec![IndexSchema::from_def( + TableId(0), + IndexDef::btree("idx_test_id_unique".into(), ColId(0), true) + )] + ); + assert_eq!( + s.constraints, + vec![ConstraintSchema::from_def( + TableId(0), + ConstraintDef::new("ct_test_id_unique".into(), Constraints::unique(), ColId(0)) + )] + ); + } + + // Not empty names + #[test] + fn test_validate_empty() { + let t = table_def(); + + fn empty(table: TableDef, ty: &[DefType], id: u32) { + assert_eq!( + table.into_schema(TableId(0)).validated(), + Err(ty + .iter() + .copied() + .map(|ty| SchemaError::EmptyName { + table: "test".to_string(), + ty, + id + }) + .collect()) + ) + } + // Empty names + let mut t_name = t.clone(); + t_name.table_name.clear(); + assert_eq!( + t_name.into_schema(TableId(0)).validated(), + Err(vec![SchemaError::EmptyTableName { table_id: TableId(0) }]) + ); + + let mut t_col = t.clone(); + t_col.columns.push(ColumnDef::sys("", AlgebraicType::U64)); + empty(t_col, &[DefType::Column], 5); + + let mut t_ct = t.clone(); + t_ct.constraints + .push(ConstraintDef::new("".into(), Constraints::primary_key(), ColId(0))); + empty(t_ct, &[DefType::Index, DefType::Constraint], 0); + + let mut t_idx = t.clone(); + t_idx.indexes.push(IndexDef::for_column("", "", ColId(0), false)); + empty(t_idx, &[DefType::Index, DefType::Constraint], 0); + + let mut t_seq = t.clone(); + t_seq.sequences.push(SequenceDef::for_column("", "", ColId(0))); + empty(t_seq, &[DefType::Sequence], 0); + } + + // Verify only one PK + #[test] + fn test_pkey() { + let t = table_def() + .with_column_constraint(Constraints::primary_key(), ColId(0)) + .with_column_constraint(Constraints::primary_key_auto(), ColId(1)) + .with_column_constraint(Constraints::primary_key_identity(), ColId(2)); + + assert_eq!( + t.into_schema(TableId(0)).validated(), + Err(vec![SchemaError::MultiplePrimaryKeys { + table: "test".into(), + pks: vec!["id".into(), "name".into(), "age".into()], + }]) + ); + } + + // All columns must exist + #[test] + fn test_column_exist() { + let t = table_def() + .with_column_sequence(ColId(1001)) + .with_column_constraint(Constraints::unique(), ColId(1002)) + .with_column_index(ColId(1003), false) + .with_column_sequence(ColId(1004)); + + let mut errs = t.into_schema(TableId(0)).validated().err().unwrap(); + errs.retain(|x| matches!(x, SchemaError::ColumnsNotFound { .. })); + + errs.sort_by_key(|x| { + if let SchemaError::ColumnsNotFound { columns, name, .. } = x { + (columns.clone(), name.clone()) + } else { + (Vec::new(), "".into()) + } + }); + + assert_eq!( + errs, + vec![ + SchemaError::ColumnsNotFound { + name: "seq_test_".to_string(), + table: "test".into(), + columns: vec![ColId(1001)], + ty: DefType::Sequence, + }, + SchemaError::ColumnsNotFound { + name: "ct_test__unique".to_string(), + table: "test".into(), + columns: vec![ColId(1002)], + ty: DefType::Constraint, + }, + SchemaError::ColumnsNotFound { + name: "idx_test__unique".to_string(), + table: "test".into(), + columns: vec![ColId(1002)], + ty: DefType::Index, + }, + SchemaError::ColumnsNotFound { + name: "ct_test__non_unique_indexed".to_string(), + table: "test".into(), + columns: vec![ColId(1003)], + ty: DefType::Constraint, + }, + SchemaError::ColumnsNotFound { + name: "idx_test__non_unique".to_string(), + table: "test".into(), + columns: vec![ColId(1003)], + ty: DefType::Index, + }, + SchemaError::ColumnsNotFound { + name: "seq_test_".to_string(), + table: "test".into(), + columns: vec![ColId(1004)], + ty: DefType::Sequence, + }, + ] + ); + } + + // Only one auto_inc + #[test] + fn test_validate_auto_inc() { + let t = table_def() + .with_column_sequence(ColId(0)) + .with_column_sequence(ColId(0)); + + assert_eq!( + t.into_schema(TableId(0)).validated(), + Err(vec![SchemaError::OneAutoInc { + table: "test".into(), + field: "id".into(), + }]) + ); + } + + // Only BTree indexes + #[test] + fn test_validate_btree() { + let t = table_def().with_indexes(vec![IndexDef { + index_name: "bad".to_string(), + is_unique: false, + index_type: IndexType::Hash, + columns: NonEmpty::new(0.into()), + }]); + + assert_eq!( + t.into_schema(TableId(0)).validated(), + Err(vec![SchemaError::OnlyBtree { + table: "test".into(), + index: "bad".to_string(), + index_type: IndexType::Hash, + }]) + ); + } +} diff --git a/crates/sats/src/db/error.rs b/crates/sats/src/db/error.rs index 285077103c..658cec68dc 100644 --- a/crates/sats/src/db/error.rs +++ b/crates/sats/src/db/error.rs @@ -3,6 +3,9 @@ use crate::product_value::InvalidFieldError; use crate::relation::{FieldName, Header}; use crate::{buffer, AlgebraicType, AlgebraicValue}; use derive_more::Display; +use nonempty::NonEmpty; +use spacetimedb_primitives::{ColId, TableId}; +use std::fmt; use std::string::FromUtf8Error; use thiserror::Error; @@ -111,21 +114,33 @@ pub enum RelationError { #[derive(Debug, Clone, Copy, Eq, PartialEq, PartialOrd, Ord, Display)] pub enum DefType { - Table, Column, Index, Sequence, Constraint, } -#[derive(thiserror::Error, Debug)] +#[derive(thiserror::Error, Debug, PartialEq)] pub enum SchemaError { - #[error("Multiple primary columns defined for table: {0}")] - MultiplePrimaryKeys(String), + #[error("Multiple primary columns defined for table: {table} columns: {pks:?}")] + MultiplePrimaryKeys { table: String, pks: Vec }, #[error("table id `{table_id}` should have name")] - EmptyTableName { table_id: u32 }, + EmptyTableName { table_id: TableId }, + #[error("{ty} {name} columns `{columns:?}` not found in table `{table}`")] + ColumnsNotFound { + name: String, + table: String, + columns: Vec, + ty: DefType, + }, #[error("table `{table}` {ty} should have name. {ty} id: {id}")] EmptyName { table: String, ty: DefType, id: u32 }, + #[error("table `{table}` have `Constraints::unset()` for columns: {columns:?}")] + ConstraintUnset { + table: String, + name: String, + columns: NonEmpty, + }, #[error("Attempt to define a column with more than 1 auto_inc sequence: Table: `{table}`, Field: `{field}`")] OneAutoInc { table: String, field: String }, #[error("Only Btree Indexes are supported: Table: `{table}`, Index: `{index}` is a `{index_type}`")] @@ -135,3 +150,12 @@ pub enum SchemaError { index_type: IndexType, }, } + +#[derive(thiserror::Error, Debug, PartialEq)] +pub struct SchemaErrors(pub Vec); + +impl fmt::Display for SchemaErrors { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_list().entries(self.0.iter()).finish() + } +}