From 1f4b5f28f353781395ea2a5373c4c1379273e5bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Gonz=C3=A1lez?= <7822554+AlexTMjugador@users.noreply.github.com> Date: Mon, 8 Sep 2025 23:55:58 +0200 Subject: [PATCH] feat(sqlite): `no_tx` migration support (#4015) * chore(sqlx-postgres): fix typo in `migrate.rs` comment * feat(sqlite): support `no_tx` migrations SQLite includes several SQL statements that are useful during migrations but must be executed outside of a transaction to take effect, such as `PRAGMA foreign_keys = ON|OFF` or `VACUUM`. Additionally, advanced migrations may want more precise control over how statements are grouped into transactions or savepoints to achieve the desired atomicity for different parts of the migration. While SQLx already supports marking migrations to run outside explicit transactions through a `-- no-transaction` comment, this feature is currently only available for `PgConnection`'s `Migrate` implementation, leaving SQLite and MySQL without this capability. Although it's possible to work around this limitation by implementing custom migration logic instead of executing `Migrator#run`, this comes at a cost of significantly reduced developer ergonomics: code that relies on the default migration logic, such as `#[sqlx::test]` or `cargo sqlx database setup`, won't support these migrations. These changes extend `SqliteConnection`'s `Migrate` implementation to support `no_tx` migrations in the same way as PostgreSQL, addressing this feature gap. I also considered implementing the same functionality for MySQL, but since I haven't found a practical use case for it yet, and every non-transaction-friendly statement I could think about in MySQL triggers implicit commits anyway, I determined it wasn't necessary at this time and could be considered an overreach. * test(sqlite): add test for `no_tx` migrations * chore(sqlx-sqlite): bring back useful comment * chore(sqlx-sqlite): unify SQL dialect in annotation comments --- sqlx-postgres/src/migrate.rs | 2 +- sqlx-sqlite/src/migrate.rs | 113 +++++++++++++-------- tests/sqlite/migrate.rs | 11 ++ tests/sqlite/migrations_no_tx/0_vacuum.sql | 3 + 4 files changed, 87 insertions(+), 42 deletions(-) create mode 100644 tests/sqlite/migrations_no_tx/0_vacuum.sql diff --git a/sqlx-postgres/src/migrate.rs b/sqlx-postgres/src/migrate.rs index b96c021be..49104672c 100644 --- a/sqlx-postgres/src/migrate.rs +++ b/sqlx-postgres/src/migrate.rs @@ -276,7 +276,7 @@ CREATE TABLE IF NOT EXISTS {table_name} ( if migration.no_tx { revert_migration(self, table_name, migration).await?; } else { - // Use a single transaction for the actual migration script and the essential bookeeping so we never + // Use a single transaction for the actual migration script and the essential bookkeeping so we never // execute migrations twice. See https://github.com/launchbadge/sqlx/issues/1966. let mut tx = self.begin().await?; revert_migration(&mut tx, table_name, migration).await?; diff --git a/sqlx-sqlite/src/migrate.rs b/sqlx-sqlite/src/migrate.rs index 6f1796d37..7eb8cc1ca 100644 --- a/sqlx-sqlite/src/migrate.rs +++ b/sqlx-sqlite/src/migrate.rs @@ -160,41 +160,27 @@ CREATE TABLE IF NOT EXISTS {table_name} ( migration: &'e Migration, ) -> BoxFuture<'e, Result> { Box::pin(async move { - let mut tx = self.begin().await?; let start = Instant::now(); - // Use a single transaction for the actual migration script and the essential bookeeping so we never - // execute migrations twice. See https://github.com/launchbadge/sqlx/issues/1966. - // The `execution_time` however can only be measured for the whole transaction. This value _only_ exists for - // data lineage and debugging reasons, so it is not super important if it is lost. So we initialize it to -1 - // and update it once the actual transaction completed. - let _ = tx - .execute(migration.sql.clone()) - .await - .map_err(|e| MigrateError::ExecuteMigration(e, migration.version))?; - - // language=SQL - let _ = query(AssertSqlSafe(format!( - r#" - INSERT INTO {table_name} ( version, description, success, checksum, execution_time ) - VALUES ( ?1, ?2, TRUE, ?3, -1 ) - "# - ))) - .bind(migration.version) - .bind(&*migration.description) - .bind(&*migration.checksum) - .execute(&mut *tx) - .await?; - - tx.commit().await?; + if migration.no_tx { + execute_migration(self, table_name, migration).await?; + } else { + // Use a single transaction for the actual migration script and the essential bookkeeping so we never + // execute migrations twice. See https://github.com/launchbadge/sqlx/issues/1966. + // The `execution_time` however can only be measured for the whole transaction. This value _only_ exists for + // data lineage and debugging reasons, so it is not super important if it is lost. So we initialize it to -1 + // and update it once the actual transaction completed. + let mut tx = self.begin().await?; + execute_migration(&mut tx, table_name, migration).await?; + tx.commit().await?; + } // Update `elapsed_time`. // NOTE: The process may disconnect/die at this point, so the elapsed time value might be lost. We accept // this small risk since this value is not super important. - let elapsed = start.elapsed(); - // language=SQL + // language=SQLite #[allow(clippy::cast_possible_truncation)] let _ = query(AssertSqlSafe(format!( r#" @@ -218,22 +204,17 @@ CREATE TABLE IF NOT EXISTS {table_name} ( migration: &'e Migration, ) -> BoxFuture<'e, Result> { Box::pin(async move { - // Use a single transaction for the actual migration script and the essential bookeeping so we never - // execute migrations twice. See https://github.com/launchbadge/sqlx/issues/1966. - let mut tx = self.begin().await?; let start = Instant::now(); - let _ = tx.execute(migration.sql.clone()).await?; - - // language=SQLite - let _ = query(AssertSqlSafe(format!( - r#"DELETE FROM {table_name} WHERE version = ?1"# - ))) - .bind(migration.version) - .execute(&mut *tx) - .await?; - - tx.commit().await?; + if migration.no_tx { + execute_migration(self, table_name, migration).await?; + } else { + // Use a single transaction for the actual migration script and the essential bookkeeping so we never + // execute migrations twice. See https://github.com/launchbadge/sqlx/issues/1966. + let mut tx = self.begin().await?; + revert_migration(&mut tx, table_name, migration).await?; + tx.commit().await?; + } let elapsed = start.elapsed(); @@ -241,3 +222,53 @@ CREATE TABLE IF NOT EXISTS {table_name} ( }) } } + +async fn execute_migration( + conn: &mut SqliteConnection, + table_name: &str, + migration: &Migration, +) -> Result<(), MigrateError> { + let _ = conn + .execute(migration.sql.clone()) + .await + .map_err(|e| MigrateError::ExecuteMigration(e, migration.version))?; + + // language=SQLite + let _ = query(AssertSqlSafe(format!( + r#" + INSERT INTO {table_name} ( version, description, success, checksum, execution_time ) + VALUES ( ?1, ?2, TRUE, ?3, -1 ) + "# + ))) + .bind(migration.version) + .bind(&*migration.description) + .bind(&*migration.checksum) + .execute(conn) + .await?; + + Ok(()) +} + +async fn revert_migration( + conn: &mut SqliteConnection, + table_name: &str, + migration: &Migration, +) -> Result<(), MigrateError> { + let _ = conn + .execute(migration.sql.clone()) + .await + .map_err(|e| MigrateError::ExecuteMigration(e, migration.version))?; + + // language=SQLite + let _ = query(AssertSqlSafe(format!( + r#" + DELETE FROM {table_name} + WHERE version = ?1 + "# + ))) + .bind(migration.version) + .execute(conn) + .await?; + + Ok(()) +} diff --git a/tests/sqlite/migrate.rs b/tests/sqlite/migrate.rs index 19e8690f9..a2315af28 100644 --- a/tests/sqlite/migrate.rs +++ b/tests/sqlite/migrate.rs @@ -66,6 +66,17 @@ async fn reversible(mut conn: PoolConnection) -> anyhow::Result<()> { Ok(()) } +#[sqlx::test(migrations = false)] +async fn no_tx(mut conn: PoolConnection) -> anyhow::Result<()> { + clean_up(&mut conn).await?; + let migrator = Migrator::new(Path::new("tests/sqlite/migrations_no_tx")).await?; + + // run migration + migrator.run(&mut conn).await?; + + Ok(()) +} + /// Ensure that we have a clean initial state. async fn clean_up(conn: &mut SqliteConnection) -> anyhow::Result<()> { conn.execute("DROP TABLE migrations_simple_test").await.ok(); diff --git a/tests/sqlite/migrations_no_tx/0_vacuum.sql b/tests/sqlite/migrations_no_tx/0_vacuum.sql new file mode 100644 index 000000000..cd42df41f --- /dev/null +++ b/tests/sqlite/migrations_no_tx/0_vacuum.sql @@ -0,0 +1,3 @@ +-- no-transaction + +VACUUM;