From 91d3aa6d6a9f5773a1646d5b0ce0dfbe2b0b84c9 Mon Sep 17 00:00:00 2001 From: Noa Date: Tue, 15 Oct 2024 16:42:24 -0500 Subject: [PATCH] Limit update() to only work on primary keys --- crates/bindings-macro/src/table.rs | 51 ++++++++++++--------------- crates/bindings/src/table.rs | 26 +++++++------- modules/benchmarks/src/ia_loop.rs | 19 +++-------- modules/benchmarks/src/synthetic.rs | 4 +-- modules/sdk-test/src/lib.rs | 53 +++++++++++++++++++---------- 5 files changed, 76 insertions(+), 77 deletions(-) diff --git a/crates/bindings-macro/src/table.rs b/crates/bindings-macro/src/table.rs index 004e512e512..79968d8e9d5 100644 --- a/crates/bindings-macro/src/table.rs +++ b/crates/bindings-macro/src/table.rs @@ -58,7 +58,7 @@ struct IndexArg { enum IndexType { BTree { columns: Vec }, - UniqueBTree { column: Ident }, + UniqueBTree { column: Ident, pk: bool }, } impl TableArgs { @@ -186,9 +186,9 @@ impl IndexArg { let cols = columns.iter().map(find_column).collect::>>()?; ValidatedIndexType::BTree { cols } } - IndexType::UniqueBTree { column } => { + IndexType::UniqueBTree { column, pk } => { let col = find_column(column)?; - ValidatedIndexType::UniqueBTree { col } + ValidatedIndexType::UniqueBTree { col, pk: *pk } } }; let index_name = match &kind { @@ -196,7 +196,7 @@ impl IndexArg { .chain(cols.iter().map(|col| col.field.name.as_deref().unwrap())) .collect::>() .join("_"), - ValidatedIndexType::UniqueBTree { col } => { + ValidatedIndexType::UniqueBTree { col, .. } => { [table_name, "btree", col.field.name.as_deref().unwrap()].join("_") } }; @@ -216,7 +216,7 @@ struct ValidatedIndex<'a> { enum ValidatedIndexType<'a> { BTree { cols: Vec<&'a Column<'a>> }, - UniqueBTree { col: &'a Column<'a> }, + UniqueBTree { col: &'a Column<'a>, pk: bool }, } impl ValidatedIndex<'_> { @@ -228,7 +228,7 @@ impl ValidatedIndex<'_> { columns: &[#(#col_ids),*] }) } - ValidatedIndexType::UniqueBTree { col } => { + ValidatedIndexType::UniqueBTree { col, .. } => { let col_id = col.index; quote!(spacetimedb::table::IndexAlgo::BTree { columns: &[#col_id] @@ -272,15 +272,18 @@ impl ValidatedIndex<'_> { } } } - ValidatedIndexType::UniqueBTree { col } => { + &ValidatedIndexType::UniqueBTree { col, pk } => { let vis = col.field.vis; let col_ty = col.field.ty; let column_ident = col.field.ident.unwrap(); - let doc = format!( + let mut doc = format!( "Gets the [`UniqueColumn`][spacetimedb::UniqueColumn] for the \ [`{column_ident}`][{row_type_ident}::{column_ident}] column." ); + if pk { + doc.push_str("\n\nThis column is the *primary key* for this table.") + } quote! { #[doc = #doc] #vis fn #column_ident(&self) -> spacetimedb::UniqueColumn { @@ -294,7 +297,7 @@ impl ValidatedIndex<'_> { fn marker_type(&self, vis: &syn::Visibility, row_type_ident: &Ident) -> TokenStream { let index_ident = self.accessor_name; let index_name = &self.index_name; - let vis = if let ValidatedIndexType::UniqueBTree { col } = self.kind { + let vis = if let ValidatedIndexType::UniqueBTree { col, .. } = self.kind { col.field.vis } else { vis @@ -311,7 +314,7 @@ impl ValidatedIndex<'_> { } } }; - if let ValidatedIndexType::UniqueBTree { col } = self.kind { + if let ValidatedIndexType::UniqueBTree { col, pk } = self.kind { let col_ty = col.ty; let col_name = col.field.name.as_deref().unwrap(); let field_ident = col.field.ident.unwrap(); @@ -325,6 +328,9 @@ impl ValidatedIndex<'_> { } } }); + if pk { + decl.extend(quote!(impl spacetimedb::table::PrimaryKey for #index_ident {})); + } } decl } @@ -479,6 +485,7 @@ pub(crate) fn table_impl(mut args: TableArgs, mut item: MutItem>>()?; - // order unique accessors before index accessors - indices.sort_by(|a, b| match (&a.kind, &b.kind) { - (ValidatedIndexType::UniqueBTree { .. }, ValidatedIndexType::UniqueBTree { .. }) => std::cmp::Ordering::Equal, - (_, ValidatedIndexType::UniqueBTree { .. }) => std::cmp::Ordering::Greater, - (ValidatedIndexType::UniqueBTree { .. }, _) => std::cmp::Ordering::Less, - _ => std::cmp::Ordering::Equal, + // put the primary key first in the docs, then unique columns, then other indices. + indices.sort_by_key(|x| match &x.kind { + ValidatedIndexType::UniqueBTree { pk: true, .. } => 0u8, + ValidatedIndexType::UniqueBTree { .. } => 1, + _ => 2, }); let index_descs = indices.iter().map(|index| index.desc()); @@ -567,7 +573,6 @@ pub(crate) fn table_impl(mut args: TableArgs, mut item: MutItem>(); let field_types = fields.iter().map(|f| f.ty).collect::>(); let tabletype_impl = quote! { @@ -602,16 +607,6 @@ pub(crate) fn table_impl(mut args: TableArgs, mut item: MutItem for #original_struct_ident { - type Field = #field_types; - fn get_field(&self) -> &Self::Field { - &self.#field_names - } - })* - }; - let row_type_to_table = quote!(<#row_type as spacetimedb::table::__MapRowTypeToTable>::Table); // Output all macro data @@ -673,8 +668,6 @@ pub(crate) fn table_impl(mut args: TableArgs, mut item: MutItem` allows -/// shared projection from `self` to its `N`th field. -#[doc(hidden)] -pub trait FieldAccess { - /// The type of the field at the `N`th position. - type Field; - - /// Project to the value of the field at position `N`. - fn get_field(&self) -> &Self::Field; -} - pub trait Column { type Row; type ColType; @@ -282,6 +269,8 @@ pub trait Column { fn get_field(row: &Self::Row) -> &Self::ColType; } +pub trait PrimaryKey {} + pub struct UniqueColumn where ColType: SpacetimeType + Serialize + DeserializeOwned, @@ -369,11 +358,20 @@ where /// /// Returns the new row as actually inserted, with any auto-inc placeholders substituted for computed values. /// + /// This method can only be called on primary keys, not any unique column. This is to reduce + /// confusion regarding what constitutes a row update vs just deleting one row and adding + /// another. To perform this same operation for a non-primary unique column `foo`, simply call + /// `.foo().delete(&row.foo)` followed by `.insert(row)`, but note that clients will not + /// consider that as an update unless the primary key of that row stays the same. + /// /// # Panics /// Panics if no row was previously present with the matching value in the unique column, /// or if either the delete or the insertion would violate a constraint. #[track_caller] - pub fn update(&self, new_row: Tbl::Row) -> Tbl::Row { + pub fn update(&self, new_row: Tbl::Row) -> Tbl::Row + where + Col: PrimaryKey, + { let (deleted, buf) = self._delete(Col::get_field(&new_row)); if !deleted { update_row_didnt_exist(Tbl::TABLE_NAME, Col::COLUMN_NAME) diff --git a/modules/benchmarks/src/ia_loop.rs b/modules/benchmarks/src/ia_loop.rs index 20869fb6165..c69a1216a51 100644 --- a/modules/benchmarks/src/ia_loop.rs +++ b/modules/benchmarks/src/ia_loop.rs @@ -322,20 +322,11 @@ fn move_agent( // If the entity is alive (which it should be), // also update the `LiveTargetableState` used by `enemy_ai_agent_loop`. - if ctx - .db - .game_live_targetable_state() - .entity_id() - .find(entity_id) - .is_some() - { - ctx.db - .game_live_targetable_state() - .entity_id() - .update(GameLiveTargetableState { - entity_id, - quad: new_hash, - }); + if ctx.db.game_live_targetable_state().entity_id().delete(entity_id) { + ctx.db.game_live_targetable_state().insert(GameLiveTargetableState { + entity_id, + quad: new_hash, + }); } let mobile_entity = ctx .db diff --git a/modules/benchmarks/src/synthetic.rs b/modules/benchmarks/src/synthetic.rs index 4d0f1ff04c3..65ccc175ca1 100644 --- a/modules/benchmarks/src/synthetic.rs +++ b/modules/benchmarks/src/synthetic.rs @@ -30,7 +30,7 @@ use std::hint::black_box; #[spacetimedb::table(name = unique_0_u32_u64_str)] pub struct unique_0_u32_u64_str_t { - #[unique] + #[primary_key] id: u32, age: u64, name: String, @@ -55,7 +55,7 @@ pub struct btree_each_column_u32_u64_str_t { #[spacetimedb::table(name = unique_0_u32_u64_u64)] pub struct unique_0_u32_u64_u64_t { - #[unique] + #[primary_key] id: u32, x: u64, y: u64, diff --git a/modules/sdk-test/src/lib.rs b/modules/sdk-test/src/lib.rs index c370b65dc3c..06d2f1ab2d9 100644 --- a/modules/sdk-test/src/lib.rs +++ b/modules/sdk-test/src/lib.rs @@ -178,16 +178,33 @@ macro_rules! define_tables { define_tables!(@impl_ops $name { $($($ops)*)? } $($field_name $ty,)*); }; + // Define a reducer for tables with a primary_key field, + // which uses `$update_method` to update by that primary_key field. + (@impl_ops $name:ident + { update_by $update:ident = $update_method:ident($pk_field:ident) + $(, $($ops:tt)* )? } + $($field_name:ident $ty:ty),* $(,)*) => { + paste::paste! { + #[spacetimedb::reducer] + pub fn $update (ctx: &ReducerContext, $($field_name : $ty,)*) { + ctx.db.[<$name:snake>]().$pk_field().update($name { $($field_name,)* }); + } + } + + define_tables!(@impl_ops $name { $($($ops)*)? } $($field_name $ty,)*); + }; + // Define a reducer for tables with a unique field, // which uses `$update_method` to update by that unique field. (@impl_ops $name:ident - { update_by $update:ident = $update_method:ident($unique_field:ident) + { update_non_pk_by $update:ident = $update_method:ident($unique_field:ident) $(, $($ops:tt)* )? } $($field_name:ident $ty:ty),* $(,)*) => { paste::paste! { #[spacetimedb::reducer] pub fn $update (ctx: &ReducerContext, $($field_name : $ty,)*) { - ctx.db.[<$name:snake>]().$unique_field().update($name { $($field_name,)* }); + assert!(ctx.db.[<$name:snake>]().$unique_field().delete(&$unique_field)); + ctx.db.[<$name:snake>]().insert($name { $($field_name,)* }); } } @@ -315,100 +332,100 @@ define_tables! { define_tables! { UniqueU8 { insert_or_panic insert_unique_u8, - update_by update_unique_u8 = update_by_n(n), + update_non_pk_by update_unique_u8 = update_by_n(n), delete_by delete_unique_u8 = delete_by_n(n: u8), } #[unique] n u8, data i32; UniqueU16 { insert_or_panic insert_unique_u16, - update_by update_unique_u16 = update_by_n(n), + update_non_pk_by update_unique_u16 = update_by_n(n), delete_by delete_unique_u16 = delete_by_n(n: u16), } #[unique] n u16, data i32; UniqueU32 { insert_or_panic insert_unique_u32, - update_by update_unique_u32 = update_by_n(n), + update_non_pk_by update_unique_u32 = update_by_n(n), delete_by delete_unique_u32 = delete_by_n(n: u32), } #[unique] n u32, data i32; UniqueU64 { insert_or_panic insert_unique_u64, - update_by update_unique_u64 = update_by_n(n), + update_non_pk_by update_unique_u64 = update_by_n(n), delete_by delete_unique_u64 = delete_by_n(n: u64), } #[unique] n u64, data i32; UniqueU128 { insert_or_panic insert_unique_u128, - update_by update_unique_u128 = update_by_n(n), + update_non_pk_by update_unique_u128 = update_by_n(n), delete_by delete_unique_u128 = delete_by_n(n: u128), } #[unique] n u128, data i32; UniqueU256 { insert_or_panic insert_unique_u256, - update_by update_unique_u256 = update_by_n(n), + update_non_pk_by update_unique_u256 = update_by_n(n), delete_by delete_unique_u256 = delete_by_n(n: u256), } #[unique] n u256, data i32; UniqueI8 { insert_or_panic insert_unique_i8, - update_by update_unique_i8 = update_by_n(n), + update_non_pk_by update_unique_i8 = update_by_n(n), delete_by delete_unique_i8 = delete_by_n(n: i8), } #[unique] n i8, data i32; UniqueI16 { insert_or_panic insert_unique_i16, - update_by update_unique_i16 = update_by_n(n), + update_non_pk_by update_unique_i16 = update_by_n(n), delete_by delete_unique_i16 = delete_by_n(n: i16), } #[unique] n i16, data i32; UniqueI32 { insert_or_panic insert_unique_i32, - update_by update_unique_i32 = update_by_n(n), + update_non_pk_by update_unique_i32 = update_by_n(n), delete_by delete_unique_i32 = delete_by_n(n: i32), } #[unique] n i32, data i32; UniqueI64 { insert_or_panic insert_unique_i64, - update_by update_unique_i64 = update_by_n(n), + update_non_pk_by update_unique_i64 = update_by_n(n), delete_by delete_unique_i64 = delete_by_n(n: i64), } #[unique] n i64, data i32; UniqueI128 { insert_or_panic insert_unique_i128, - update_by update_unique_i128 = update_by_n(n), + update_non_pk_by update_unique_i128 = update_by_n(n), delete_by delete_unique_i128 = delete_by_n(n: i128), } #[unique] n i128, data i32; UniqueI256 { insert_or_panic insert_unique_i256, - update_by update_unique_i256 = update_by_n(n), + update_non_pk_by update_unique_i256 = update_by_n(n), delete_by delete_unique_i256 = delete_by_n(n: i256), } #[unique] n i256, data i32; UniqueBool { insert_or_panic insert_unique_bool, - update_by update_unique_bool = update_by_b(b), + update_non_pk_by update_unique_bool = update_by_b(b), delete_by delete_unique_bool = delete_by_b(b: bool), } #[unique] b bool, data i32; UniqueString { insert_or_panic insert_unique_string, - update_by update_unique_string = update_by_s(s), + update_non_pk_by update_unique_string = update_by_s(s), delete_by delete_unique_string = delete_by_s(s: String), } #[unique] s String, data i32; UniqueIdentity { insert_or_panic insert_unique_identity, - update_by update_unique_identity = update_by_i(i), + update_non_pk_by update_unique_identity = update_by_i(i), delete_by delete_unique_identity = delete_by_i(i: Identity), } #[unique] i Identity, data i32; UniqueAddress { insert_or_panic insert_unique_address, - update_by update_unique_address = update_by_a(a), + update_non_pk_by update_unique_address = update_by_a(a), delete_by delete_unique_address = delete_by_a(a: Address), } #[unique] a Address, data i32; }