From 9b6ae2d38a5d1a38ad04ad62a273624a8e460357 Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Wed, 17 Jun 2026 12:10:45 +0200 Subject: [PATCH 1/2] Don't create transactions automatically --- crates/core/src/error.rs | 8 +++-- crates/core/src/macros.rs | 39 -------------------- crates/core/src/schema/management.rs | 12 +++---- crates/core/src/sync/interface.rs | 4 +-- crates/core/src/utils/mod.rs | 21 +++++++++-- crates/core/src/view_admin.rs | 24 +++++++------ dart/test/crud_test.dart | 40 ++++++++++----------- dart/test/error_test.dart | 6 ++-- dart/test/js_key_encoding_test.dart | 6 ++-- dart/test/migration_test.dart | 36 +++++++++---------- dart/test/schema_test.dart | 41 ++++++++++++++-------- dart/test/sync_local_performance_test.dart | 2 +- dart/test/sync_stream_test.dart | 11 +++--- dart/test/sync_test.dart | 29 ++++++++------- dart/test/update_hooks_test.dart | 3 +- dart/test/utils/test_utils.dart | 13 +++++++ 16 files changed, 155 insertions(+), 140 deletions(-) diff --git a/crates/core/src/error.rs b/crates/core/src/error.rs index e39cb43d..093f8f9f 100644 --- a/crates/core/src/error.rs +++ b/crates/core/src/error.rs @@ -125,7 +125,7 @@ impl PowerSyncError { match self.inner.as_ref() { Sqlite(desc) => desc.code, ArgumentError { .. } => ResultCode::CONSTRAINT_DATATYPE, - StateError { .. } => ResultCode::MISUSE, + StateError { .. } | MustBeCalledInTransaction { .. } => ResultCode::MISUSE, MissingClientId | SyncProtocolError { .. } | DownMigrationDidNotUpdateVersion { .. } @@ -193,7 +193,7 @@ impl From for PowerSyncError { /// A structured enumeration of possible errors that can occur in the core extension. #[derive(Error, Debug)] -enum RawPowerSyncError { +pub enum RawPowerSyncError { /// An internal call to SQLite made by the core extension has failed. We store the original /// result code and an optional context describing what the core extension was trying to do when /// the error occurred. @@ -248,10 +248,12 @@ enum RawPowerSyncError { libversion_number: c_int, libversion: &'static str, }, + #[error("{function_name} may only be called in transactions.")] + MustBeCalledInTransaction { function_name: &'static str }, } #[derive(Debug)] -struct SqliteError { +pub struct SqliteError { code: ResultCode, errstr: Option, context: Option>, diff --git a/crates/core/src/macros.rs b/crates/core/src/macros.rs index 6d7d25ae..4e60c953 100644 --- a/crates/core/src/macros.rs +++ b/crates/core/src/macros.rs @@ -43,42 +43,3 @@ macro_rules! create_sqlite_optional_text_fn { } }; } - -// Wrap a function in an auto-transaction. -// Gives the equivalent of SQLite's auto-commit behaviour, except that applies to all statements -// inside the function. Otherwise, each statement inside the function would be a transaction on its -// own if the function itself is not wrapped in a transaction. -#[macro_export] -macro_rules! create_auto_tx_function { - ($fn_name:ident, $fn_impl_name:ident) => { - fn $fn_name( - ctx: *mut sqlite::context, - args: &[*mut sqlite::value], - ) -> Result { - let db = ctx.db_handle(); - - // Auto-start a transaction if we're not in a transaction - let started_tx = if db.get_autocommit() { - db.exec_safe("BEGIN")?; - true - } else { - false - }; - - let result = $fn_impl_name(ctx, args); - if result.is_err() { - // Always ROLLBACK, even when we didn't start the transaction. - // Otherwise the user may be able to continue the transaction and end up in an inconsistent state. - // We ignore rollback errors. - if !db.get_autocommit() { - let _ignore = db.exec_safe("ROLLBACK"); - } - } else if started_tx { - // Only COMMIT our own transactions. - db.exec_safe("COMMIT")?; - } - - result - } - }; -} diff --git a/crates/core/src/schema/management.rs b/crates/core/src/schema/management.rs index 98fed058..f4467ba7 100644 --- a/crates/core/src/schema/management.rs +++ b/crates/core/src/schema/management.rs @@ -13,17 +13,17 @@ use powersync_sqlite_nostd as sqlite; use powersync_sqlite_nostd::Context; use sqlite::{Connection, ResultCode, Value}; +use crate::create_sqlite_text_fn; use crate::error::{PSResult, PowerSyncError}; use crate::ext::ExtendedDatabase; use crate::schema::inspection::{ExistingTable, ExistingView}; use crate::schema::table_info::Index; use crate::state::DatabaseState; -use crate::utils::SqlBuffer; +use crate::utils::{SqlBuffer, verify_in_transaction}; use crate::views::{ powersync_trigger_delete_sql, powersync_trigger_insert_sql, powersync_trigger_update_sql, powersync_view_sql, }; -use crate::{create_auto_tx_function, create_sqlite_text_fn}; use super::Schema; @@ -265,13 +265,14 @@ fn powersync_replace_schema_impl( ctx: *mut sqlite::context, args: &[*mut sqlite::value], ) -> Result { + let db = ctx.db_handle(); + verify_in_transaction(db, "powersync_replace_schema")?; + let schema = args[0].text(); let state = unsafe { DatabaseState::from_context(&ctx) }; let parsed_schema = serde_json::from_str::(schema).map_err(PowerSyncError::as_argument_error)?; - let db = ctx.db_handle(); - // language=SQLite db.exec_safe("SELECT powersync_init()").into_db_result(db)?; @@ -283,10 +284,9 @@ fn powersync_replace_schema_impl( Ok(String::from("")) } -create_auto_tx_function!(powersync_replace_schema_tx, powersync_replace_schema_impl); create_sqlite_text_fn!( powersync_replace_schema, - powersync_replace_schema_tx, + powersync_replace_schema_impl, "powersync_replace_schema" ); diff --git a/crates/core/src/sync/interface.rs b/crates/core/src/sync/interface.rs index 546c843e..3c263068 100644 --- a/crates/core/src/sync/interface.rs +++ b/crates/core/src/sync/interface.rs @@ -22,7 +22,7 @@ use serde_json::value::RawValue; use sqlite::{ResultCode, Value}; use crate::sync::BucketPriority; -use crate::utils::JsonString; +use crate::utils::{JsonString, verify_in_transaction}; /// Payload provided by SDKs when requesting a sync iteration. #[derive(Deserialize)] @@ -206,7 +206,7 @@ pub fn register(db: *mut sqlite::sqlite3, state: Rc) -> Result<() ) -> () { let result = (|| -> Result<(), PowerSyncError> { let db = ctx.db_handle(); - debug_assert!(!db.get_autocommit()); + verify_in_transaction(db, "powersync_control")?; let state = unsafe { DatabaseState::from_context(&ctx) }; let args = sqlite::args!(argc, argv); diff --git a/crates/core/src/utils/mod.rs b/crates/core/src/utils/mod.rs index 06b7c50d..dc71dcea 100644 --- a/crates/core/src/utils/mod.rs +++ b/crates/core/src/utils/mod.rs @@ -3,14 +3,31 @@ mod sql_buffer; use core::{cmp::Ordering, fmt::Display, hash::Hash}; use alloc::{boxed::Box, string::String}; -use powersync_sqlite_nostd::{ColumnType, ManagedStmt}; +use powersync_sqlite_nostd::{ColumnType, Connection, ManagedStmt, sqlite3}; use serde::Serialize; use serde_json::value::RawValue; pub use sql_buffer::{InsertIntoCrud, SqlBuffer, WriteType}; -use crate::error::PowerSyncError; +use crate::error::{PowerSyncError, RawPowerSyncError}; use uuid::Uuid; +#[cold] +fn must_be_in_tx_error(function_name: &'static str) -> PowerSyncError { + return RawPowerSyncError::MustBeCalledInTransaction { function_name }.into(); +} + +#[inline] +pub fn verify_in_transaction( + db: *mut sqlite3, + function_name: &'static str, +) -> Result<(), PowerSyncError> { + if db.get_autocommit() { + return Err(must_be_in_tx_error(function_name)); + } + + Ok(()) +} + /// Calls [read] to read a column if it's not null, otherwise returns [None]. #[inline] pub fn column_nullable Result>( diff --git a/crates/core/src/view_admin.rs b/crates/core/src/view_admin.rs index 75163438..4a82afe9 100644 --- a/crates/core/src/view_admin.rs +++ b/crates/core/src/view_admin.rs @@ -10,12 +10,12 @@ use powersync_sqlite_nostd as sqlite; use powersync_sqlite_nostd::{Connection, Context}; use sqlite::{ResultCode, Value}; +use crate::create_sqlite_text_fn; use crate::error::{PSResult, PowerSyncError}; use crate::migrations::{LATEST_VERSION, powersync_migrate}; use crate::schema::inspection::ExistingView; use crate::state::DatabaseState; -use crate::utils::SqlBuffer; -use crate::{create_auto_tx_function, create_sqlite_text_fn}; +use crate::utils::{SqlBuffer, verify_in_transaction}; // Used in old down migrations, do not remove. extern "C" fn powersync_drop_view( @@ -35,6 +35,8 @@ fn powersync_init_impl( ctx: *mut sqlite::context, _args: &[*mut sqlite::value], ) -> Result { + let db = ctx.db_handle(); + verify_in_transaction(db, "powersync_init")?; powersync_migrate(ctx, LATEST_VERSION)?; // Register the powersync_internal_close vtab to implement a "pre-close hook". @@ -45,23 +47,24 @@ fn powersync_init_impl( Ok(String::from("")) } -create_auto_tx_function!(powersync_init_tx, powersync_init_impl); -create_sqlite_text_fn!(powersync_init, powersync_init_tx, "powersync_init"); +create_sqlite_text_fn!(powersync_init, powersync_init_impl, "powersync_init"); fn powersync_test_migration_impl( ctx: *mut sqlite::context, args: &[*mut sqlite::value], ) -> Result { + let db = ctx.db_handle(); + verify_in_transaction(db, "powersync_test_migration")?; + let target_version = args[0].int(); powersync_migrate(ctx, target_version)?; Ok(String::from("")) } -create_auto_tx_function!(powersync_test_migration_tx, powersync_test_migration_impl); create_sqlite_text_fn!( powersync_test_migration, - powersync_test_migration_tx, + powersync_test_migration_impl, "powersync_test_migration" ); @@ -70,6 +73,7 @@ fn powersync_clear_impl( args: &[*mut sqlite::value], ) -> Result { let local_db = ctx.db_handle(); + verify_in_transaction(local_db, "powersync_clear")?; let state = unsafe { DatabaseState::from_context(&ctx) }; let flags = PowerSyncClearFlags(args[0].int()); @@ -176,6 +180,8 @@ fn powersync_trigger_resync_impl( args: &[*mut sqlite::value], ) -> Result { let local_db = ctx.db_handle(); + verify_in_transaction(local_db, "powersync_trigger_resync")?; + let state = unsafe { DatabaseState::from_context(&ctx) }; trigger_resync(local_db, state)?; @@ -187,10 +193,9 @@ fn powersync_trigger_resync_impl( Ok(Default::default()) } -create_auto_tx_function!(powersync_trigger_resync_tx, powersync_trigger_resync_impl); create_sqlite_text_fn!( powersync_trigger_resync, - powersync_trigger_resync_tx, + powersync_trigger_resync_impl, "powersync_trigger_resync" ); @@ -210,8 +215,7 @@ impl PowerSyncClearFlags { } } -create_auto_tx_function!(powersync_clear_tx, powersync_clear_impl); -create_sqlite_text_fn!(powersync_clear, powersync_clear_tx, "powersync_clear"); +create_sqlite_text_fn!(powersync_clear, powersync_clear_impl, "powersync_clear"); 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. diff --git a/dart/test/crud_test.dart b/dart/test/crud_test.dart index 49dc43a2..5d6d379b 100644 --- a/dart/test/crud_test.dart +++ b/dart/test/crud_test.dart @@ -72,10 +72,10 @@ void main() { {'name': 'items', 'columns': columns} ] }; - db.select('select powersync_init()'); + db.executeInTx('select powersync_init()'); // 1. Test schema initialization - db.select( + db.executeInTx( 'select powersync_replace_schema(?)', [jsonEncode(tableSchema)]); var columnNames = columns.map((c) => c['name']).join(', '); @@ -141,10 +141,10 @@ void main() { {'name': 'items', 'columns': columns, 'local_only': true} ] }; - db.select('select powersync_init()'); + db.executeInTx('select powersync_init()'); // 1. Test schema initialization - db.select( + db.executeInTx( 'select powersync_replace_schema(?)', [jsonEncode(tableSchema)]); var columnNames = columns.map((c) => c['name']).join(', '); @@ -181,10 +181,10 @@ void main() { {'name': 'items', 'columns': columns, 'insert_only': true} ] }; - db.select('select powersync_init()'); + db.executeInTx('select powersync_init()'); // 1. Test schema initialization - db.select( + db.executeInTx( 'select powersync_replace_schema(?)', [jsonEncode(tableSchema)]); var columnNames = columns.map((c) => c['name']).join(', '); @@ -226,7 +226,7 @@ void main() { group('crud vtab', () { setUp(() { - db.select('select powersync_init()'); + db.executeInTx('select powersync_init()'); }); group('simple', () { @@ -382,8 +382,8 @@ void main() { ] }; - db.select('select powersync_init()'); - db.select( + db.executeInTx('select powersync_init()'); + db.executeInTx( 'select powersync_replace_schema(?)', [jsonEncode(tableSchema)]); } @@ -524,8 +524,8 @@ void main() { ] }; - db.select('select powersync_init()'); - db.select( + db.executeInTx('select powersync_init()'); + db.executeInTx( 'select powersync_replace_schema(?)', [jsonEncode(tableSchema)]); } @@ -616,7 +616,7 @@ void main() { test('includes empty updates by default', () { db - ..execute('select powersync_replace_schema(?)', [ + ..executeInTx('select powersync_replace_schema(?)', [ json.encode({ 'tables': [ { @@ -638,7 +638,7 @@ void main() { test('can ignore empty updates', () { db - ..execute('select powersync_replace_schema(?)', [ + ..executeInTx('select powersync_replace_schema(?)', [ json.encode({ 'tables': [ { @@ -661,7 +661,7 @@ void main() { test('preserves values in text column', () { db - ..execute('select powersync_replace_schema(?)', [ + ..executeInTx('select powersync_replace_schema(?)', [ json.encode({ 'tables': [ { @@ -687,7 +687,7 @@ void main() { test('preserves mismatched type', () { db - ..execute('select powersync_replace_schema(?)', [ + ..executeInTx('select powersync_replace_schema(?)', [ json.encode({ 'tables': [ { @@ -715,7 +715,7 @@ void main() { group('insert only', () { test('smoke test', () { db - ..execute('select powersync_replace_schema(?)', [ + ..executeInTx('select powersync_replace_schema(?)', [ json.encode({ 'tables': [ { @@ -745,7 +745,7 @@ void main() { test('has no effect on local-only tables', () { db - ..execute('select powersync_replace_schema(?)', [ + ..executeInTx('select powersync_replace_schema(?)', [ json.encode({ 'tables': [ { @@ -779,7 +779,7 @@ void main() { group('raw tables', () { void createRawTableTriggers(Object table, {bool insert = true, bool update = true, bool delete = true}) { - db.execute('SELECT powersync_init()'); + db.executeInTx('SELECT powersync_init()'); if (insert) { db.execute('SELECT powersync_create_raw_table_crud_trigger(?, ?, ?)', @@ -954,7 +954,7 @@ void main() { createRawTableTriggers(rawTableDescription( {'table_name': 'users', 'ignore_empty_update': true})); - db.execute('select powersync_replace_schema(?)', [ + db.executeInTx('select powersync_replace_schema(?)', [ json.encode({ 'tables': [], 'raw_tables': [ @@ -963,7 +963,7 @@ void main() { }) ]); - db.execute('SELECT powersync_clear(2)'); + db.executeInTx('SELECT powersync_clear(2)'); expect(db.select('SELECT * FROM ps_crud'), isEmpty); }); }); diff --git a/dart/test/error_test.dart b/dart/test/error_test.dart index 62a9bf69..c4226ee5 100644 --- a/dart/test/error_test.dart +++ b/dart/test/error_test.dart @@ -17,7 +17,7 @@ void main() { db.execute('CREATE TABLE IF NOT EXISTS ps_migration(foo TEXT)'); expect( - () => db.execute('SELECT powersync_init()'), + () => db.executeInTx('SELECT powersync_init()'), throwsA(isSqliteException( 1, 'powersync_init: internal SQLite call returned ERROR: no such column: id', @@ -27,7 +27,7 @@ void main() { test('missing client id', () { db - ..execute('SELECT powersync_init()') + ..executeInTx('SELECT powersync_init()') ..execute('DELETE FROM ps_kv;'); expect( @@ -40,7 +40,7 @@ void main() { }); group('sync protocol', () { - setUp(() => db.execute('SELECT powersync_init()')); + setUp(() => db.executeInTx('SELECT powersync_init()')); test('invalid json', () { const stmt = 'SELECT powersync_control(?,?)'; diff --git a/dart/test/js_key_encoding_test.dart b/dart/test/js_key_encoding_test.dart index 17a773cf..7e5c68b2 100644 --- a/dart/test/js_key_encoding_test.dart +++ b/dart/test/js_key_encoding_test.dart @@ -4,14 +4,16 @@ import 'package:sqlite3/common.dart'; import 'package:test/test.dart'; import 'utils/native_test_utils.dart'; +import 'utils/test_utils.dart'; void main() { late CommonDatabase db; setUp(() async { db = openTestDatabase() - ..select('select powersync_init();') - ..select('select powersync_replace_schema(?)', [json.encode(_schema)]); + ..executeInTx('select powersync_init();') + ..executeInTx( + 'select powersync_replace_schema(?)', [json.encode(_schema)]); }); test('can fix JS key encoding', () { diff --git a/dart/test/migration_test.dart b/dart/test/migration_test.dart index d442edd1..9203f74f 100644 --- a/dart/test/migration_test.dart +++ b/dart/test/migration_test.dart @@ -41,7 +41,7 @@ void main() { /// Get this test passing before any others below, since it tests the /// finalState fixture, which is an input in other tests. test('create database from scratch', () async { - db.select('select powersync_init()'); + db.executeInTx('select powersync_init()'); final schema = '${getSchema(db)}\n${getMigrations(db)}'; final expected = fixtures.finalState.trim(); if (expected != schema) { @@ -60,7 +60,7 @@ void main() { /// This tests with just the base tables test('migrate from $startState', () async { db.execute(fixtures.expectedState[startState]!); - db.select('select powersync_init()'); + db.executeInTx('select powersync_init()'); final schema = '${getSchema(db)}\n${getMigrations(db)}'; expect(schema, equals(fixtures.finalState.trim())); }); @@ -69,7 +69,7 @@ void main() { test('migrate from $startState with data1', () async { db.execute(fixtures.expectedState[startState]!); db.execute(fixtures.data1[startState]!); - db.select('select powersync_init()'); + db.executeInTx('select powersync_init()'); final data = getData(db); expect(data, equals(fixtures.finalData1.trim())); }); @@ -82,7 +82,7 @@ void main() { /// This tests with just the base tables test('migrate down to $endState', () async { db.execute(fixtures.finalState); - db.select('select powersync_test_migration(?)', [endState]); + db.executeInTx('select powersync_test_migration(?)', [endState]); final schema = '${getSchema(db)}\n${getMigrations(db)}'; expect(schema, equals(fixtures.expectedState[endState]!.trim())); }); @@ -91,7 +91,7 @@ void main() { test('migrate down to $endState with data1', () async { db.execute(fixtures.finalState); db.execute(fixtures.data1[fixtures.databaseVersion]!); - db.select('select powersync_test_migration(?)', [endState]); + db.executeInTx('select powersync_test_migration(?)', [endState]); final data = getData(db); expect(data, equals(fixtures.dataDown1[endState]!.trim())); }); @@ -110,8 +110,8 @@ void main() { } ] }; - db.select('select powersync_init()'); - db.select( + db.executeInTx('select powersync_init()'); + db.executeInTx( 'select powersync_replace_schema(?)', [jsonEncode(tableSchema)]); final schema = getSchema(db); @@ -134,8 +134,8 @@ void main() { } ] }; - db.select('select powersync_init()'); - db.select( + db.executeInTx('select powersync_init()'); + db.executeInTx( 'select powersync_replace_schema(?)', [jsonEncode(tableSchema)]); final schema = getSchema(db); @@ -149,7 +149,7 @@ void main() { test('schema 5 -> 4', () async { db.execute(fixtures.expectedState[5]!); db.execute(fixtures.schema5); - db.select('select powersync_test_migration(4)'); + db.executeInTx('select powersync_test_migration(4)'); final schema = getSchema(db); // Note that this schema contains no views - those are deleted during the migration @@ -165,7 +165,7 @@ void main() { test('schema 5 -> 3', () async { db.execute(fixtures.expectedState[5]!); db.execute(fixtures.schema5); - db.select('select powersync_test_migration(3)'); + db.executeInTx('select powersync_test_migration(3)'); final schema = getSchema(db); // Note that this schema contains no views - those are deleted during the migration @@ -191,14 +191,14 @@ void main() { } ] }; - db.select('select powersync_init()'); - db.select( + db.executeInTx('select powersync_init()'); + db.executeInTx( 'select powersync_replace_schema(?)', [jsonEncode(tableSchema)]); - db.select('select powersync_test_migration(5)'); + db.executeInTx('select powersync_test_migration(5)'); db.execute(fix035.dataBroken); - db.select('select powersync_init()'); + db.executeInTx('select powersync_init()'); final data = getData(db); expect(data, equals(fix035.dataMigrated.trim())); @@ -245,8 +245,8 @@ end'''); } ] }; - db.select('select powersync_init()'); - db.select( + db.executeInTx('select powersync_init()'); + db.executeInTx( 'select powersync_replace_schema(?)', [jsonEncode(tableSchema)]); final schema = getSchema(db); @@ -274,7 +274,7 @@ end'''; ); } - db.execute('SELECT powersync_test_migration(8);'); + db.executeInTx('SELECT powersync_test_migration(8);'); expect(db.select('SELECT * FROM ps_sync_state ORDER BY priority'), [ { diff --git a/dart/test/schema_test.dart b/dart/test/schema_test.dart index 1e73a457..dfbd4ff1 100644 --- a/dart/test/schema_test.dart +++ b/dart/test/schema_test.dart @@ -4,6 +4,7 @@ import 'package:sqlite3/common.dart'; import 'package:test/test.dart'; import 'utils/native_test_utils.dart'; +import 'utils/test_utils.dart'; void main() { late CommonDatabase db; @@ -16,24 +17,28 @@ void main() { test('Schema versioning', () { // Test that powersync_replace_schema() is a no-op when the schema is not // modified. - db.execute('SELECT powersync_replace_schema(?)', [json.encode(schema)]); + db.executeInTx( + 'SELECT powersync_replace_schema(?)', [json.encode(schema)]); final [versionBefore] = db.select('PRAGMA schema_version'); - db.execute('SELECT powersync_replace_schema(?)', [json.encode(schema)]); + db.executeInTx( + 'SELECT powersync_replace_schema(?)', [json.encode(schema)]); final [versionAfter] = db.select('PRAGMA schema_version'); // No change expect(versionAfter['schema_version'], equals(versionBefore['schema_version'])); - db.execute('SELECT powersync_replace_schema(?)', [json.encode(schema2)]); + db.executeInTx( + 'SELECT powersync_replace_schema(?)', [json.encode(schema2)]); final [versionAfter2] = db.select('PRAGMA schema_version'); // Updated expect(versionAfter2['schema_version'], greaterThan(versionAfter['schema_version'] as int)); - db.execute('SELECT powersync_replace_schema(?)', [json.encode(schema3)]); + db.executeInTx( + 'SELECT powersync_replace_schema(?)', [json.encode(schema3)]); final [versionAfter3] = db.select('PRAGMA schema_version'); // Updated again (index) @@ -70,7 +75,8 @@ void main() { test('from synced to local', () { // Start with synced table, and sync row - db.execute('SELECT powersync_replace_schema(?)', [json.encode(synced)]); + db.executeInTx( + 'SELECT powersync_replace_schema(?)', [json.encode(synced)]); db.execute( 'INSERT INTO ps_data__users (id, data) VALUES (?, ?)', [ @@ -80,7 +86,8 @@ void main() { ); // Migrate to local table. - db.execute('SELECT powersync_replace_schema(?)', [json.encode(local)]); + db.executeInTx( + 'SELECT powersync_replace_schema(?)', [json.encode(local)]); // The synced table should not exist anymore. expect(() => db.select('SELECT * FROM ps_data__users'), @@ -98,13 +105,15 @@ void main() { test('from local to synced', () { // Start with local table, and local row - db.execute('SELECT powersync_replace_schema(?)', [json.encode(local)]); + db.executeInTx( + '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)]); + db.executeInTx( + 'SELECT powersync_replace_schema(?)', [json.encode(synced)]); expect(db.select('SELECT * FROM users'), isEmpty); // The local table should not exist anymore. @@ -137,7 +146,7 @@ void main() { } test('enabling', () { - db.execute('SELECT powersync_replace_schema(?)', + db.executeInTx('SELECT powersync_replace_schema(?)', [json.encode(createSchema(false))]); expect( db.select("select * from sqlite_schema where type = 'trigger' " @@ -146,7 +155,7 @@ void main() { hasLength(1), ); - db.execute('SELECT powersync_replace_schema(?)', + db.executeInTx('SELECT powersync_replace_schema(?)', [json.encode(createSchema(true))]); expect( db.select("select * from sqlite_schema where type = 'trigger' " @@ -158,10 +167,12 @@ void main() { test('unchanged', () { final schema = createSchema(true); - db.execute('SELECT powersync_replace_schema(?)', [json.encode(schema)]); + db.executeInTx( + 'SELECT powersync_replace_schema(?)', [json.encode(schema)]); final [versionBefore] = db.select('PRAGMA schema_version'); - db.execute('SELECT powersync_replace_schema(?)', [json.encode(schema)]); + db.executeInTx( + 'SELECT powersync_replace_schema(?)', [json.encode(schema)]); final [versionAfter] = db.select('PRAGMA schema_version'); expect(versionAfter['schema_version'], @@ -169,7 +180,7 @@ void main() { }); test('disabling', () { - db.execute('SELECT powersync_replace_schema(?)', + db.executeInTx('SELECT powersync_replace_schema(?)', [json.encode(createSchema(true))]); expect( db.select("select * from sqlite_schema where type = 'trigger' " @@ -178,7 +189,7 @@ void main() { hasLength(2), ); - db.execute('SELECT powersync_replace_schema(?)', + db.executeInTx('SELECT powersync_replace_schema(?)', [json.encode(createSchema(false))]); expect( db.select("select * from sqlite_schema where type = 'trigger' " @@ -190,7 +201,7 @@ void main() { }); test('raw tables', () { - db.execute('SELECT powersync_replace_schema(?)', [ + db.executeInTx('SELECT powersync_replace_schema(?)', [ json.encode({ 'raw_tables': [ { diff --git a/dart/test/sync_local_performance_test.dart b/dart/test/sync_local_performance_test.dart index 9f68597c..92ba1c2b 100644 --- a/dart/test/sync_local_performance_test.dart +++ b/dart/test/sync_local_performance_test.dart @@ -47,7 +47,7 @@ void testFilesystemOperations( setUp(() { // Optional: set a custom cache size - it affects the number of filesystem operations. // db.execute('PRAGMA cache_size=-50000'); - db.execute('SELECT powersync_replace_schema(?)', [json.encode(schema)]); + db.executeInTx('SELECT powersync_replace_schema(?)', [json.encode(schema)]); // Generate dummy data // We can replace this with actual similated download operations later db.execute(''' diff --git a/dart/test/sync_stream_test.dart b/dart/test/sync_stream_test.dart index ef77e6d3..e5ba8ac1 100644 --- a/dart/test/sync_stream_test.dart +++ b/dart/test/sync_stream_test.dart @@ -25,8 +25,9 @@ void main() { setUp(() async { db = openTestDatabase(vfs: vfs) - ..select('select powersync_init();') - ..select('select powersync_replace_schema(?)', [json.encode(testSchema)]) + ..executeInTx('select powersync_init();') + ..executeInTx( + 'select powersync_replace_schema(?)', [json.encode(testSchema)]) ..execute('update ps_kv set value = ?2 where key = ?1', ['client_id', 'test-test-test-test']); }); @@ -654,14 +655,14 @@ void main() { }), ); expect(db.select('select * from ps_stream_subscriptions'), isNotEmpty); - db.execute('select powersync_clear(0);'); + db.executeInTx('select powersync_clear(0);'); expect(db.select('select * from ps_stream_subscriptions'), isEmpty); }); syncTest('can migrate from old timestamps', (async) { // Migrate ps_sync_state to text-based date values and stream subscriptions // to timestamps with second precision. - db.execute('SELECT powersync_test_migration(?)', [12]); + db.executeInTx('SELECT powersync_test_migration(?)', [12]); // Mark as synced db @@ -682,7 +683,7 @@ void main() { ], ); - db.execute('SELECT powersync_test_migration(?)', [13]); + db.executeInTx('SELECT powersync_test_migration(?)', [13]); final [statusRow] = db.select('SELECT powersync_offline_sync_status()'); expect( diff --git a/dart/test/sync_test.dart b/dart/test/sync_test.dart index ab92e249..579ece80 100644 --- a/dart/test/sync_test.dart +++ b/dart/test/sync_test.dart @@ -74,8 +74,9 @@ void _syncTests({ setUp(() async { db = openTestDatabase(vfs: vfs) - ..select('select powersync_init();') - ..select('select powersync_replace_schema(?)', [json.encode(testSchema)]) + ..executeInTx('select powersync_init();') + ..executeInTx( + 'select powersync_replace_schema(?)', [json.encode(testSchema)]) ..execute('update ps_kv set value = ?2 where key = ?1', ['client_id', 'test-test-test-test']); @@ -378,7 +379,7 @@ void _syncTests({ expect(db.select('SELECT priority FROM ps_sync_state').single, {'priority': 2147483647}); - db.execute('SELECT powersync_clear(0)'); + db.executeInTx('SELECT powersync_clear(0)'); expect(db.select('SELECT * FROM ps_sync_state'), hasLength(0)); }); @@ -394,8 +395,9 @@ void _syncTests({ invokeControl('stop', null); // Soft clear - db.execute('SELECT powersync_clear(2)'); - db.select('select powersync_replace_schema(?)', [json.encode(testSchema)]); + db.executeInTx('SELECT powersync_clear(2)'); + db.executeInTx( + 'select powersync_replace_schema(?)', [json.encode(testSchema)]); expect(db.select('SELECT * FROM items'), hasLength(0)); expect( db.select(r"SELECT * FROM ps_buckets WHERE name = '$local'"), isEmpty); @@ -424,7 +426,7 @@ void _syncTests({ invokeControl('start', null); expect( - () => db.select('SELECT powersync_trigger_resync(1)'), + () => db.executeInTx('SELECT powersync_trigger_resync(1)'), throwsA( isSqliteException( 3091, @@ -443,7 +445,7 @@ void _syncTests({ invokeControl('stop', null); db.execute('delete from ps_data__items'); - db.execute('select powersync_trigger_resync(0)'); + db.executeInTx('select powersync_trigger_resync(0)'); final instructions = invokeControl('start', null); expect( @@ -476,7 +478,7 @@ void _syncTests({ pushCheckpointComplete(); invokeControl('stop', null); - db.execute('select powersync_trigger_resync(1)'); + db.executeInTx('select powersync_trigger_resync(1)'); final [row] = db.select('select powersync_offline_sync_status()'); expect(json.decode(row.columnAt(0)), containsPair('priority_status', isEmpty)); @@ -1002,8 +1004,8 @@ void _syncTests({ final fileName = d.path('test.db'); db = openTestDatabase(fileName: fileName) - ..select('select powersync_init();') - ..select( + ..executeInTx('select powersync_init();') + ..executeInTx( 'select powersync_replace_schema(?)', [json.encode(testSchema)]) ..execute('update ps_kv set value = ?2 where key = ?1', ['client_id', 'test-test-test-test']); @@ -1503,11 +1505,12 @@ END; test('clear', () { setupRawTables(); - db.execute('SELECT powersync_replace_schema(?)', [json.encode(schema)]); + db.executeInTx( + 'SELECT powersync_replace_schema(?)', [json.encode(schema)]); db.execute('INSERT INTO users (id, name) VALUES (uuid(), ?)', ['test']); expect(db.select('SELECT * FROM users'), hasLength(1)); - db.execute('SELECT powersync_clear(0)'); + db.executeInTx('SELECT powersync_clear(0)'); expect(db.select('SELECT * FROM users'), hasLength(0)); }); @@ -1636,7 +1639,7 @@ CREATE TRIGGER users_ref_delete addTearDown(() => sqlite3.unregisterVirtualFileSystem(vfs)); db = openTestDatabase(vfs: vfs, fileName: '/test.db') - ..select('select powersync_init();'); + ..executeInTx('select powersync_init();'); invokeControl('start', null); expect(vfs.openFiles, isPositive); db.close(); diff --git a/dart/test/update_hooks_test.dart b/dart/test/update_hooks_test.dart index 1d36503d..4aa796c5 100644 --- a/dart/test/update_hooks_test.dart +++ b/dart/test/update_hooks_test.dart @@ -4,13 +4,14 @@ import 'package:sqlite3/common.dart'; import 'package:test/test.dart'; import 'utils/native_test_utils.dart'; +import 'utils/test_utils.dart'; void main() { late CommonDatabase db; setUp(() async { db = openTestDatabase() - ..select('select powersync_init()') + ..executeInTx('select powersync_init()') ..execute('CREATE TABLE foo (bar INTEGER);') ..select("SELECT powersync_update_hooks('install')"); }); diff --git a/dart/test/utils/test_utils.dart b/dart/test/utils/test_utils.dart index 53ad01c2..943f632d 100644 --- a/dart/test/utils/test_utils.dart +++ b/dart/test/utils/test_utils.dart @@ -76,3 +76,16 @@ const testSchema = { } ] }; + +extension TransactionUtils on CommonDatabase { + void executeInTx(String sql, [List args = const []]) { + execute('BEGIN'); + try { + execute(sql, args); + execute('COMMIT'); + } on Object { + execute('ROLLBACK'); + rethrow; + } + } +} From 73bcfdc133fb6bd9bb0267c9d31f5a4445feb3b6 Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Wed, 17 Jun 2026 12:27:33 +0200 Subject: [PATCH 2/2] Expand tests --- crates/core/src/error.rs | 4 ++-- crates/core/src/schema/management.rs | 2 +- crates/core/src/sync/interface.rs | 2 +- crates/core/src/utils/mod.rs | 11 ++++------ crates/core/src/view_admin.rs | 8 ++++---- dart/test/error_test.dart | 30 ++++++++++++++++++++++++++++ 6 files changed, 42 insertions(+), 15 deletions(-) diff --git a/crates/core/src/error.rs b/crates/core/src/error.rs index 093f8f9f..e9a8856d 100644 --- a/crates/core/src/error.rs +++ b/crates/core/src/error.rs @@ -248,8 +248,8 @@ pub enum RawPowerSyncError { libversion_number: c_int, libversion: &'static str, }, - #[error("{function_name} may only be called in transactions.")] - MustBeCalledInTransaction { function_name: &'static str }, + #[error("This function may only be called in transactions.")] + MustBeCalledInTransaction, } #[derive(Debug)] diff --git a/crates/core/src/schema/management.rs b/crates/core/src/schema/management.rs index f4467ba7..5c29287f 100644 --- a/crates/core/src/schema/management.rs +++ b/crates/core/src/schema/management.rs @@ -266,7 +266,7 @@ fn powersync_replace_schema_impl( args: &[*mut sqlite::value], ) -> Result { let db = ctx.db_handle(); - verify_in_transaction(db, "powersync_replace_schema")?; + verify_in_transaction(db)?; let schema = args[0].text(); let state = unsafe { DatabaseState::from_context(&ctx) }; diff --git a/crates/core/src/sync/interface.rs b/crates/core/src/sync/interface.rs index 3c263068..e09eba94 100644 --- a/crates/core/src/sync/interface.rs +++ b/crates/core/src/sync/interface.rs @@ -206,7 +206,7 @@ pub fn register(db: *mut sqlite::sqlite3, state: Rc) -> Result<() ) -> () { let result = (|| -> Result<(), PowerSyncError> { let db = ctx.db_handle(); - verify_in_transaction(db, "powersync_control")?; + verify_in_transaction(db)?; let state = unsafe { DatabaseState::from_context(&ctx) }; let args = sqlite::args!(argc, argv); diff --git a/crates/core/src/utils/mod.rs b/crates/core/src/utils/mod.rs index dc71dcea..34f233a6 100644 --- a/crates/core/src/utils/mod.rs +++ b/crates/core/src/utils/mod.rs @@ -12,17 +12,14 @@ use crate::error::{PowerSyncError, RawPowerSyncError}; use uuid::Uuid; #[cold] -fn must_be_in_tx_error(function_name: &'static str) -> PowerSyncError { - return RawPowerSyncError::MustBeCalledInTransaction { function_name }.into(); +fn must_be_in_tx_error() -> PowerSyncError { + return RawPowerSyncError::MustBeCalledInTransaction.into(); } #[inline] -pub fn verify_in_transaction( - db: *mut sqlite3, - function_name: &'static str, -) -> Result<(), PowerSyncError> { +pub fn verify_in_transaction(db: *mut sqlite3) -> Result<(), PowerSyncError> { if db.get_autocommit() { - return Err(must_be_in_tx_error(function_name)); + return Err(must_be_in_tx_error()); } Ok(()) diff --git a/crates/core/src/view_admin.rs b/crates/core/src/view_admin.rs index 4a82afe9..6b9496c5 100644 --- a/crates/core/src/view_admin.rs +++ b/crates/core/src/view_admin.rs @@ -36,7 +36,7 @@ fn powersync_init_impl( _args: &[*mut sqlite::value], ) -> Result { let db = ctx.db_handle(); - verify_in_transaction(db, "powersync_init")?; + verify_in_transaction(db)?; powersync_migrate(ctx, LATEST_VERSION)?; // Register the powersync_internal_close vtab to implement a "pre-close hook". @@ -54,7 +54,7 @@ fn powersync_test_migration_impl( args: &[*mut sqlite::value], ) -> Result { let db = ctx.db_handle(); - verify_in_transaction(db, "powersync_test_migration")?; + verify_in_transaction(db)?; let target_version = args[0].int(); powersync_migrate(ctx, target_version)?; @@ -73,7 +73,7 @@ fn powersync_clear_impl( args: &[*mut sqlite::value], ) -> Result { let local_db = ctx.db_handle(); - verify_in_transaction(local_db, "powersync_clear")?; + verify_in_transaction(local_db)?; let state = unsafe { DatabaseState::from_context(&ctx) }; let flags = PowerSyncClearFlags(args[0].int()); @@ -180,7 +180,7 @@ fn powersync_trigger_resync_impl( args: &[*mut sqlite::value], ) -> Result { let local_db = ctx.db_handle(); - verify_in_transaction(local_db, "powersync_trigger_resync")?; + verify_in_transaction(local_db)?; let state = unsafe { DatabaseState::from_context(&ctx) }; trigger_resync(local_db, state)?; diff --git a/dart/test/error_test.dart b/dart/test/error_test.dart index c4226ee5..74da34a6 100644 --- a/dart/test/error_test.dart +++ b/dart/test/error_test.dart @@ -1,3 +1,5 @@ +import 'dart:convert'; + import 'package:sqlite3/common.dart'; import 'package:test/test.dart'; @@ -12,6 +14,34 @@ void main() { db = openTestDatabase(); }); + test('not in transaction', () { + void expectErrorOutsideOfTransaction(String sql, String functionName, + [List args = const []]) { + expect( + () => db.execute(sql, args), + throwsA(isSqliteException( + 21, + '$functionName: This function may only be called in transactions.', + )), + ); + } + + expectErrorOutsideOfTransaction( + 'SELECT powersync_init()', 'powersync_init'); + db.executeInTx('SELECT powersync_init()'); + + expectErrorOutsideOfTransaction('SELECT powersync_control(?, ?)', + 'powersync_control', ['STOP', null]); + expectErrorOutsideOfTransaction('SELECT powersync_replace_schema(?)', + 'powersync_replace_schema', [json.encode({})]); + expectErrorOutsideOfTransaction('SELECT powersync_test_migration(?)', + 'powersync_test_migration', [123]); + expectErrorOutsideOfTransaction( + 'SELECT powersync_clear(?)', 'powersync_clear', [0]); + expectErrorOutsideOfTransaction( + 'SELECT powersync_trigger_resync(TRUE)', 'powersync_trigger_resync'); + }); + test('contain inner SQLite descriptions', () { // Create a wrong migrations table for the core extension to trip over. db.execute('CREATE TABLE IF NOT EXISTS ps_migration(foo TEXT)');