diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 01d2689900..e2a1802307 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -344,6 +344,7 @@ jobs: - run: cargo test --test '*' --features tests-features,sqlx-sqlite,runtime-${{ matrix.runtime }} - run: cargo test --manifest-path sea-orm-migration/Cargo.toml --test '*' --features sqlx-sqlite,runtime-${{ matrix.runtime }} --no-run - run: cargo test --manifest-path sea-orm-migration/Cargo.toml --test '*' --features sqlx-sqlite,runtime-${{ matrix.runtime }} + - run: cargo test --manifest-path sea-orm-migration/Cargo.toml --test '*' --features sqlx-sqlite,runtime-${{ matrix.runtime }},feat-sqlite-transactional-migrations rusqlite: name: rusqlite diff --git a/CHANGELOG.md b/CHANGELOG.md index 2694d67f67..8e3854e882 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Support invoking `sea-orm-cli` as a Cargo subcommand (`cargo sea generate entity`, `cargo sea migrate`) https://github.com/SeaQL/sea-orm/pull/3187 +- Add the opt-in `feat-sqlite-transactional-migrations` feature to `sea-orm-migration` to run SQLite migrations in transactions by default + ### Enhancements - Use streaming collection by default for `.all()`, pagination, and cursor queries on SQLx backends https://github.com/SeaQL/sea-orm/pull/3215 diff --git a/sea-orm-migration/Cargo.toml b/sea-orm-migration/Cargo.toml index 6d7d2cc3a8..5059b2164f 100644 --- a/sea-orm-migration/Cargo.toml +++ b/sea-orm-migration/Cargo.toml @@ -45,6 +45,7 @@ tokio = { version = "1", features = ["rt-multi-thread", "macros"] } cli = ["clap", "dotenvy", "sea-orm-cli/cli"] default = ["cli"] entity-registry = ["sea-orm/entity-registry"] +feat-sqlite-transactional-migrations = [] runtime-async-std = [ "sea-orm/runtime-async-std", "sea-schema/runtime-async-std", diff --git a/sea-orm-migration/src/lib.rs b/sea-orm-migration/src/lib.rs index 4834393c97..5f4dfa1193 100644 --- a/sea-orm-migration/src/lib.rs +++ b/sea-orm-migration/src/lib.rs @@ -34,7 +34,8 @@ pub trait MigrationTrait: MigrationName + Send + Sync { /// Control whether this migration runs inside a transaction. /// - /// - `None` (default): follow backend convention (Postgres = transaction, MySQL/SQLite = no transaction) + /// - `None` (default): use a transaction on Postgres, and on SQLite when + /// `feat-sqlite-transactional-migrations` is enabled /// - `Some(true)`: force wrapping in a transaction on any backend /// - `Some(false)`: disable automatic transaction wrapping (use `manager.begin()` for manual control) fn use_transaction(&self) -> Option { diff --git a/sea-orm-migration/src/migrator/exec.rs b/sea-orm-migration/src/migrator/exec.rs index a02753d04d..ffa7816481 100644 --- a/sea-orm-migration/src/migrator/exec.rs +++ b/sea-orm-migration/src/migrator/exec.rs @@ -200,7 +200,11 @@ async fn drop_everything_impl(db: &C) -> Result<(), DbErr> { fn should_use_transaction(migration: &dyn crate::MigrationTrait, backend: DbBackend) -> bool { match migration.use_transaction() { Some(v) => v, - None => backend == DbBackend::Postgres, + None => match backend { + DbBackend::Postgres => true, + DbBackend::Sqlite => cfg!(feature = "feat-sqlite-transactional-migrations"), + _ => false, + }, } } diff --git a/sea-orm-migration/tests/common/migration/m20250101_000001_create_test_table.rs b/sea-orm-migration/tests/common/migration/m20250101_000001_create_test_table.rs index 6e6085d645..24bce1d9e1 100644 --- a/sea-orm-migration/tests/common/migration/m20250101_000001_create_test_table.rs +++ b/sea-orm-migration/tests/common/migration/m20250101_000001_create_test_table.rs @@ -1,9 +1,9 @@ use sea_orm_migration::prelude::*; use sea_orm_migration::schema::*; -use sea_orm_migration::sea_orm::DbBackend; pub struct Migration { pub use_transaction: Option, + pub expect_transaction: bool, pub should_fail: bool, } @@ -20,13 +20,10 @@ impl MigrationTrait for Migration { } async fn up(&self, manager: &SchemaManager) -> Result<(), DbErr> { - let expect_txn = self - .use_transaction - .unwrap_or(manager.get_database_backend() == DbBackend::Postgres); assert_eq!( manager.get_connection().is_transaction(), - expect_txn, - "up: expected is_transaction() = {expect_txn}" + self.expect_transaction, + "up: unexpected transaction state" ); manager @@ -47,13 +44,10 @@ impl MigrationTrait for Migration { } async fn down(&self, manager: &SchemaManager) -> Result<(), DbErr> { - let expect_txn = self - .use_transaction - .unwrap_or(manager.get_database_backend() == DbBackend::Postgres); assert_eq!( manager.get_connection().is_transaction(), - expect_txn, - "down: expected is_transaction() = {expect_txn}" + self.expect_transaction, + "down: unexpected transaction state" ); manager diff --git a/sea-orm-migration/tests/common/migrator/transaction_test.rs b/sea-orm-migration/tests/common/migrator/transaction_test.rs index 68727df907..dfeacc44e0 100644 --- a/sea-orm-migration/tests/common/migrator/transaction_test.rs +++ b/sea-orm-migration/tests/common/migrator/transaction_test.rs @@ -3,6 +3,7 @@ use sea_orm_migration::{MigratorTraitSelf, prelude::*}; pub struct Migrator { pub use_transaction: Option, + pub expect_transaction: bool, pub should_fail: bool, } @@ -11,6 +12,7 @@ impl MigratorTraitSelf for Migrator { fn migrations(&self) -> Vec> { vec![Box::new(m20250101_000001_create_test_table::Migration { use_transaction: self.use_transaction, + expect_transaction: self.expect_transaction, should_fail: self.should_fail, })] } diff --git a/sea-orm-migration/tests/main.rs b/sea-orm-migration/tests/main.rs index 50ec7b693e..d92faa1118 100644 --- a/sea-orm-migration/tests/main.rs +++ b/sea-orm-migration/tests/main.rs @@ -241,10 +241,16 @@ where assert!(!manager.has_table("cake").await?); assert!(!manager.has_table("fruit").await?); - // Tests rolling back a failing migration on Postgres. + let uses_transactions_by_default = match db.get_database_backend() { + DbBackend::Postgres => true, + DbBackend::Sqlite => cfg!(feature = "feat-sqlite-transactional-migrations"), + _ => false, + }; + + // Tests rolling back a failing migration in a transaction. // With per-migration transactions, only the failing migration is rolled back; // earlier migrations that committed successfully are preserved. - if matches!(db.get_database_backend(), DbBackend::Postgres) { + if uses_transactions_by_default { println!("\nRoll back changes when encounter errors"); // Set a flag to throw error inside `m20230109_000001_seed_cake_table.rs` @@ -269,6 +275,8 @@ where assert!(manager.has_table("cake").await?); assert!(manager.has_table("fruit").await?); + assert_eq!(migrator.get_applied_migrations(db).await?.len(), 5); + // Unset the flag unsafe { std::env::remove_var("ABORT_MIGRATION"); @@ -298,12 +306,12 @@ where assert!(manager.has_column("cake", "name").await?); assert!(manager.has_column("fruit", "cake_id").await?); - // Tests rolling back a failing migration-down on Postgres. + // Tests rolling back a failing down migration in a transaction. // With per-migration transactions, rollbacks happen one at a time in reverse. // Migrations 6-2 roll back and commit successfully. Migration 1 (drops cake // then ABORTs) fails, so its DROP is restored. But migration 2's DROP of // the fruit table already committed. - if matches!(db.get_database_backend(), DbBackend::Postgres) { + if uses_transactions_by_default { println!("\nRoll back changes when encounter errors"); // Set a flag to throw error inside `m20220118_000001_create_cake_table.rs` @@ -328,6 +336,8 @@ where assert!(manager.has_table("cake").await?); assert!(!manager.has_table("fruit").await?); + assert_eq!(migrator.get_applied_migrations(db).await?.len(), 1); + // Unset the flag unsafe { std::env::remove_var("ABORT_MIGRATION"); @@ -374,11 +384,16 @@ async fn run_transaction_test(url: &str, db_name: &str, schema: &str) -> Result< let backend = db.get_database_backend(); let manager = SchemaManager::new(db); - // use_transaction = None: Postgres wraps by default, others don't. + // use_transaction = None: follows the backend default. // The assertion happens inside the migration's up()/down() body. println!("\nTransaction test: use_transaction = None"); let m = transaction_test::Migrator { use_transaction: None, + expect_transaction: match backend { + DbBackend::Postgres => true, + DbBackend::Sqlite => cfg!(feature = "feat-sqlite-transactional-migrations"), + _ => false, + }, should_fail: false, }; m.up(db, None).await?; @@ -391,6 +406,7 @@ async fn run_transaction_test(url: &str, db_name: &str, schema: &str) -> Result< println!("\nTransaction test: use_transaction = Some(true)"); let m = transaction_test::Migrator { use_transaction: Some(true), + expect_transaction: true, should_fail: false, }; m.up(db, None).await?; @@ -403,6 +419,7 @@ async fn run_transaction_test(url: &str, db_name: &str, schema: &str) -> Result< println!("\nTransaction test: use_transaction = Some(false)"); let m = transaction_test::Migrator { use_transaction: Some(false), + expect_transaction: false, should_fail: false, }; m.up(db, None).await?; @@ -415,6 +432,7 @@ async fn run_transaction_test(url: &str, db_name: &str, schema: &str) -> Result< println!("\nTransaction test: failure with transaction"); let m = transaction_test::Migrator { use_transaction: Some(true), + expect_transaction: true, should_fail: true, }; assert!(m.up(db, None).await.is_err()); @@ -430,6 +448,7 @@ async fn run_transaction_test(url: &str, db_name: &str, schema: &str) -> Result< println!("\nTransaction test: failure without transaction"); let m = transaction_test::Migrator { use_transaction: Some(false), + expect_transaction: false, should_fail: true, }; assert!(m.up(db, None).await.is_err());