-
Notifications
You must be signed in to change notification settings - Fork 1k
Fix index schema from st tables. #5145
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,4 +38,5 @@ mod sql; | |
| mod sql_connect_hook; | ||
| mod templates; | ||
| mod timestamp_route; | ||
| mod typescript_index_source_name; | ||
| mod views; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| use spacetimedb_smoketests::{random_string, require_local_server, require_pnpm, Smoketest}; | ||
|
|
||
| const TYPESCRIPT_MODULE_WITHOUT_NEW_COLUMNS: &str = r#"import { schema, table, t } from "spacetimedb/server"; | ||
|
|
||
| const users = table( | ||
| { name: "users", public: false }, | ||
| { | ||
| id: t.u64().primaryKey().autoInc(), | ||
| name: t.string(), | ||
| emailAddress: t.string().index("btree"), | ||
| }, | ||
| ); | ||
|
|
||
| const spacetimedb = schema({ | ||
| users, | ||
| }); | ||
| export default spacetimedb; | ||
|
|
||
| export const insert_user = spacetimedb.reducer( | ||
| { | ||
| name: t.string(), | ||
| emailAddress: t.string(), | ||
| }, | ||
| (ctx, { name, emailAddress }) => { | ||
| ctx.db.users.insert({ | ||
| id: 0n, | ||
| name, | ||
| emailAddress, | ||
| }); | ||
| }, | ||
| ); | ||
| "#; | ||
|
|
||
| const TYPESCRIPT_MODULE_WITH_NEW_COLUMNS: &str = r#"import { schema, table, t } from "spacetimedb/server"; | ||
|
|
||
| const users = table( | ||
| { name: "users", public: false }, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we make the table accessor differ from the canonical table name? The reported issue involved runtime/source index names being built from both table and column accessor names while the stored index used canonical snake_case names. This test covers column accessor divergence but not table accessor divergence because both names are currently |
||
| { | ||
| id: t.u64().primaryKey().autoInc(), | ||
| name: t.string(), | ||
| emailAddress: t.string().index("btree"), | ||
| age: t.number().optional().default(undefined), | ||
| isActive: t.bool().default(false).index(), | ||
| }, | ||
| ); | ||
|
|
||
| const spacetimedb = schema({ | ||
| users, | ||
| }); | ||
| export default spacetimedb; | ||
|
|
||
| export const find_user_by_email = spacetimedb.reducer( | ||
| { emailAddress: t.string() }, | ||
| (ctx, { emailAddress }) => { | ||
| let count = 0; | ||
| for (const _row of ctx.db.users.emailAddress.filter(emailAddress)) { | ||
| count += 1; | ||
| } | ||
| console.info(`matched ${count}`); | ||
| }, | ||
| ); | ||
|
|
||
| export const find_users_by_active_status = spacetimedb.reducer( | ||
| { isActive: t.bool() }, | ||
| (ctx, { isActive }) => { | ||
| let count = 0; | ||
| for (const _row of ctx.db.users.isActive.filter(isActive)) { | ||
| count += 1; | ||
| } | ||
| console.info(`matched active users ${count}`); | ||
| }, | ||
| ); | ||
| "#; | ||
|
|
||
| #[test] | ||
| fn test_typescript_add_optional_columns() { | ||
| require_pnpm!(); | ||
| require_local_server!(); | ||
|
|
||
| let mut test = Smoketest::builder().autopublish(false).build(); | ||
| let module_name = format!("typescript-add-optional-columns-{}", random_string()); | ||
|
|
||
| let database_identity = test | ||
| .publish_typescript_module_source( | ||
| "typescript-add-optional-columns-v1", | ||
| &module_name, | ||
| TYPESCRIPT_MODULE_WITHOUT_NEW_COLUMNS, | ||
| ) | ||
| .unwrap(); | ||
|
|
||
| test.call("insert_user", &["Alice", "alice@example.com"]).unwrap(); | ||
|
|
||
| test.restart_server(); | ||
|
|
||
| test.publish_typescript_module_source_clear( | ||
| "typescript-add-optional-columns-v2", | ||
| &database_identity, | ||
| TYPESCRIPT_MODULE_WITH_NEW_COLUMNS, | ||
| false, | ||
| ) | ||
| .unwrap(); | ||
|
|
||
| test.call("find_user_by_email", &["alice@example.com"]).unwrap(); | ||
| test.call("find_users_by_active_status", &["false"]).unwrap(); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This restores the persisted
source_namealias when rebuildingIndexSchema, which fixes the restart/recreate path, but I think auto-migration still has the same issue.auto_migrate_indexesonly comparesaccessor_nameandalgorithm, notIndexDef.source_name. If a TS module changes the generated source name while keeping the same canonical index name/accessor/algorithm, the migration can succeed without updatingst_index_accessor, and the runtime will still callindex_id_from_namewith the new source name.Should we reject
source_namechanges as requiring manual migration, or add a step to updatest_index_accessor?