Skip to content

Commit 4c2518c

Browse files
Ludv1gLclaude
andcommitted
feat: Make create_constraint actually enforce index uniqueness
Addresses review feedback on PR clockworklabs#4465. The previous implementation only inserted metadata into system tables but never converted the in-memory index from non-unique to unique. Changes: - Add SameKeyEntry::count(), iter_duplicates(), and check_and_into_unique() on MultiMap/HashIndex for duplicate detection via existing index infra - Add from_non_unique()/into_non_unique() on UniqueMap/UniqueHashIndex for lossless conversion between unique and non-unique index types - Add TypedIndex::make_unique()/make_non_unique()/iter_duplicates() via define_uniqueness_conversions! macro covering all 36 variant pairs - Add Table::take_pointer_map()/restore_pointer_map()/has_unique_index() - Rename create_constraint -> create_st_constraint (metadata-only, used by create_table), add new create_constraint that calls create_st_constraint then makes index unique on both tx and commit tables with can_merge check - Same pattern for drop_constraint/drop_st_constraint - Enrich PendingSchemaChange::ConstraintAdded with IndexId and PointerMap for correct rollback (make_non_unique + restore pointer map) - Remove CheckAddUniqueConstraintValid precheck (duplicate detection now happens inside create_constraint using the index directly) - Add MutTxDatastore::create_constraint_mut_tx and RelationalDB wrapper - Add AddConstraint formatter and remove dead AddUniqueConstraint error - Add 6 transactionality tests for create/drop constraint Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 8cb2038 commit 4c2518c

16 files changed

Lines changed: 816 additions & 74 deletions

File tree

crates/core/src/db/relational_db.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ use spacetimedb_sats::{AlgebraicType, AlgebraicValue, ProductType, ProductValue}
5050
use spacetimedb_schema::def::{ModuleDef, TableDef, ViewDef};
5151
use spacetimedb_schema::reducer_name::ReducerName;
5252
use spacetimedb_schema::schema::{
53-
ColumnSchema, IndexSchema, RowLevelSecuritySchema, Schema, SequenceSchema, TableSchema,
53+
ColumnSchema, ConstraintSchema, IndexSchema, RowLevelSecuritySchema, Schema, SequenceSchema, TableSchema,
5454
};
5555
use spacetimedb_schema::table_name::TableName;
5656
use spacetimedb_snapshot::{ReconstructedSnapshot, SnapshotError, SnapshotRepository};
@@ -1482,6 +1482,15 @@ impl RelationalDB {
14821482
Ok(self.inner.drop_sequence_mut_tx(tx, seq_id)?)
14831483
}
14841484

1485+
/// Creates a constraint, making the corresponding index unique if applicable.
1486+
pub fn create_constraint(
1487+
&self,
1488+
tx: &mut MutTx,
1489+
constraint: ConstraintSchema,
1490+
) -> Result<ConstraintId, DBError> {
1491+
Ok(self.inner.create_constraint_mut_tx(tx, constraint)?)
1492+
}
1493+
14851494
///Removes the [Constraints] from database instance
14861495
pub fn drop_constraint(&self, tx: &mut MutTx, constraint_id: ConstraintId) -> Result<(), DBError> {
14871496
Ok(self.inner.drop_constraint_mut_tx(tx, constraint_id)?)

crates/core/src/db/update.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use spacetimedb_lib::db::auth::StTableType;
66
use spacetimedb_lib::identity::AuthCtx;
77
use spacetimedb_lib::AlgebraicValue;
88
use spacetimedb_primitives::{ColSet, TableId};
9+
use spacetimedb_schema::schema::ConstraintSchema;
910
use spacetimedb_schema::auto_migrate::{AutoMigratePlan, ManualMigratePlan, MigratePlan};
1011
use spacetimedb_schema::def::{TableDef, ViewDef};
1112
use spacetimedb_schema::schema::{column_schemas_from_defs, IndexSchema, Schema, SequenceSchema, TableSchema};
@@ -220,6 +221,24 @@ fn auto_migrate_database(
220221
);
221222
stdb.drop_constraint(tx, constraint_schema.constraint_id)?;
222223
}
224+
spacetimedb_schema::auto_migrate::AutoMigrateStep::AddConstraint(constraint_name) => {
225+
let table_def = plan.new.stored_in_table_def(constraint_name).unwrap();
226+
let constraint_def = &table_def.constraints[constraint_name];
227+
let table_id = stdb.table_id_from_name_mut(tx, &table_def.name)?.unwrap();
228+
let constraint_schema = ConstraintSchema::from_module_def(
229+
plan.new,
230+
constraint_def,
231+
table_id,
232+
spacetimedb_primitives::ConstraintId::SENTINEL,
233+
);
234+
log!(
235+
logger,
236+
"Adding constraint `{}` on table `{}`",
237+
constraint_name,
238+
table_def.name
239+
);
240+
stdb.create_constraint(tx, constraint_schema)?;
241+
}
223242
spacetimedb_schema::auto_migrate::AutoMigrateStep::AddSequence(sequence_name) => {
224243
let table_def = plan.new.stored_in_table_def(sequence_name).unwrap();
225244
let sequence_def = table_def.sequences.get(sequence_name).unwrap();

crates/datastore/src/locking_tx_datastore/committed_state.rs

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ impl CommittedState {
149149
}
150150

151151
/// Returns the views that perform a full scan of this table
152-
pub(super) fn views_for_table_scan(&self, table_id: &TableId) -> impl Iterator<Item = &ViewCallInfo> + use<'_> {
152+
pub(super) fn views_for_table_scan(&self, table_id: &TableId) -> impl Iterator<Item = &ViewCallInfo> {
153153
self.read_sets.views_for_table_scan(table_id)
154154
}
155155

@@ -158,7 +158,7 @@ impl CommittedState {
158158
&'a self,
159159
table_id: &TableId,
160160
row_ref: RowRef<'a>,
161-
) -> impl Iterator<Item = &'a ViewCallInfo> + use<'a> {
161+
) -> impl Iterator<Item = &'a ViewCallInfo> {
162162
self.read_sets.views_for_index_seek(table_id, row_ref)
163163
}
164164
}
@@ -1324,14 +1324,30 @@ impl CommittedState {
13241324
.unwrap_or_else(|e| match e {});
13251325
}
13261326
// A constraint was removed. Add it back.
1327-
ConstraintRemoved(table_id, constraint_schema) => {
1327+
ConstraintRemoved(table_id, constraint_schema, index_id) => {
13281328
let table = self.tables.get_mut(&table_id)?;
13291329
table.with_mut_schema(|s| s.update_constraint(constraint_schema));
1330+
// If the constraint had a unique index, make it unique again.
1331+
if let Some(index_id) = index_id {
1332+
if let Some(idx) = table.indexes.get_mut(&index_id) {
1333+
idx.make_unique().expect("rollback: index should have no duplicates");
1334+
}
1335+
}
13301336
}
13311337
// A constraint was added. Remove it.
1332-
ConstraintAdded(table_id, constraint_id) => {
1338+
ConstraintAdded(table_id, constraint_id, index_id, pointer_map) => {
13331339
let table = self.tables.get_mut(&table_id)?;
13341340
table.with_mut_schema(|s| s.remove_constraint(constraint_id));
1341+
// If the constraint made an index unique, revert it to non-unique.
1342+
if let Some(index_id) = index_id {
1343+
if let Some(idx) = table.indexes.get_mut(&index_id) {
1344+
idx.make_non_unique();
1345+
}
1346+
}
1347+
// Restore the pointer map if it was taken.
1348+
if let Some(pm) = pointer_map {
1349+
table.restore_pointer_map(pm);
1350+
}
13351351
}
13361352
// A sequence was removed. Add it back.
13371353
SequenceRemoved(table_id, seq, schema) => {

crates/datastore/src/locking_tx_datastore/datastore.rs

Lines changed: 208 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ use spacetimedb_sats::{memory_usage::MemoryUsage, Deserialize};
4242
use spacetimedb_schema::table_name::TableName;
4343
use spacetimedb_schema::{
4444
reducer_name::ReducerName,
45-
schema::{ColumnSchema, IndexSchema, SequenceSchema, TableSchema},
45+
schema::{ColumnSchema, ConstraintSchema, IndexSchema, SequenceSchema, TableSchema},
4646
};
4747
use spacetimedb_snapshot::{ReconstructedSnapshot, SnapshotRepository};
4848
use spacetimedb_table::{
@@ -575,6 +575,14 @@ impl MutTxDatastore for Locking {
575575
tx.sequence_id_from_name(sequence_name)
576576
}
577577

578+
fn create_constraint_mut_tx(
579+
&self,
580+
tx: &mut Self::MutTx,
581+
constraint: ConstraintSchema,
582+
) -> Result<ConstraintId> {
583+
tx.create_constraint(constraint)
584+
}
585+
578586
fn drop_constraint_mut_tx(&self, tx: &mut Self::MutTx, constraint_id: ConstraintId) -> Result<()> {
579587
tx.drop_constraint(constraint_id)
580588
}
@@ -1206,13 +1214,14 @@ impl<F: FnMut(u64)> spacetimedb_commitlog::payload::txdata::Visitor for ReplayVi
12061214
// TODO: avoid clone
12071215
Ok(schema) => schema.table_name.clone(),
12081216

1209-
Err(_) => match self.dropped_table_names.remove(&table_id) {
1210-
Some(name) => name,
1211-
_ => {
1217+
Err(_) => {
1218+
if let Some(name) = self.dropped_table_names.remove(&table_id) {
1219+
name
1220+
} else {
12121221
return self
12131222
.process_error(anyhow!("Error looking up name for truncated table {table_id:?}").into());
12141223
}
1215-
},
1224+
}
12161225
};
12171226

12181227
if let Err(e) = self.committed_state.replay_truncate(table_id).with_context(|| {
@@ -1300,7 +1309,7 @@ mod tests {
13001309
use spacetimedb_lib::error::ResultTest;
13011310
use spacetimedb_lib::st_var::StVarValue;
13021311
use spacetimedb_lib::{resolved_type_via_v9, ScheduleAt, TimeDuration};
1303-
use spacetimedb_primitives::{col_list, ArgId, ColId, ScheduleId, ViewId};
1312+
use spacetimedb_primitives::{col_list, ArgId, ColId, ColSet, ScheduleId, ViewId};
13041313
use spacetimedb_sats::algebraic_value::ser::value_serialize;
13051314
use spacetimedb_sats::bsatn::ToBsatn;
13061315
use spacetimedb_sats::layout::RowTypeLayout;
@@ -3975,4 +3984,197 @@ mod tests {
39753984
);
39763985
Ok(())
39773986
}
3987+
3988+
/// Helper: create a table with a non-unique btree index on `col_pos` but no constraints.
3989+
fn table_with_non_unique_index(col_pos: u16) -> TableSchema {
3990+
let indices = vec![IndexSchema::for_test(
3991+
"Foo_idx_btree",
3992+
BTreeAlgorithm::from(col_pos),
3993+
)];
3994+
basic_table_schema_with_indices(indices, Vec::<ConstraintSchema>::new())
3995+
}
3996+
3997+
/// Helper: create a table with a non-unique btree index on multiple columns but no constraints.
3998+
fn table_with_non_unique_multi_col_index(cols: impl Into<ColList>) -> TableSchema {
3999+
let indices = vec![IndexSchema::for_test(
4000+
"Foo_multi_idx_btree",
4001+
BTreeAlgorithm { columns: cols.into() },
4002+
)];
4003+
basic_table_schema_with_indices(indices, Vec::<ConstraintSchema>::new())
4004+
}
4005+
4006+
#[test]
4007+
fn test_create_constraint_makes_index_unique() -> ResultTest<()> {
4008+
let datastore = get_datastore()?;
4009+
4010+
// TX1: create table with non-unique index on col 0.
4011+
let mut tx = begin_mut_tx(&datastore);
4012+
let schema = table_with_non_unique_index(0);
4013+
let table_id = datastore.create_table_mut_tx(&mut tx, schema)?;
4014+
commit(&datastore, tx)?;
4015+
4016+
// TX2: insert unique rows and commit.
4017+
let mut tx = begin_mut_tx(&datastore);
4018+
insert(&datastore, &mut tx, table_id, &u32_str_u32(1, "Alice", 30))?;
4019+
insert(&datastore, &mut tx, table_id, &u32_str_u32(2, "Bob", 25))?;
4020+
commit(&datastore, tx)?;
4021+
4022+
// TX3: add unique constraint — should succeed since data is unique.
4023+
let mut tx = begin_mut_tx(&datastore);
4024+
let mut constraint = ConstraintSchema::unique_for_test("Foo_id_unique", 0u16);
4025+
constraint.table_id = table_id;
4026+
datastore.create_constraint_mut_tx(&mut tx, constraint)?;
4027+
4028+
// Inserting a duplicate should now fail (index is unique).
4029+
let dup_result = insert(&datastore, &mut tx, table_id, &u32_str_u32(1, "Charlie", 20));
4030+
assert!(dup_result.is_err(), "duplicate insert should fail after adding unique constraint");
4031+
commit(&datastore, tx)?;
4032+
4033+
Ok(())
4034+
}
4035+
4036+
#[test]
4037+
fn test_create_constraint_rollback_restores_non_unique() -> ResultTest<()> {
4038+
let datastore = get_datastore()?;
4039+
4040+
// TX1: create table with non-unique index on col 0.
4041+
let mut tx = begin_mut_tx(&datastore);
4042+
let schema = table_with_non_unique_index(0);
4043+
let table_id = datastore.create_table_mut_tx(&mut tx, schema)?;
4044+
commit(&datastore, tx)?;
4045+
4046+
// TX2: insert unique rows and commit.
4047+
let mut tx = begin_mut_tx(&datastore);
4048+
insert(&datastore, &mut tx, table_id, &u32_str_u32(1, "Alice", 30))?;
4049+
insert(&datastore, &mut tx, table_id, &u32_str_u32(2, "Bob", 25))?;
4050+
commit(&datastore, tx)?;
4051+
4052+
// TX3: add unique constraint, then rollback.
4053+
let mut tx = begin_mut_tx(&datastore);
4054+
let mut constraint = ConstraintSchema::unique_for_test("Foo_id_unique", 0u16);
4055+
constraint.table_id = table_id;
4056+
datastore.create_constraint_mut_tx(&mut tx, constraint)?;
4057+
let _ = datastore.rollback_mut_tx(tx);
4058+
4059+
// TX4: after rollback, duplicates should be allowed again.
4060+
let mut tx = begin_mut_tx(&datastore);
4061+
let result = insert(&datastore, &mut tx, table_id, &u32_str_u32(1, "Charlie", 20));
4062+
assert!(result.is_ok(), "duplicate insert should succeed after rollback of unique constraint");
4063+
Ok(())
4064+
}
4065+
4066+
#[test]
4067+
fn test_create_constraint_fails_with_duplicates() -> ResultTest<()> {
4068+
let datastore = get_datastore()?;
4069+
4070+
// TX1: create table with non-unique index on col 0.
4071+
let mut tx = begin_mut_tx(&datastore);
4072+
let schema = table_with_non_unique_index(0);
4073+
let table_id = datastore.create_table_mut_tx(&mut tx, schema)?;
4074+
commit(&datastore, tx)?;
4075+
4076+
// TX2: insert duplicate rows and commit.
4077+
let mut tx = begin_mut_tx(&datastore);
4078+
insert(&datastore, &mut tx, table_id, &u32_str_u32(1, "Alice", 30))?;
4079+
insert(&datastore, &mut tx, table_id, &u32_str_u32(1, "Bob", 25))?; // duplicate id=1
4080+
commit(&datastore, tx)?;
4081+
4082+
// TX3: try to add unique constraint — should fail.
4083+
let mut tx = begin_mut_tx(&datastore);
4084+
let mut constraint = ConstraintSchema::unique_for_test("Foo_id_unique", 0u16);
4085+
constraint.table_id = table_id;
4086+
let result = datastore.create_constraint_mut_tx(&mut tx, constraint);
4087+
assert!(result.is_err(), "create_constraint should fail when duplicates exist");
4088+
4089+
Ok(())
4090+
}
4091+
4092+
#[test]
4093+
fn test_create_constraint_multi_col() -> ResultTest<()> {
4094+
let datastore = get_datastore()?;
4095+
4096+
// TX1: create table with non-unique multi-column index on (col 0, col 2).
4097+
let mut tx = begin_mut_tx(&datastore);
4098+
let schema = table_with_non_unique_multi_col_index(col_list![0, 2]);
4099+
let table_id = datastore.create_table_mut_tx(&mut tx, schema)?;
4100+
commit(&datastore, tx)?;
4101+
4102+
// TX2: insert rows unique on (id, age) and commit.
4103+
let mut tx = begin_mut_tx(&datastore);
4104+
insert(&datastore, &mut tx, table_id, &u32_str_u32(1, "Alice", 30))?;
4105+
insert(&datastore, &mut tx, table_id, &u32_str_u32(1, "Bob", 25))?; // same id, different age
4106+
commit(&datastore, tx)?;
4107+
4108+
// TX3: add unique constraint on (col 0, col 2) — should succeed.
4109+
let mut tx = begin_mut_tx(&datastore);
4110+
let mut constraint = ConstraintSchema::unique_for_test(
4111+
"Foo_id_age_unique",
4112+
ColSet::from(col_list![0, 2]),
4113+
);
4114+
constraint.table_id = table_id;
4115+
datastore.create_constraint_mut_tx(&mut tx, constraint)?;
4116+
commit(&datastore, tx)?;
4117+
4118+
Ok(())
4119+
}
4120+
4121+
#[test]
4122+
fn test_drop_constraint_makes_index_non_unique() -> ResultTest<()> {
4123+
let datastore = get_datastore()?;
4124+
4125+
// TX1: create table with unique constraint.
4126+
let mut tx = begin_mut_tx(&datastore);
4127+
let schema = basic_table_schema_with_indices(basic_indices(), basic_constraints());
4128+
let table_id = datastore.create_table_mut_tx(&mut tx, schema)?;
4129+
commit(&datastore, tx)?;
4130+
4131+
// TX2: insert a row.
4132+
let mut tx = begin_mut_tx(&datastore);
4133+
insert(&datastore, &mut tx, table_id, &u32_str_u32(1, "Alice", 30))?;
4134+
commit(&datastore, tx)?;
4135+
4136+
// TX3: drop the unique constraint on col 0.
4137+
let mut tx = begin_mut_tx(&datastore);
4138+
let constraint_id = tx
4139+
.constraint_id_from_name("Foo_id_key")?
4140+
.expect("constraint should exist");
4141+
datastore.drop_constraint_mut_tx(&mut tx, constraint_id)?;
4142+
4143+
// Inserting a duplicate on col 0 should now succeed.
4144+
let result = insert(&datastore, &mut tx, table_id, &u32_str_u32(1, "Bob", 25));
4145+
assert!(result.is_ok(), "duplicate insert should succeed after dropping unique constraint");
4146+
commit(&datastore, tx)?;
4147+
4148+
Ok(())
4149+
}
4150+
4151+
#[test]
4152+
fn test_drop_constraint_rollback_keeps_unique() -> ResultTest<()> {
4153+
let datastore = get_datastore()?;
4154+
4155+
// TX1: create table with unique constraint.
4156+
let mut tx = begin_mut_tx(&datastore);
4157+
let schema = basic_table_schema_with_indices(basic_indices(), basic_constraints());
4158+
let table_id = datastore.create_table_mut_tx(&mut tx, schema)?;
4159+
commit(&datastore, tx)?;
4160+
4161+
// TX2: insert a row.
4162+
let mut tx = begin_mut_tx(&datastore);
4163+
insert(&datastore, &mut tx, table_id, &u32_str_u32(1, "Alice", 30))?;
4164+
commit(&datastore, tx)?;
4165+
4166+
// TX3: drop constraint, then rollback.
4167+
let mut tx = begin_mut_tx(&datastore);
4168+
let constraint_id = tx
4169+
.constraint_id_from_name("Foo_id_key")?
4170+
.expect("constraint should exist");
4171+
datastore.drop_constraint_mut_tx(&mut tx, constraint_id)?;
4172+
let _ = datastore.rollback_mut_tx(tx);
4173+
4174+
// TX4: after rollback, constraint should be back — duplicates should fail.
4175+
let mut tx = begin_mut_tx(&datastore);
4176+
let dup_result = insert(&datastore, &mut tx, table_id, &u32_str_u32(1, "Bob", 25));
4177+
assert!(dup_result.is_err(), "duplicate insert should fail after rollback of drop constraint");
4178+
Ok(())
4179+
}
39784180
}

0 commit comments

Comments
 (0)