From 254786b3851f2ca844dbad290b32f83060ed915e Mon Sep 17 00:00:00 2001 From: Rudy Nurhadi Date: Sun, 29 Mar 2026 13:19:02 +0700 Subject: [PATCH] bindings-macro: add custom index name to prevent index name collisions --- crates/bindings-macro/src/table.rs | 76 ++++++++++++++---------------- 1 file changed, 35 insertions(+), 41 deletions(-) diff --git a/crates/bindings-macro/src/table.rs b/crates/bindings-macro/src/table.rs index 1cbba4c5ca1..44296feae59 100644 --- a/crates/bindings-macro/src/table.rs +++ b/crates/bindings-macro/src/table.rs @@ -245,36 +245,10 @@ impl IndexArg { } sym::name => { check_duplicate(&canonical_name, &meta)?; - canonical_name = Some(meta.value()?.parse()?); - } - - sym::btree => { - check_duplicate_msg(&algo, &meta, "index algorithm specified twice")?; - algo = Some(Self::parse_btree(meta)?); - } - sym::hash => { - check_duplicate_msg(&algo, &meta, "index algorithm specified twice")?; - algo = Some(Self::parse_hash(meta)?); - } - sym::direct => { - check_duplicate_msg(&algo, &meta, "index algorithm specified twice")?; - algo = Some(Self::parse_direct(meta)?); - } - sym::name => { // If the user is trying to specify a `name`, do a bit of guessing at what their goal is. // This is going to be best-effort, and we're not going to try to do lookahead or anything. - return Err(if accessor.is_some() { - // If the user's already specified an `accessor`, - // then probably they're trying to specify the canonical name, - // like you can for tables. - // Print an error that says this is unsupported. - meta.error( - "Unexpected argument `name` in index definition. - -Overwriting the `name` of an index is currently unsupported.", - ) - } else if let Ok(sym) = meta.value().and_then(|val| val.parse::()) { + if let Ok(ident) = meta.value().and_then(|val| val.parse::()) { // If we haven't seen an `accessor` yet, and the value is an ident, // then probably this is 1.* syntax that needs a migration. // Note that, if the user specifies `name = {ident}` followed by `accessor = {ident}`, @@ -282,21 +256,33 @@ Overwriting the `name` of an index is currently unsupported.", // I (pgoldman 2026-02-18) don't see a good way to distinguish this case // without making our parsing dramatically more complicated, // and it seems unlikely to occur. - meta.error(format_args!( + return Err(meta.error(format_args!( "Expected an `accessor` in index definition, but got a `name` instead. -If you're migrating from SpacetimeDB 1.*, replace `name = {sym}` with `accessor = {sym}`." - )) +If you're migrating from SpacetimeDB 1.*, replace `name = {ident}` with `accessor = {ident}`." + ))); + } else if let Ok(lit_str) = meta.value()?.parse::() { + // User provided a string literal - use it as custom index name + canonical_name = Some(lit_str); } else { - // If we haven't seen an `accessor` yet, but the value is not an ident, - // then we're not really sure what's going wrong, so print a more generic error message. - meta.error(format_args!( - "Unexpected argument `name` in index definition. - -Overwriting the `name` of an index is currently unsupported. -Did you mean to specify an `accessor` instead? Do so with `accessor = my_index`, where `my_index` is an unquoted identifier." - )) - }); + return Err(meta.error( + "Unexpected argument `name` in index definition.\n\n\ +Use a string literal for a custom index name: `name = \"my_index_name\"`.", + )); + } + } + + sym::btree => { + check_duplicate_msg(&algo, &meta, "index algorithm specified twice")?; + algo = Some(Self::parse_btree(meta)?); + } + sym::hash => { + check_duplicate_msg(&algo, &meta, "index algorithm specified twice")?; + algo = Some(Self::parse_hash(meta)?); + } + sym::direct => { + check_duplicate_msg(&algo, &meta, "index algorithm specified twice")?; + algo = Some(Self::parse_direct(meta)?); } }); Ok(()) @@ -369,6 +355,7 @@ Did you mean to specify an `accessor` instead? Do so with `accessor = my_index`, /// Parses an inline `#[index(btree)]`, `#[index(hash)]`, or `#[index(direct)]` attribute on a field. fn parse_index_attr(field: &Ident, attr: &syn::Attribute) -> syn::Result { let mut kind = None; + let mut name = None; attr.parse_nested_meta(|meta| { match_meta!(match meta { sym::btree => { @@ -387,6 +374,10 @@ Did you mean to specify an `accessor` instead? Do so with `accessor = my_index`, check_duplicate_msg(&kind, &meta, "index type specified twice")?; kind = Some(IndexType::Direct { column: field.clone() }) } + sym::name => { + check_duplicate(&name, &meta)?; + name = Some(meta.value()?.parse()?); + } }); Ok(()) })?; @@ -395,7 +386,7 @@ Did you mean to specify an `accessor` instead? Do so with `accessor = my_index`, // Default accessor = field name if not provided let accessor = field.clone(); - Ok(IndexArg::new(accessor, kind, None)) + Ok(IndexArg::new(accessor, kind, name)) } fn validate<'a>(&'a self, table_name: &str, cols: &'a [Column<'a>]) -> syn::Result> { @@ -436,7 +427,10 @@ Did you mean to specify an `accessor` instead? Do so with `accessor = my_index`, is_unique: self.is_unique, // This must be the canonical name (name used internally in database), // as it is used in `index_id_from_name` abi. - index_name: gen_index_name(), + index_name: match self.canonical_name.as_ref() { + Some(s) => s.value(), + None => gen_index_name(), + }, accessor_name: &self.accessor, kind, canonical_name: self.canonical_name.as_ref().map(|s| s.value()),