From 48d69d43890301a8c597c83244727d5868e4db27 Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Wed, 5 Nov 2025 16:47:44 +0100 Subject: [PATCH 1/3] Refactor table admin view --- crates/core/src/fix_data.rs | 10 +- crates/core/src/schema/inspection.rs | 50 ++++++++++ crates/core/src/schema/management.rs | 144 +++++++++------------------ crates/core/src/schema/table_info.rs | 6 +- crates/core/src/view_admin.rs | 103 ------------------- 5 files changed, 109 insertions(+), 204 deletions(-) diff --git a/crates/core/src/fix_data.rs b/crates/core/src/fix_data.rs index 08c5cadd..f0026b40 100644 --- a/crates/core/src/fix_data.rs +++ b/crates/core/src/fix_data.rs @@ -5,6 +5,7 @@ use alloc::string::String; use crate::create_sqlite_optional_text_fn; use crate::error::{PSResult, PowerSyncError}; +use crate::schema::inspection::ExistingTable; use powersync_sqlite_nostd::{self as sqlite, ColumnType, Value}; use powersync_sqlite_nostd::{Connection, Context, ResultCode}; @@ -23,12 +24,15 @@ use crate::util::quote_identifier; pub fn apply_v035_fix(db: *mut sqlite::sqlite3) -> Result { // language=SQLite let statement = db - .prepare_v2("SELECT name, powersync_external_table_name(name) FROM sqlite_master WHERE type='table' AND name GLOB 'ps_data__*'") - .into_db_result(db)?; + .prepare_v2("SELECT name FROM sqlite_master WHERE type='table' AND name GLOB 'ps_data__*'") + .into_db_result(db)?; while statement.step()? == ResultCode::ROW { let full_name = statement.column_text(0)?; - let short_name = statement.column_text(1)?; + let Some((short_name, _)) = ExistingTable::external_name(full_name) else { + continue; + }; + let quoted = quote_identifier(full_name); // language=SQLite diff --git a/crates/core/src/schema/inspection.rs b/crates/core/src/schema/inspection.rs index 3bd4c767..663602e8 100644 --- a/crates/core/src/schema/inspection.rs +++ b/crates/core/src/schema/inspection.rs @@ -79,3 +79,53 @@ SELECT Ok(()) } } + +pub struct ExistingTable { + pub name: String, + pub internal_name: String, + pub local_only: bool, +} + +impl ExistingTable { + pub fn list(db: *mut sqlite::sqlite3) -> Result, PowerSyncError> { + let mut results = vec![]; + let stmt = db + .prepare_v2( + " +SELECT name FROM sqlite_master WHERE type = 'table' AND name GLOB 'ps_data_*'; + ", + ) + .into_db_result(db)?; + + while stmt.step()? == ResultCode::ROW { + let internal_name = stmt.column_text(0)?; + let Some((name, local_only)) = Self::external_name(internal_name) else { + continue; + }; + + results.push(ExistingTable { + internal_name: internal_name.to_owned(), + name: name.to_owned(), + local_only: local_only, + }); + } + + Ok(results) + } + + /// Extracts the public name from a `ps_data__` or a `ps_data_local__` table. + /// + /// Also returns whether the name is from a local table. + pub fn external_name(name: &str) -> Option<(&str, bool)> { + const LOCAL_PREFIX: &str = "ps_data_local__"; + const NORMAL_PREFIX: &str = "ps_data__"; + + if name.starts_with(LOCAL_PREFIX) { + Some((&name[LOCAL_PREFIX.len()..], true)) + } else if name.starts_with(NORMAL_PREFIX) { + Some((&name[NORMAL_PREFIX.len()..], false)) + } else { + None + } + } +} diff --git a/crates/core/src/schema/management.rs b/crates/core/src/schema/management.rs index 6ba25f14..feac68d0 100644 --- a/crates/core/src/schema/management.rs +++ b/crates/core/src/schema/management.rs @@ -14,7 +14,7 @@ use sqlite::{Connection, ResultCode, Value}; use crate::error::{PSResult, PowerSyncError}; use crate::ext::ExtendedDatabase; -use crate::schema::inspection::ExistingView; +use crate::schema::inspection::{ExistingTable, ExistingView}; use crate::state::DatabaseState; use crate::util::{quote_identifier, quote_json_path}; use crate::views::{ @@ -25,112 +25,62 @@ use crate::{create_auto_tx_function, create_sqlite_text_fn}; use super::Schema; -fn update_tables(db: *mut sqlite::sqlite3, schema: &str) -> Result<(), PowerSyncError> { - { - // In a block so that the statement is finalized before dropping tables - // language=SQLite - let statement = db - .prepare_v2( - "\ -SELECT - json_extract(json_each.value, '$.name') as name, - powersync_internal_table_name(json_each.value) as internal_name, - ifnull(json_extract(json_each.value, '$.local_only'), 0) as local_only - FROM json_each(json_extract(?, '$.tables')) - WHERE name NOT IN (SELECT name FROM powersync_tables)", - ) - .into_db_result(db)?; - statement.bind_text(1, schema, sqlite::Destructor::STATIC)?; - - while statement.step().into_db_result(db)? == ResultCode::ROW { - let name = statement.column_text(0)?; - let internal_name = statement.column_text(1)?; - let local_only = statement.column_int(2) != 0; - - db.exec_safe(&format!( - "CREATE TABLE {:}(id TEXT PRIMARY KEY NOT NULL, data TEXT)", - quote_identifier(internal_name) - )) - .into_db_result(db)?; +fn update_tables(db: *mut sqlite::sqlite3, schema: &Schema) -> Result<(), PowerSyncError> { + let existing_tables = ExistingTable::list(db)?; + let mut existing_tables = { + let mut map = BTreeMap::new(); + for table in &existing_tables { + map.insert(&*table.name, table); + } + map + }; - if !local_only { - // MOVE data if any - db.exec_text( - &format!( - "INSERT INTO {:}(id, data) - SELECT id, data - FROM ps_untyped - WHERE type = ?", - quote_identifier(internal_name) - ), - name, - ) + { + // In a block so that all statements are finalized before dropping tables. + for table in &schema.tables { + if let Some(_) = existing_tables.remove(&*table.name) { + // This table exists already, nothing to do. + // TODO: Handle switch between local only <-> regular tables? + } else { + // New table. + let quoted_internal_name = quote_identifier(&table.internal_name()); + + db.exec_safe(&format!( + "CREATE TABLE {:}(id TEXT PRIMARY KEY NOT NULL, data TEXT)", + quoted_internal_name + )) .into_db_result(db)?; - // language=SQLite - db.exec_text( - "DELETE - FROM ps_untyped - WHERE type = ?", - name, - )?; - } - - if !local_only { - // MOVE data if any - db.exec_text( - &format!( - "INSERT INTO {:}(id, data) + if !table.local_only() { + // MOVE data if any + db.exec_text( + &format!( + "INSERT INTO {:}(id, data) SELECT id, data FROM ps_untyped WHERE type = ?", - quote_identifier(internal_name) - ), - name, - ) - .into_db_result(db)?; - - // language=SQLite - db.exec_text( - "DELETE - FROM ps_untyped - WHERE type = ?", - name, - )?; + quoted_internal_name + ), + &table.name, + ) + .into_db_result(db)?; + + // language=SQLite + db.exec_text("DELETE FROM ps_untyped WHERE type = ?", &table.name)?; + } } } - } - - let mut tables_to_drop: Vec = alloc::vec![]; - - { - // In a block so that the statement is finalized before dropping tables - // language=SQLite - let statement = db - .prepare_v2( - "\ -SELECT name, internal_name, local_only FROM powersync_tables WHERE name NOT IN ( - SELECT json_extract(json_each.value, '$.name') - FROM json_each(json_extract(?, '$.tables')) - )", - ) - .into_db_result(db)?; - statement.bind_text(1, schema, sqlite::Destructor::STATIC)?; - - while statement.step()? == ResultCode::ROW { - let name = statement.column_text(0)?; - let internal_name = statement.column_text(1)?; - let local_only = statement.column_int(2) != 0; - - tables_to_drop.push(String::from(internal_name)); - if !local_only { + // Remaining tables need to be dropped. But first, we want to move their contents to + // ps_untyped. + for remaining in existing_tables.values() { + if !remaining.local_only { db.exec_text( &format!( "INSERT INTO ps_untyped(type, id, data) SELECT ?, id, data FROM {:}", - quote_identifier(internal_name) + quote_identifier(&remaining.internal_name) ), - name, + &remaining.name, ) .into_db_result(db)?; } @@ -139,8 +89,8 @@ SELECT name, internal_name, local_only FROM powersync_tables WHERE name NOT IN ( // We cannot have any open queries on sqlite_master at the point that we drop tables, otherwise // we get "table is locked" errors. - for internal_name in tables_to_drop { - let q = format!("DROP TABLE {:}", quote_identifier(&internal_name)); + for remaining in existing_tables.values() { + let q = format!("DROP TABLE {:}", quote_identifier(&remaining.internal_name)); db.exec_safe(&q).into_db_result(db)?; } @@ -305,7 +255,7 @@ fn powersync_replace_schema_impl( // language=SQLite db.exec_safe("SELECT powersync_init()").into_db_result(db)?; - update_tables(db, schema)?; + update_tables(db, &parsed_schema)?; update_indexes(db, &parsed_schema)?; update_views(db, &parsed_schema)?; diff --git a/crates/core/src/schema/table_info.rs b/crates/core/src/schema/table_info.rs index d8a6f218..5a6a77b3 100644 --- a/crates/core/src/schema/table_info.rs +++ b/crates/core/src/schema/table_info.rs @@ -35,8 +35,12 @@ impl Table { .unwrap_or(self.name.as_str()) } + pub fn local_only(&self) -> bool { + self.flags.local_only() + } + pub fn internal_name(&self) -> String { - if self.flags.local_only() { + if self.local_only() { format!("ps_data_local__{:}", self.name) } else { format!("ps_data__{:}", self.name) diff --git a/crates/core/src/view_admin.rs b/crates/core/src/view_admin.rs index 4ee57509..69295df3 100644 --- a/crates/core/src/view_admin.rs +++ b/crates/core/src/view_admin.rs @@ -31,72 +31,10 @@ extern "C" fn powersync_drop_view( } } -fn powersync_internal_table_name_impl( - ctx: *mut sqlite::context, - args: &[*mut sqlite::value], -) -> Result { - // schema: JSON - let schema = args[0].text(); - - let local_db = ctx.db_handle(); - - // language=SQLite - let stmt1 = local_db.prepare_v2( - "SELECT json_extract(?1, '$.name') as name, ifnull(json_extract(?1, '$.local_only'), 0)", - )?; - stmt1.bind_text(1, schema, sqlite::Destructor::STATIC)?; - - let step_result = stmt1.step()?; - if step_result != ResultCode::ROW { - return Err(ResultCode::SCHEMA); - } - - let name = stmt1.column_text(0)?; - let local_only = stmt1.column_int(1) != 0; - - if local_only { - Ok(format!("ps_data_local__{:}", name)) - } else { - Ok(format!("ps_data__{:}", name)) - } -} - -create_sqlite_text_fn!( - powersync_internal_table_name, - powersync_internal_table_name_impl, - "powersync_internal_table_name" -); - -fn powersync_external_table_name_impl( - _ctx: *mut sqlite::context, - args: &[*mut sqlite::value], -) -> Result { - // name: full table name - let name = args[0].text(); - - if name.starts_with("ps_data_local__") { - Ok(String::from(&name[15..])) - } else if name.starts_with("ps_data__") { - Ok(String::from(&name[9..])) - } else { - Err(PowerSyncError::argument_error("not a powersync table")) - } -} - -create_sqlite_text_fn!( - powersync_external_table_name, - powersync_external_table_name_impl, - "powersync_external_table_name" -); - fn powersync_init_impl( ctx: *mut sqlite::context, _args: &[*mut sqlite::value], ) -> Result { - let local_db = ctx.db_handle(); - - setup_internal_views(local_db)?; - powersync_migrate(ctx, LATEST_VERSION)?; Ok(String::from("")) @@ -219,25 +157,6 @@ impl PowerSyncClearFlags { create_auto_tx_function!(powersync_clear_tx, powersync_clear_impl); create_sqlite_text_fn!(powersync_clear, powersync_clear_tx, "powersync_clear"); -fn setup_internal_views(db: *mut sqlite::sqlite3) -> Result<(), ResultCode> { - // TODO: This should not be a public view - implement internally instead - // language=SQLite - - // language=SQLite - db.exec_safe( - "\ - CREATE TEMP VIEW IF NOT EXISTS powersync_tables(name, internal_name, local_only) - AS SELECT - powersync_external_table_name(name) as name, - name as internal_name, - name GLOB 'ps_data_local__*' as local_only - FROM sqlite_master - WHERE type = 'table' AND name GLOB 'ps_data_*';", - )?; - - Ok(()) -} - pub fn register(db: *mut sqlite::sqlite3, state: Rc) -> Result<(), ResultCode> { // This entire module is just making it easier to edit sqlite_master using queries. @@ -287,27 +206,5 @@ pub fn register(db: *mut sqlite::sqlite3, state: Rc) -> Result<() Some(DatabaseState::destroy_rc), )?; - db.create_function_v2( - "powersync_external_table_name", - 1, - sqlite::UTF8 | sqlite::DETERMINISTIC, - None, - Some(powersync_external_table_name), - None, - None, - None, - )?; - - db.create_function_v2( - "powersync_internal_table_name", - 1, - sqlite::UTF8 | sqlite::DETERMINISTIC, - None, - Some(powersync_internal_table_name), - None, - None, - None, - )?; - Ok(()) } From 4747b8b4e7846e1332a59084b3100ed7054946d4 Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Thu, 6 Nov 2025 09:35:53 +0100 Subject: [PATCH 2/3] Add test migrating between local and synced --- crates/core/src/schema/management.rs | 69 ++++++++++++++++---------- dart/test/schema_test.dart | 73 ++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 25 deletions(-) diff --git a/crates/core/src/schema/management.rs b/crates/core/src/schema/management.rs index feac68d0..31497c63 100644 --- a/crates/core/src/schema/management.rs +++ b/crates/core/src/schema/management.rs @@ -38,36 +38,55 @@ fn update_tables(db: *mut sqlite::sqlite3, schema: &Schema) -> Result<(), PowerS { // In a block so that all statements are finalized before dropping tables. for table in &schema.tables { - if let Some(_) = existing_tables.remove(&*table.name) { - // This table exists already, nothing to do. - // TODO: Handle switch between local only <-> regular tables? - } else { - // New table. - let quoted_internal_name = quote_identifier(&table.internal_name()); - - db.exec_safe(&format!( - "CREATE TABLE {:}(id TEXT PRIMARY KEY NOT NULL, data TEXT)", - quoted_internal_name - )) - .into_db_result(db)?; + if let Some(existing) = existing_tables.remove(&*table.name) { + if existing.local_only && !table.local_only() { + // Migrate from a local-only to a synced table. Because none of the local writes + // would have created CRUD entries, we'll have to re-create the table from + // scratch. + + // To delete the old existing table in the end. + existing_tables.insert(&existing.name, existing); + } else if !existing.local_only && table.local_only() { + // Migrate from a synced table to a local-only table. We can keep existing rows + // and will also keep existing CRUD data to be uploaded before the switch. + db.exec_safe(&format!( + "ALTER TABLE {} RENAME TO {}", + quote_identifier(&existing.internal_name), + quote_identifier(&table.internal_name()), + )) + .into_db_result(db)?; + continue; + } else { + // Identical table exists already, nothing to do. + continue; + } + } + + // New table. + let quoted_internal_name = quote_identifier(&table.internal_name()); + + db.exec_safe(&format!( + "CREATE TABLE {:}(id TEXT PRIMARY KEY NOT NULL, data TEXT)", + quoted_internal_name + )) + .into_db_result(db)?; - if !table.local_only() { - // MOVE data if any - db.exec_text( - &format!( - "INSERT INTO {:}(id, data) + if !table.local_only() { + // MOVE data if any + db.exec_text( + &format!( + "INSERT INTO {:}(id, data) SELECT id, data FROM ps_untyped WHERE type = ?", - quoted_internal_name - ), - &table.name, - ) - .into_db_result(db)?; + quoted_internal_name + ), + &table.name, + ) + .into_db_result(db)?; - // language=SQLite - db.exec_text("DELETE FROM ps_untyped WHERE type = ?", &table.name)?; - } + // language=SQLite + db.exec_text("DELETE FROM ps_untyped WHERE type = ?", &table.name)?; } } diff --git a/dart/test/schema_test.dart b/dart/test/schema_test.dart index 8d1646b3..f0f92034 100644 --- a/dart/test/schema_test.dart +++ b/dart/test/schema_test.dart @@ -45,6 +45,79 @@ void main() { greaterThan(versionAfter2['schema_version'] as int)); }); + group('migrate tables', () { + final local = { + "tables": [ + { + "name": "users", + "local_only": true, + "insert_only": false, + "columns": [ + {"name": "name", "type": "TEXT"}, + ], + }, + ] + }; + + final synced = { + "tables": [ + { + "name": "users", + "local_only": false, + "insert_only": false, + "columns": [ + {"name": "name", "type": "TEXT"}, + ], + }, + ] + }; + + test('from synced to local', () { + // Start with synced table, and sync row + db.execute('SELECT powersync_replace_schema(?)', [json.encode(synced)]); + db.execute( + 'INSERT INTO ps_data__users (id, data) VALUES (?, ?)', + [ + 'synced-id', + json.encode({'name': 'name'}) + ], + ); + + // Migrate to local table. + db.execute('SELECT powersync_replace_schema(?)', [json.encode(local)]); + + // The synced table should not exist anymore. + expect(() => db.select('SELECT * FROM ps_data__users'), + throwsA(isA())); + + // Data should still be there. + expect(db.select('SELECT * FROM users'), [ + {'id': 'synced-id', 'name': 'name'} + ]); + + // Inserting into local-only table should not record CRUD item. + db.execute( + 'INSERT INTO users (id, name) VALUES (uuid(), ?)', ['local']); + expect(db.select('SELECT * FROM ps_crud'), isEmpty); + }); + + test('from local to synced', () { + // Start with local table, and local row + db.execute('SELECT powersync_replace_schema(?)', [json.encode(local)]); + db.execute( + 'INSERT INTO users (id, name) VALUES (uuid(), ?)', ['local']); + + // Migrate to synced table. Because the previous local write would never + // get uploaded, this clears local data. + db.execute('SELECT powersync_replace_schema(?)', [json.encode(synced)]); + expect(db.select('SELECT * FROM users'), isEmpty); + + // The local table should not exist anymore. + expect(() => db.select('SELECT * FROM ps_data_local__users'), + throwsA(isA())); + }); + }); + group('metadata', () { // This is a special because we have two delete triggers when // include_metadata is true (one for actual `DELETE` statements and one From 409d892ad27e6f53720b5374175e89e92d4f8053 Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Thu, 6 Nov 2025 11:13:45 +0100 Subject: [PATCH 3/3] Delete data from synced -> local --- crates/core/src/schema/management.rs | 21 ++++++--------------- crates/core/src/sync_local.rs | 25 ++++++++++--------------- dart/test/schema_test.dart | 5 ++--- 3 files changed, 18 insertions(+), 33 deletions(-) diff --git a/crates/core/src/schema/management.rs b/crates/core/src/schema/management.rs index 31497c63..66a3d8b0 100644 --- a/crates/core/src/schema/management.rs +++ b/crates/core/src/schema/management.rs @@ -39,25 +39,16 @@ fn update_tables(db: *mut sqlite::sqlite3, schema: &Schema) -> Result<(), PowerS // In a block so that all statements are finalized before dropping tables. for table in &schema.tables { if let Some(existing) = existing_tables.remove(&*table.name) { - if existing.local_only && !table.local_only() { - // Migrate from a local-only to a synced table. Because none of the local writes - // would have created CRUD entries, we'll have to re-create the table from - // scratch. + if existing.local_only != table.local_only() { + // Migrating between local-only and synced tables. This works by deleting + // existing and re-creating the table from scratch. We can re-create first and + // delete the old table afterwards because they have a different name + // (local-only tables have a ps_data_local prefix). // To delete the old existing table in the end. existing_tables.insert(&existing.name, existing); - } else if !existing.local_only && table.local_only() { - // Migrate from a synced table to a local-only table. We can keep existing rows - // and will also keep existing CRUD data to be uploaded before the switch. - db.exec_safe(&format!( - "ALTER TABLE {} RENAME TO {}", - quote_identifier(&existing.internal_name), - quote_identifier(&table.internal_name()), - )) - .into_db_result(db)?; - continue; } else { - // Identical table exists already, nothing to do. + // Compatible table exists already, nothing to do. continue; } } diff --git a/crates/core/src/sync_local.rs b/crates/core/src/sync_local.rs index f66206fe..bbaabe98 100644 --- a/crates/core/src/sync_local.rs +++ b/crates/core/src/sync_local.rs @@ -5,6 +5,7 @@ use alloc::vec::Vec; use serde::Deserialize; use crate::error::{PSResult, PowerSyncError}; +use crate::schema::inspection::ExistingTable; use crate::schema::{PendingStatement, PendingStatementValue, RawTable, Schema}; use crate::state::DatabaseState; use crate::sync::BucketPriority; @@ -434,22 +435,16 @@ impl<'a> ParsedDatabaseSchema<'a> { } fn add_from_db(&mut self, db: *mut sqlite::sqlite3) -> Result<(), PowerSyncError> { - // language=SQLite - let statement = db - .prepare_v2( - "SELECT name FROM sqlite_master WHERE type='table' AND name GLOB 'ps_data_*'", - ) - .into_db_result(db)?; - - while statement.step()? == ResultCode::ROW { - let name = statement.column_text(0)?; - // Strip the ps_data__ prefix so that we can lookup tables by their sync protocol name. - let visible_name = name.get(9..).unwrap_or(name); - - // Tables which haven't been passed explicitly are assumed to not be raw tables. - self.tables - .insert(String::from(visible_name), ParsedSchemaTable::json_table()); + let tables = ExistingTable::list(db)?; + for table in tables { + if !table.local_only { + let visible_name = table.name; + + self.tables + .insert(visible_name, ParsedSchemaTable::json_table()); + } } + Ok(()) } } diff --git a/dart/test/schema_test.dart b/dart/test/schema_test.dart index f0f92034..995e6937 100644 --- a/dart/test/schema_test.dart +++ b/dart/test/schema_test.dart @@ -91,9 +91,8 @@ void main() { throwsA(isA())); // Data should still be there. - expect(db.select('SELECT * FROM users'), [ - {'id': 'synced-id', 'name': 'name'} - ]); + expect(db.select('SELECT * FROM ps_untyped'), hasLength(1)); + expect(db.select('SELECT * FROM users'), isEmpty); // Inserting into local-only table should not record CRUD item. db.execute(