Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Limit update() to only work on primary keys #1862

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Limit update() to only work on primary keys #1862

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 22 additions & 29 deletions crates/bindings-macro/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ struct IndexArg {

enum IndexType {
BTree { columns: Vec<Ident> },
UniqueBTree { column: Ident },
UniqueBTree { column: Ident, pk: bool },
}

impl TableArgs {
Expand Down Expand Up @@ -586,17 +586,17 @@ impl IndexArg {
let cols = columns.iter().map(find_column).collect::<syn::Result<Vec<_>>>()?;
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 {
ValidatedIndexType::BTree { cols } => ([table_name, "btree"].into_iter())
.chain(cols.iter().map(|col| col.field.name.as_deref().unwrap()))
.collect::<Vec<_>>()
.join("_"),
ValidatedIndexType::UniqueBTree { col } => {
ValidatedIndexType::UniqueBTree { col, .. } => {
[table_name, "btree", col.field.name.as_deref().unwrap()].join("_")
}
};
Expand All @@ -616,7 +616,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<'_> {
Expand All @@ -628,7 +628,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]
Expand Down Expand Up @@ -672,15 +672,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<Self, #col_ty, __indices::#index_ident> {
Expand All @@ -694,7 +697,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
Expand All @@ -711,7 +714,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();
Expand All @@ -725,6 +728,9 @@ impl ValidatedIndex<'_> {
}
}
});
if pk {
decl.extend(quote!(impl spacetimedb::table::PrimaryKey for #index_ident {}));
}
}
decl
}
Expand Down Expand Up @@ -966,6 +972,7 @@ fn table_impl(mut args: TableArgs, mut item: MutItem<syn::DeriveInput>) -> syn::
name: field_ident.clone(),
kind: IndexType::UniqueBTree {
column: field_ident.clone(),
pk: primary_key.is_some(),
},
});
}
Expand All @@ -988,12 +995,11 @@ fn table_impl(mut args: TableArgs, mut item: MutItem<syn::DeriveInput>) -> syn::
.map(|index| index.validate(&table_name, &columns))
.collect::<syn::Result<Vec<_>>>()?;

// 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());
Expand Down Expand Up @@ -1042,7 +1048,6 @@ fn table_impl(mut args: TableArgs, mut item: MutItem<syn::DeriveInput>) -> syn::
quote!(::core::convert::Infallible)
};

let field_names = fields.iter().map(|f| f.ident.unwrap()).collect::<Vec<_>>();
let field_types = fields.iter().map(|f| f.ty).collect::<Vec<_>>();

let tabletype_impl = quote! {
Expand Down Expand Up @@ -1077,16 +1082,6 @@ fn table_impl(mut args: TableArgs, mut item: MutItem<syn::DeriveInput>) -> syn::
}
};

let col_num = 0u16..;
let field_access_impls = quote! {
#(impl spacetimedb::table::FieldAccess<#col_num> 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
Expand Down Expand Up @@ -1148,8 +1143,6 @@ fn table_impl(mut args: TableArgs, mut item: MutItem<syn::DeriveInput>) -> syn::
#deserialize_impl
#serialize_impl

#field_access_impls

#scheduled_reducer_type_check
};

Expand Down
26 changes: 12 additions & 14 deletions crates/bindings/src/table.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,26 +255,15 @@ impl MaybeError for AutoIncOverflow {
}
}

/// A trait for types exposing an operation to access their `N`th field.
///
/// In other words, a type implementing `FieldAccess<N>` allows
/// shared projection from `self` to its `N`th field.
#[doc(hidden)]
pub trait FieldAccess<const N: u16> {
/// 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;
const COLUMN_NAME: &'static str;
fn get_field(row: &Self::Row) -> &Self::ColType;
}

pub trait PrimaryKey {}

pub struct UniqueColumn<Tbl: Table, ColType, Col>
where
ColType: SpacetimeType + Serialize + DeserializeOwned,
Expand Down Expand Up @@ -362,11 +351,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)
Expand Down
19 changes: 5 additions & 14 deletions modules/benchmarks/src/ia_loop.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions modules/benchmarks/src/synthetic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
Loading
Loading