Skip to content

Commit 57b7ef5

Browse files
Limit update() to only work on primary keys (#4279)
## Summary - Restricts `update()` across all three server-side SDKs (Rust, C#, TypeScript) to only work on primary key columns, not all unique columns - Calling `update()` on a non-PK unique column is semantically a delete+insert — clients won't see it as a row update unless the primary key stays the same - Fresh re-implementation of #1862, extended to cover C# and TypeScript bindings ### Design principle Primary key is a constraint on columns, not a property of indexes. An index is unique or not unique — whether `update()` is available is derived from the column's attributes at the point of use, not stored on the index. This differs from #1862 which introduced a `Uniqueness` enum (`No | Unique | PrimaryKey`) on the index itself. Instead, we keep `is_unique: bool` and check the column metadata where needed. ### Rust changes - Add `PrimaryKey` marker trait and `where Col: PrimaryKey` bound on `UniqueColumn::update()` - `marker_type()` receives `primary_key_column` and checks the column directly — no `is_pk` field on the index - `sdk-test` unique tables use `update_non_pk_by` (delete+insert) instead of `update_by` - Benchmarks that used `update()` on `#[unique]` columns changed to either `#[primary_key]` or delete+insert ### C# changes - Only emit `Update()` on `UniqueIndex` when the column has `PrimaryKey` attrs - Update `unique_*` test reducers in `sdk-test-cs` to use `Delete()`+`Insert()` instead ### TypeScript changes - `UniqueIndex` now has only `find` + `delete` (no `update`) - `AllColumnsPrimaryKey` type-level helper checks column metadata from the `TableDef` - `Index` routing conditionally intersects `{ update() }` when columns are PK - Runtime only attaches `update` method for PK unique indexes - `sdk-test-ts` unique tables use delete+insert instead of update - Type-level test in `schema.test-d.ts` verifies `id.update()` compiles (PK) and `name2.update` errors (non-PK unique) ## Test plan - [x] C# codegen tests pass (`dotnet test crates/bindings-csharp/Codegen.Tests`) - [x] Rust SDK test module compiles - [x] Benchmarks compile - [x] TypeScript type checks pass (`npx tsc --noEmit`) - [ ] CI passes Closes #1862
1 parent 626e209 commit 57b7ef5

17 files changed

Lines changed: 201 additions & 117 deletions

File tree

crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#FFI.verified.cs

Lines changed: 0 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/bindings-csharp/Codegen.Tests/fixtures/server/snapshots/Module#FFI.verified.cs

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1256,8 +1256,6 @@ internal BarUniqueIndex()
12561256
// C# generics don't play well with nullable types and can't accept both struct-type-based and class-type-based
12571257
// `globalName` in one generic definition, leading to buggy `Row?` expansion for either one or another.
12581258
public global::MultiTableRow? Find(uint key) => FindSingle(key);
1259-
1260-
public global::MultiTableRow Update(global::MultiTableRow row) => DoUpdate(row);
12611259
}
12621260

12631261
public BarUniqueIndex Bar => new();
@@ -1482,10 +1480,6 @@ internal Unique1UniqueIndex()
14821480
// `globalName` in one generic definition, leading to buggy `Row?` expansion for either one or another.
14831481
public global::RegressionMultipleUniqueIndexesHadSameName? Find(uint key) =>
14841482
FindSingle(key);
1485-
1486-
public global::RegressionMultipleUniqueIndexesHadSameName Update(
1487-
global::RegressionMultipleUniqueIndexesHadSameName row
1488-
) => DoUpdate(row);
14891483
}
14901484

14911485
internal Unique1UniqueIndex Unique1 => new();
@@ -1506,10 +1500,6 @@ internal Unique2UniqueIndex()
15061500
// `globalName` in one generic definition, leading to buggy `Row?` expansion for either one or another.
15071501
public global::RegressionMultipleUniqueIndexesHadSameName? Find(uint key) =>
15081502
FindSingle(key);
1509-
1510-
public global::RegressionMultipleUniqueIndexesHadSameName Update(
1511-
global::RegressionMultipleUniqueIndexesHadSameName row
1512-
) => DoUpdate(row);
15131503
}
15141504

15151505
internal Unique2UniqueIndex Unique2 => new();

crates/bindings-csharp/Codegen/Module.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -498,14 +498,17 @@ public IEnumerable<string> GenerateTableAccessorFilters(TableAccessor tableAcces
498498
continue;
499499
}
500500
var standardIndexName = ct.ToIndex().StandardIndexName(tableAccessor);
501+
var updateMethod = ct.Attr.HasFlag(ColumnAttrs.PrimaryKey)
502+
? $"public {globalName} Update({globalName} row) => DoUpdate(row);"
503+
: "";
501504
yield return $$"""
502505
{{vis}} sealed class {{f.Name}}UniqueIndex : {{uniqueIndexBase}}<{{tableAccessor.Name}}, {{globalName}}, {{f.Type.Name}}, {{f.Type.BSATNName}}> {
503506
internal {{f.Name}}UniqueIndex() : base("{{standardIndexName}}") {}
504507
// Important: don't move this to the base class.
505508
// C# generics don't play well with nullable types and can't accept both struct-type-based and class-type-based
506509
// `globalName` in one generic definition, leading to buggy `Row?` expansion for either one or another.
507510
public {{globalName}}? Find({{f.Type.Name}} key) => FindSingle(key);
508-
public {{globalName}} Update({{globalName}} row) => DoUpdate(row);
511+
{{updateMethod}}
509512
}
510513
{{vis}} {{f.Name}}UniqueIndex {{f.Name}} => new();
511514
""";

crates/bindings-macro/src/table.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,12 @@ impl ValidatedIndex<'_> {
499499
}
500500
}
501501

502-
fn marker_type(&self, vis: &syn::Visibility, tablehandle_ident: &Ident) -> TokenStream {
502+
fn marker_type(
503+
&self,
504+
vis: &syn::Visibility,
505+
tablehandle_ident: &Ident,
506+
primary_key_column: Option<&Column<'_>>,
507+
) -> TokenStream {
503508
let index_ident = self.accessor_name;
504509
let index_name = &self.index_name;
505510

@@ -560,6 +565,9 @@ impl ValidatedIndex<'_> {
560565
}
561566
}
562567
});
568+
if primary_key_column.is_some_and(|pk| col.ident == pk.ident) {
569+
decl.extend(quote!(impl spacetimedb::table::PrimaryKey for #index_ident {}));
570+
}
563571
}
564572
decl
565573
}
@@ -824,7 +832,10 @@ pub(crate) fn table_impl(mut args: TableArgs, item: &syn::DeriveInput) -> syn::R
824832
let index_accessors_ro = indices
825833
.iter()
826834
.map(|index| index.accessor(vis, original_struct_ident, &tablehandle_ident, AccessorType::Read));
827-
let index_marker_types = indices.iter().map(|index| index.marker_type(vis, &tablehandle_ident));
835+
let index_marker_types: Vec<_> = indices
836+
.iter()
837+
.map(|index| index.marker_type(vis, &tablehandle_ident, primary_key_column.as_ref()))
838+
.collect();
828839

829840
// Generate `integrate_generated_columns`
830841
// which will integrate all generated auto-inc col values into `_row`.

crates/bindings-typescript/src/lib/indexes.ts

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,35 @@ export type Indexes<
4747
[k in keyof I]: Index<TableDef, I[k]>;
4848
};
4949

50+
/**
51+
* Check whether every column in an index is a primary key column.
52+
*/
53+
type AllColumnsPrimaryKey<
54+
TableDef extends UntypedTableDef,
55+
Columns extends readonly string[],
56+
> = Columns extends readonly [
57+
infer Head extends keyof TableDef['columns'] & string,
58+
...infer Tail extends readonly string[],
59+
]
60+
? TableDef['columns'][Head]['columnMetadata'] extends { isPrimaryKey: true }
61+
? AllColumnsPrimaryKey<TableDef, Tail>
62+
: false
63+
: true;
64+
5065
/**
5166
* A type representing a database index,
5267
* which can either be unique or filter for a single value or range of values.
68+
* Unique indexes on primary key columns additionally support `update`.
5369
*/
5470
export type Index<
5571
TableDef extends UntypedTableDef,
5672
I extends UntypedIndex<keyof TableDef['columns'] & string>,
5773
> = I['unique'] extends true
58-
? UniqueIndex<TableDef, I>
74+
? AllColumnsPrimaryKey<TableDef, I['columns']> extends true
75+
? UniqueIndex<TableDef, I> & {
76+
update(row: Prettify<RowType<TableDef>>): Prettify<RowType<TableDef>>;
77+
}
78+
: UniqueIndex<TableDef, I>
5979
: I['algorithm'] extends 'hash'
6080
? PointIndex<TableDef, I>
6181
: RangedIndex<TableDef, I>;
@@ -103,7 +123,6 @@ export interface UniqueIndex<
103123
I extends UntypedIndex<keyof TableDef['columns'] & string>,
104124
> extends ReadonlyUniqueIndex<TableDef, I> {
105125
delete(colVal: IndexVal<TableDef, I>): boolean;
106-
update(colVal: Prettify<RowType<TableDef>>): Prettify<RowType<TableDef>>;
107126
}
108127

109128
/**

crates/bindings-typescript/src/server/runtime.ts

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,11 @@ function makeTableView(
538538
.filter(x => x.data.tag === 'Unique')
539539
.some(x => columnSet.isSubsetOf(new Set(x.data.value.columns)));
540540

541+
const isPrimaryKey =
542+
isUnique &&
543+
column_ids.length === table.primaryKey.length &&
544+
column_ids.every((id, i) => table.primaryKey[i] === id);
545+
541546
const indexSerializers = column_ids.map(id =>
542547
AlgebraicType.makeSerializer(
543548
rowType.value.elements[id].algebraicType,
@@ -574,7 +579,7 @@ function makeTableView(
574579
let index: Index<any, any>;
575580
if (isUnique && serializeSinglePoint) {
576581
// numColumns == 1, unique index
577-
index = {
582+
const base = {
578583
find: (colVal: IndexVal<any, any>): RowType<any> | null => {
579584
const buf = LEAF_BUF;
580585
const point_len = serializeSinglePoint(buf, colVal);
@@ -595,7 +600,9 @@ function makeTableView(
595600
);
596601
return num > 0;
597602
},
598-
update: (row: RowType<any>): RowType<any> => {
603+
};
604+
if (isPrimaryKey) {
605+
(base as any).update = (row: RowType<any>): RowType<any> => {
599606
const buf = LEAF_BUF;
600607
BINARY_WRITER.reset(buf);
601608
serializeRow(BINARY_WRITER, row);
@@ -607,11 +614,12 @@ function makeTableView(
607614
);
608615
integrateGeneratedColumns?.(row, buf.view);
609616
return row;
610-
},
611-
} as UniqueIndex<any, any>;
617+
};
618+
}
619+
index = base as UniqueIndex<any, any>;
612620
} else if (isUnique) {
613621
// numColumns != 1, unique index
614-
index = {
622+
const base = {
615623
find: (colVal: IndexVal<any, any>): RowType<any> | null => {
616624
if (colVal.length !== numColumns) {
617625
throw new TypeError('wrong number of elements');
@@ -638,7 +646,9 @@ function makeTableView(
638646
);
639647
return num > 0;
640648
},
641-
update: (row: RowType<any>): RowType<any> => {
649+
};
650+
if (isPrimaryKey) {
651+
(base as any).update = (row: RowType<any>): RowType<any> => {
642652
const buf = LEAF_BUF;
643653
BINARY_WRITER.reset(buf);
644654
serializeRow(BINARY_WRITER, row);
@@ -650,8 +660,9 @@ function makeTableView(
650660
);
651661
integrateGeneratedColumns?.(row, buf.view);
652662
return row;
653-
},
654-
} as UniqueIndex<any, any>;
663+
};
664+
}
665+
index = base as UniqueIndex<any, any>;
655666
} else if (serializeSinglePoint) {
656667
// numColumns == 1
657668
const rawIndex = {

crates/bindings-typescript/src/server/schema.test-d.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,4 +45,18 @@ spacetimedb.init(ctx => {
4545
const _id2 = ctx.db.person.id2;
4646

4747
ctx.db.person.id.find(2);
48+
49+
// update() is allowed on primary key indexes
50+
ctx.db.person.id.update({
51+
id: 1,
52+
name: '',
53+
name2: '',
54+
married: false,
55+
id2: '' as any,
56+
age: 0,
57+
age2: 0,
58+
});
59+
60+
// @ts-expect-error update() is NOT allowed on non-PK unique indexes
61+
const _update = ctx.db.person.name2.update;
4862
});

crates/bindings/src/table.rs

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,11 @@ pub trait Column {
274274
fn get_field(row: &<Self::Table as Table>::Row) -> &Self::ColType;
275275
}
276276

277+
/// A marker trait for columns that are the primary key of their table.
278+
///
279+
/// This is used to restrict [`UniqueColumn::update`] to only work on primary key columns.
280+
pub trait PrimaryKey {}
281+
277282
/// A handle to a unique index on a column.
278283
/// Available for `#[unique]` and `#[primary_key]` columns.
279284
///
@@ -358,13 +363,21 @@ impl<Tbl: Table, Col: Index + Column<Table = Tbl>> UniqueColumn<Tbl, Col::ColTyp
358363
/// Deletes the row where the value in the unique column matches that in the corresponding field of `new_row`, and
359364
/// then inserts the `new_row`.
360365
///
361-
/// Returns the new row as actually inserted, with computed values substituted for any auto-inc placeholders.
366+
/// Returns the new row as actually inserted, with computed values substituted for any auto-inc placeholders.
367+
///
368+
/// This method can only be called on primary key columns, not any unique column.
369+
/// This prevents confusion regarding what constitutes a row update vs. a delete+insert.
370+
/// To perform this operation for a non-primary unique column, call
371+
/// `.delete(key)` followed by `.insert(row)`.
362372
///
363373
/// # Panics
364374
/// Panics if no row was previously present with the matching value in the unique column,
365375
/// or if either the delete or the insertion would violate a constraint.
366376
#[track_caller]
367-
pub fn update(&self, new_row: Tbl::Row) -> Tbl::Row {
377+
pub fn update(&self, new_row: Tbl::Row) -> Tbl::Row
378+
where
379+
Col: PrimaryKey,
380+
{
368381
let buf = IterBuf::take();
369382
update::<Tbl>(Col::index_id(), new_row, buf)
370383
}

modules/benchmarks-cs/ia_loop.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,9 +281,9 @@ ulong current_time_ms
281281
targetable.quad = new_hash;
282282
ctx.Db.game_targetable_state.entity_id.Update(targetable);
283283

284-
if (ctx.Db.game_live_targetable_state.entity_id.Find(entity_id) is not null)
284+
if (ctx.Db.game_live_targetable_state.entity_id.Delete(entity_id))
285285
{
286-
ctx.Db.game_live_targetable_state.entity_id.Update(new(entity_id, new_hash));
286+
ctx.Db.game_live_targetable_state.Insert(new(entity_id, new_hash));
287287
}
288288

289289
GameMobileEntityState mobile_entity =

modules/benchmarks-cs/synthetic.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ public static partial class synthetic
99
[SpacetimeDB.Table(Name = "unique_0_u32_u64_str")]
1010
public partial struct unique_0_u32_u64_str_t
1111
{
12-
[Unique]
12+
[PrimaryKey]
1313
public uint id;
1414
public ulong age;
1515
public string name;
@@ -39,7 +39,7 @@ public partial struct btree_each_column_u32_u64_str_t
3939
[SpacetimeDB.Table(Name = "unique_0_u32_u64_u64")]
4040
public partial struct unique_0_u32_u64_u64_t
4141
{
42-
[Unique]
42+
[PrimaryKey]
4343
public uint id;
4444
public ulong x;
4545
public ulong y;

0 commit comments

Comments
 (0)