feat: Allow adding unique constraints to existing tables by Ludv1gL · Pull Request #4465 · clockworklabs/SpacetimeDB

Ludv1gL · 2026-02-26T00:19:43Z

Summary

Adding #[unique] or #[primary_key] to an existing column currently triggers AutoMigrateError::AddUniqueConstraint, forcing a full database clear to apply the schema change. This PR makes it a non-breaking migration by validating existing data first:

If all values are unique: constraint is added seamlessly (non-breaking migration)
If duplicates exist: migration fails with a detailed error listing up to 10 duplicate groups

Changes

auto_migrate.rs: Replace hard AddUniqueConstraint error with CheckAddUniqueConstraintValid precheck + AddConstraint migration step
update.rs: Implement precheck (full table scan, project constrained columns, count duplicates) and AddConstraint step execution
relational_db.rs: Expose create_constraint() (counterpart to existing drop_constraint())
traits.rs / datastore.rs: Add create_constraint_mut_tx to MutTxDatastore trait
mut_tx.rs: Make create_constraint public
formatter.rs: Format the new AddConstraint step

Safety

Transaction safety: Precheck and constraint creation run in the same MutTx — no window for concurrent duplicate inserts
Index creation: auto_migrate_indexes() already handles adding the backing btree index (with is_unique=true from the new schema). The constraint step only adds metadata.
Rollback: If the precheck finds duplicates, the entire migration aborts before any changes are applied
Error quality: Duplicate error shows table name, column names, and up to 10 example duplicate values with counts

Example error output

Precheck failed: cannot add unique constraint 'Users_email_key' on table 'Users' column(s) [email]:
3 duplicate group(s) found.
  - String("[email protected]") appears 2 times
  - String("[email protected]") appears 3 times
  - String("[email protected]") appears 2 times

Test plan

All 12 auto_migrate tests pass
cargo check passes for spacetimedb-schema and spacetimedb-core
Verified the previously-expected AddUniqueConstraint error test is updated
Manual test: add #[unique] to existing column with clean data → succeeds
Manual test: add #[unique] to existing column with duplicates → fails with detailed error

🤖 Generated with Claude Code

bfops · 2026-02-26T20:32:45Z

Interesting, thank you for filing this! We'll work on getting it reviewed.

Centril

This needs some work to actually implement the feature.

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 <[email protected]>

Ludv1gL · 2026-03-24T22:05:22Z

Rebased onto latest master (includes the MultiMap → BTreeIndex / UniqueMap → UniqueBTreeIndex rename from #4655). All changes updated to use the new type and file names.

Single squashed commit with all round 1 feedback incorporated. Ready for round 2 review.

…4465 review) Addresses Centril's review on PR clockworklabs#4465. The previous implementation only inserted metadata but never converted the in-memory index to unique. - Add make_unique/make_non_unique/iter_duplicates on TypedIndex/TableIndex - Rename create_constraint -> create_st_constraint (metadata-only) - New create_constraint makes index unique with can_merge + rollback - Remove CheckAddUniqueConstraintValid precheck - Add 6 transactionality tests Co-Authored-By: Claude Opus 4.6 <[email protected]>

Ludv1gL · 2026-04-02T14:50:11Z

Rebased onto latest master (cdd8ba77a) which includes #4311 (BytesKey optimization).

Changes for rebase compatibility:

hash_index.rs and unique_btree_index.rs: Our migration helper methods now coexist with the new relaxed-bound delete() and seek_point() methods from Bypass AlgebraicValue for datastore updates and bsatn based index scans + BytesKey optimization #4311 in the same impl block
mod.rs: define_uniqueness_conversions! macro closures updated for Packed<u128>/Packed<i128> — iter_duplicates() returns raw key types, so closures now wrap with Packed() when constructing AlgebraicValue
unique_btree_index.rs: Trait bound updated to K: KeySize + Ord (was K: Ord + KeySize) to match upstream ordering
Added use spacetimedb_sats::algebraic_value::Packed; import to mod.rs

All 15 constraint tests pass (cargo test -p spacetimedb-datastore --lib --features test -- constraint).

Centril

Getting there but there are still some issues to be worked out.

Ludv1gL · 2026-04-14T17:26:54Z

Rebased onto latest master and addressed all round 2 feedback. Single squashed commit.

Changes since last review:

.unwrap() → .expect() in AddConstraint handler (update.rs)
Backticks in AddUniqueConstraint comment (auto_migrate.rs)
Restored removed comments: Ensures: doc section, clone-write comments on both create_st_constraint and drop_st_constraint
Doc comment: /// Inserts constraint metadata... (imperative third-person)
Multiple indices on same columns: Added Table::get_indexes_by_cols() returning all matches. Both create_constraint and drop_constraint now iterate over all matching indices. PendingSchemaChange uses Vec<IndexId> instead of Option<IndexId>.
Rollback bug fixed: When can_merge() fails, both commit and tx indices are reverted to non-unique before returning the error. New test test_create_constraint_merge_error_reverts_uniqueness verifies rollback leaves the index non-unique.
Table::make_index_non_unique(): Encapsulates index revert + pointer map rebuild via rebuild_pointer_map() (no more inlined scan_rows().map().collect()). Used by drop_constraint for both commit and tx tables.
tx_table.take_pointer_map() moved into the if !had_unique branch alongside commit_table.take_pointer_map()
BytesKey indices added to define_uniqueness_conversions! (all 4 hash pairs: HashBytesKey8/24/56/120)
Test helpers merged: table_with_non_unique_index(cols: impl Into<ColList>) subsumes the multi-col variant
New tests: test_create_constraint_fails_with_tx_state_duplicates, test_create_constraint_merge_error_reverts_uniqueness
Removed rebase artifacts: duplicate RemoveTable variant, duplicate import

17 constraint tests pass. Full spacetimedb-standalone --release build clean (0 warnings).

Ludv1gL · 2026-04-20T22:31:10Z

Pushed 7 commits on top of 4d2568d9. Five of them directly address your 2026-04-20 review — one commit per comment, mapped below. The remaining two (0e9514d4, 0e6182e0) are proactive fixes for bugs surfaced while auditing the constraint-toggle rollback paths; they're on-theme with your round-1 concern about rollback correctness (thread on mut_tx.rs:1885). Both carry differential tests — I verified each FAILS on the pre-fix tree and PASSES with the fix.

Commit	Addresses
`00790a3f`	`mut_tx.rs:1735`, `mut_tx.rs:1794` (comment restoration)
`9e4cd3c6`	`datastore.rs:4229` (delete redundant test)
`ae5f5a65`	`datastore.rs:4260` (pre-rollback assertion)
`154d763a`	`table_index/mod.rs:1688` (TODO pointing at #4733)
`e6b66b02`	`mut_tx.rs:1828` (validate before `create_st_constraint`)
`0e9514d4`	follow-up: `ConstraintRemoved` rollback pointer-map invariant
`0e6182e0`	follow-up: Existed-path `last_mut()` clobber

I've left per-thread replies pointing at the exact SHAs. Happy to rebase or split anything further.

During auto-migration (spacetime publish), adding #[unique] or step instead of an error. Implementation: - AutoMigrateStep::AddConstraint variant in the schema planner - create_st_constraint (metadata-only, used by create_table) split from create_constraint (full index conversion + validation) - iter_duplicates() on TableIndex via SameKeyEntry::count() - make_unique() / make_non_unique() in-place index conversion - from_non_unique() / into_non_unique() on unique index types - define_uniqueness_conversions! macro covering all 40 variant pairs including BytesKey hash indices - Table::make_index_non_unique() with pointer map rebuild - Table::get_indexes_by_cols() for multi-index support - Transactional rollback via PendingSchemaChange with Vec<IndexId> - Revert indices on can_merge() failure before error propagation Tests: 8 new constraint tests covering create, drop, rollback, multi-col, tx-state duplicates, and merge-error revert scenarios.

- `create_st_constraint`: add back the `Requires:` bullets, rewording the `is_unique` bullet to reflect the new split — the caller is now responsible for backing-index uniqueness, or should use `Self::create_constraint` which does the conversion. - `drop_st_constraint`: restore the `// Remove constraint in transaction's insert table.` inline comment above the insert-table re-borrow. - `drop_constraint` (public): restore the `TODO(1.0) / NOTE(centril)` block after the `push_schema_change` call. Addresses review comments at mut_tx.rs:1735 and mut_tx.rs:1794.

…state_duplicates Its body is a strict prefix of test_create_constraint_merge_error_reverts_uniqueness. Addresses review comment at datastore.rs:4229.

Adds a pre-rollback duplicate insert to test_create_constraint_merge_error_reverts_uniqueness. Without the added assertion, the test could pass purely by virtue of rollback discarding the tx state — the new check proves the `make_non_unique` revert actually runs on the `create_constraint` error path before returning, while the failing tx is still open. Addresses review comment at datastore.rs:4260.

…abs#4733 The uniqueness-conversion macro has `HashBytesKey{8,24,56,120}` entries but no btree counterparts. Adding them needs `RangeCompatBytesKey` (a range-compatible ordering over bytes keys), which is introduced in clockworklabs#4733. A TODO inside the btree arm of `define_uniqueness_conversions!` makes the dependency explicit; the follow-up will also revisit the `HashBytesKey*` closures flagged in the review. Addresses review comment at table_index/mod.rs:1688.

Previously `create_constraint` silently no-op'd in two cases: when the constraint was not a unique one (other `ConstraintData` variants), and when no existing index covered the constrained columns. Both cases left an orphan `st_constraint` row. Both checks now fire before `create_st_constraint` is called, and both return descriptive errors: - non-unique constraint kind → "adding non-unique constraints is not supported" - no backing index → "unique constraint on table N column(s) ... requires at least one backing index on those columns" The rest of `create_constraint` is unchanged; the previously nested `if let Some(cols) / if !index_ids.is_empty()` branches become unconditional post-validation flow. A `debug_assert!` encodes the pre-validation invariant at the re-borrow site. Adds `test_create_constraint_fails_without_backing_index` which asserts both the error message and that `constraint_id_from_name` returns `None` afterward (proving no orphan st_constraint row was written before the check fired). Addresses review comment at mut_tx.rs:1828.

…ollback Forward `drop_constraint` calls `Table::make_index_non_unique`, which rebuilds the table's pointer map when no unique index remains (a unique index subsumes the map, so dropping the last one must reinstate the map). The `ConstraintRemoved` rollback arm previously only called `make_unique` on each index — the rebuilt map was never discarded. Result: a rolled-back table ended up with BOTH a unique index AND a pointer map, violating the invariant at `table.rs` ("pointer map is present iff no unique index exists"). Fix: after restoring the unique indices, unconditionally call `take_pointer_map`. `Option::take` is idempotent, so it's safe whether or not the forward path rebuilt anything. Also exposes `Table::has_pointer_map` (symmetric with `has_unique_index`) so the new test can probe the invariant directly. Adds `test_drop_constraint_rollback_restores_pointer_map_invariant`. The test uses a schema with exactly one unique constraint so the forward rebuild path actually fires; confirmed to FAIL without the rollback fix and PASS with it. Thematically on-topic with the round-1 review concern about rollback correctness for constraint-toggle schema changes (thread 2878384473, `ConstraintAdded` side).

…onstraint row `create_st_constraint` short-circuits on `RowRefInsertion::Existed` (byte-identical row already present in the tx insert table) without pushing a `PendingSchemaChange::ConstraintAdded`. The old `create_constraint` then unconditionally overwrote `pending_schema_changes.last_mut()` — silently clobbering whatever unrelated change happened to be at the tail of the list. Fix: - `create_st_constraint` now returns `(ConstraintId, bool)`; the bool is `true` iff a new row was inserted (and therefore a `ConstraintAdded` was pushed). - `create_constraint` short-circuits with `return Ok(constraint_id)` when the row already existed — skipping the `last_mut()` overwrite AND the redundant `make_unique`/`can_merge`/pointer-map work (the indices are already in the correct state, since an earlier call in the same tx already converted them). - `create_table` ignores the new bool (a fresh table's `st_constraint` is empty, so every row is newly inserted). Adds `test_create_constraint_is_idempotent_and_does_not_clobber_pending_changes` which injects an unrelated marker `PendingSchemaChange::TableAdded(SENTINEL)` at the tail of the pending list, then forces the Existed path by re-calling `create_constraint` with an identical `ConstraintSchema` (matching constraint_id). Confirmed to FAIL on pre-fix code (marker clobbered) and PASS on post-fix.

`TableIndex::indexed_columns` became private in upstream clockworklabs#4782; our new `get_indexes_by_cols` helper and the `merge`-path violation projection both accessed it as a field. Switch to the accessor.

Thread the index's key type through `TypedIndex::iter_duplicates` and the `define_uniqueness_conversions!` macro so bytes-keyed variants can round- trip their keys back to `AlgebraicValue` via `TypedIndexKey`'s existing `decode_algebraic_value` path. - Add `BTreeBytesKey{8,16,32,64,128} <=> UniqueBTreeBytesKey*` conversion pairs to the macro, now unblocked by upstream clockworklabs#4733 providing `RangeCompatBytesKey`. - Replace the `HashBytesKey{8,24,56,120}` placeholder closures (which debug-stringified the raw bytes into `AlgebraicValue::String`) with the correct `TypedIndexKey::BytesKey*H(*k).into_algebraic_value(ty)` round-trip; reviewer-requested "via `IndexKey` → `AlgebraicValue`" shape. - Drop the `TODO(clockworklabs#4733)` comment and grow all closures to accept the `&AlgebraicType` key-type parameter (ignored by scalar variants).

Previously `create_constraint` scanned the committed index once via `iter_duplicates` to produce a detailed error message, then called `make_unique` on each matching index — which internally scans the index again to find the first duplicate. On the happy path (data is already unique), both scans are O(n) full-table walks. Fold the two into one pass: call `make_unique` first and let it fail fast. Only call `iter_duplicates` on the failure path, to build the human-readable error. Also factor the "revert previously-made-unique indices" cleanup into a closure shared by both the duplicate path and the tx-merge-conflict path, and detect tx-local duplicates (same-tx inserts prior to the constraint add) rather than assuming the tx table is clean.

Clippy flagged the `BTreeString` / `HashString` closure receivers as `&Box<str>` where `&str` works via deref coercion. Switch to `&str` and use `k.into()` on the resulting reference for the `Box<str>` expected by `AlgebraicValue::String`.

Ludv1gL · 2026-04-23T20:20:41Z

Rebased onto current upstream/master (includes #4733 bytes-key btrees, #4782 index housekeeping, #4850 replay refactor, #4666 primary-key migration). Force-pushed — SHAs changed, mapping below.

Deferred round-3 items resolved (`9d4f4a9bcb`)

Now that #4733 has landed, the two table_index/mod.rs items are addressed:

mod.rs:1688 — 5 new BTreeBytesKey{8,16,32,64,128} <=> UniqueBTreeBytesKey* pairs added to define_uniqueness_conversions!, using the new RangeCompatBytesKey<N>.
mod.rs:1717 — HashBytesKey{8,24,56,120} closures rewritten to go through TypedIndexKey::*.into_algebraic_value(key_type) per your "via IndexKey → AlgebraicValue" hint. The format!(\"{k:?}\") placeholder is gone.

To thread the key type through, the macro-generated iter_duplicates grew a key_type: &AlgebraicType parameter, TableIndex::iter_duplicates passes &self.key_type, and all ~33 pre-existing scalar closures now accept (and ignore) the extra arg.

Other rebase work

480377251e — adopt accessor indexed_columns() for the field privatized in Indices: house keeping (privatize stuff + fix minor bug in insert_index) #4782.
78fcb7018d — eliminated a double-scan in create_constraint. Was running iter_duplicates up front, then make_unique (which scans internally). Now make_unique runs first and iter_duplicates only fires on the failure path for the detailed error message. Also catches tx-local duplicates (was previously a latent .expect(\"tx table should have no duplicates\")).
7aea551466 — clippy: &str over &Box<str> in the two string closures.

SHA mapping for previously-verified commits

Old SHA	New SHA	Subject
`4d2568d9`	`87ec15e0`	feat: Allow adding unique constraints to existing tables
`00790a3f`	`4cd9e232`	docs(mut_tx): restore pre-split doc blocks on constraint methods
`9e4cd3c6`	`84d0fbaf`	test(datastore): drop redundant test_create_constraint_fails_with_tx_state_duplicates
`ae5f5a65`	`070ee5f7`	test(datastore): verify merge-error revert happens inside the failing tx
`154d763a`	`c7cffdce`	docs(table_index): note BTreeBytesKey follow-up (TODO now deleted by `9d4f4a9b`)
`e6b66b02`	`d0b3b149`	feat(mut_tx): validate constraint data before writing st_constraint
`0e9514d4`	`85c6a217`	fix(committed_state): drop rebuilt pointer map on ConstraintRemoved rollback
`0e6182e0`	`3479d85e`	fix(mut_tx): short-circuit create_constraint on already-existing st_constraint row

Validation

cargo test -p spacetimedb-datastore --features test — 93/93 pass (incl. 10 constraint tests)
cargo test -p spacetimedb-schema — 101/101
cargo test -p spacetimedb-core (update) — 14/14
cargo test -p spacetimedb-table — 103/103
cargo build -j32 -p spacetimedb-standalone --release — clean
cargo clippy --tests on touched crates — clean

…or tx_table Addresses Centril's round-3 review on PR clockworklabs#4465 (mut_tx.rs:1939). Extract the "Cannot add unique constraint … N duplicate group(s) found" error-building block into a `dup_err` closure so both the committed-state and tx-state `make_unique` failure paths surface the same human-readable list of duplicate groups. Previously, only the committed-state path ran `iter_duplicates()`; the tx-state path returned a generic "duplicate values exist in the current transaction" string. The closure takes a `source` string ("committed state" / "current transaction") so the user can tell which side fired. No semantic change to the success path. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

Ludv1gL · 2026-04-24T20:26:13Z

New commit dc7fee3324 addresses mut_tx.rs:1939.

Extracted a dup_err closure that runs iter_duplicates() and formats the "N duplicate group(s) found" error, then reused it on both the committed-state and tx-state make_unique failure paths. The tx-state path used to return only a generic "duplicate values exist in the current transaction" — now both surfaces list up to 10 duplicate groups. A source string ("committed state" / "current transaction") distinguishes which side fired.

Validated: 19/19 test_*unique* + test_*create_constraint* + test_*drop_constraint* pass; cargo clippy clean.

CLAassistant · 2026-05-08T14:52:50Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ Ludv1gL
❌ Centril
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

Centril

Thanks! This looks good now.

Centril self-requested a review February 26, 2026 13:34

Centril requested changes Mar 3, 2026

View reviewed changes

bfops assigned Centril Mar 3, 2026

Ludv1gL mentioned this pull request Mar 4, 2026

feat: Make create_constraint enforce index uniqueness #4542

Closed

4 tasks

Ludv1gL force-pushed the feat/allow-add-unique-constraint branch from 9b7d651 to 4c2518c Compare March 4, 2026 17:13

Ludv1gL force-pushed the feat/allow-add-unique-constraint branch from 4c2518c to 27d321b Compare March 24, 2026 21:57

Ludv1gL force-pushed the feat/allow-add-unique-constraint branch from 27d321b to f331dc8 Compare April 2, 2026 14:45

Ludv1gL requested review from bfops, cloutiertyler and jdetter as code owners April 2, 2026 14:45

Ludv1gL force-pushed the feat/allow-add-unique-constraint branch from f331dc8 to 2dea430 Compare April 2, 2026 14:49

Centril requested changes Apr 13, 2026

View reviewed changes

Ludv1gL force-pushed the feat/allow-add-unique-constraint branch from 2dea430 to 4d2568d Compare April 14, 2026 17:26

Centril requested changes Apr 20, 2026

View reviewed changes

Ludv1gL added 9 commits April 23, 2026 21:30

test(datastore): drop redundant test_create_constraint_fails_with_tx_…

84d0fba

…state_duplicates Its body is a strict prefix of test_create_constraint_merge_error_reverts_uniqueness. Addresses review comment at datastore.rs:4229.

fixup(rebase): adopt post-clockworklabs#4782 private-field accessors

4803772

`TableIndex::indexed_columns` became private in upstream clockworklabs#4782; our new `get_indexes_by_cols` helper and the `merge`-path violation projection both accessed it as a field. Switch to the accessor.

Ludv1gL added 3 commits April 23, 2026 21:35

Ludv1gL force-pushed the feat/allow-add-unique-constraint branch from 0e6182e to 7aea551 Compare April 23, 2026 20:20

Centril requested changes Apr 24, 2026

View reviewed changes

Comment thread crates/datastore/src/locking_tx_datastore/mut_tx.rs Outdated

Merge branch 'master' into feat/allow-add-unique-constraint

8e72a82

Centril approved these changes May 8, 2026

View reviewed changes

Conversation

Ludv1gL commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Safety

Example error output

Test plan

Uh oh!

bfops commented Feb 26, 2026

Uh oh!

Centril left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ludv1gL commented Mar 24, 2026

Uh oh!

Ludv1gL commented Apr 2, 2026

Uh oh!

Centril left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ludv1gL commented Apr 14, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ludv1gL commented Apr 20, 2026

Uh oh!

Ludv1gL commented Apr 23, 2026

Deferred round-3 items resolved (9d4f4a9bcb)

Other rebase work

SHA mapping for previously-verified commits

Validation

Uh oh!

Uh oh!

Ludv1gL commented Apr 24, 2026

Uh oh!

CLAassistant commented May 8, 2026

Uh oh!

Centril left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Ludv1gL commented Feb 26, 2026 •

edited

Loading

Deferred round-3 items resolved (`9d4f4a9bcb`)