Incompatible set_grouped with no transaction

Author: deven96

README.md

@@ -96,7 +96,7 @@ This would stop developer 1's migration from ever running if you were using cont
 
 refinery works by creating a table that keeps all the applied migrations' versions and their metadata. When you [run](https://docs.rs/refinery/latest/refinery/struct.Runner.html#method.run) the migrations `Runner`, refinery compares the applied migrations with the ones to be applied, checking for [divergent](https://docs.rs/refinery/latest/refinery/struct.Runner.html#method.set_abort_divergent) and [missing](https://docs.rs/refinery/latest/refinery/struct.Runner.html#method.set_abort_missing) and executing unapplied migrations.\
 By default, refinery runs each migration in a single transaction. Alternatively, you can also configure refinery to wrap the entire execution of all migrations in a single transaction by setting [set_grouped](https://docs.rs/refinery/latest/refinery/struct.Runner.html#method.set_grouped) to true.
-Directive `-- +refinery NO TRANSACTION` can be used to escape running a migration within a transaction. [!IMPORTANT]: `set_grouped` takes precedence over no transaction directive.
+Directive `-- +refinery NO TRANSACTION` can be used to escape running a migration within a transaction. [!IMPORTANT]: `set_grouped` is incompatible with the no transaction directive.
 The rust crate intentionally ignores new migration files until your sourcecode is rebuild. This prevents accidental migrations and altering the database schema without any code changes. We can also bake the migrations into the binary, so no additional files are needed when deployed.
 
 ### Rollback

refinery/tests/postgres.rs

@@ -384,6 +384,37 @@ mod postgres {
         });
     }
 
+    #[test]
+    fn no_transaction_fails_in_set_grouped() {
+        run_test(|| {
+            let mut client = Client::connect(&db_uri(), NoTls).unwrap();
+
+            embedded::migrations::runner().run(&mut client).unwrap();
+
+            let migrations = get_migrations();
+
+            let mchecksum = migrations[5].checksum();
+            let err = client
+                .migrate(
+                    &migrations,
+                    true,
+                    true,
+                    true,
+                    Target::Latest,
+                    DEFAULT_TABLE_NAME,
+                )
+                .unwrap_err();
+
+            match err.kind() {
+                Kind::NoTransactionGroupedMigration(last) => {
+                    assert_eq!(6, last.version());
+                    assert_eq!(mchecksum, last.checksum());
+                }
+                _ => panic!("failed test"),
+            }
+        });
+    }
+
     #[test]
     fn migrates_to_target_migration() {
         run_test(|| {

refinery_core/src/error.rs

@@ -57,6 +57,9 @@ pub enum Kind {
     /// An Error from an divergent version, the applied version is different to the filesystem one
     #[error("applied migration {0} is different than filesystem one {1}")]
     DivergentVersion(Migration, Migration),
+    /// An Error from running in grouped mode with a migration that opts out of transactions
+    #[error("migration {0} opts out of transactions, cannot run with `set-grouped`")]
+    NoTransactionGroupedMigration(Migration),
     /// An Error from an divergent version, the applied version is missing on the filesystem
     #[error("migration {0} is missing from the filesystem")]
     MissingVersion(Migration),

refinery_core/src/traits/async.rs

@@ -1,4 +1,4 @@
-use crate::error::WrapMigrationError;
+use crate::error::{Kind, WrapMigrationError};
 use crate::traits::{
     insert_migration_query, verify_migrations, ASSERT_MIGRATIONS_TABLE_QUERY,
     GET_APPLIED_MIGRATIONS_QUERY, GET_LAST_APPLIED_MIGRATION_QUERY,
@@ -83,6 +83,12 @@ async fn migrate_grouped<T: AsyncExecutor>(
                 break;
             }
         }
+        if !migration.flags().run_in_transaction {
+            return Err(Error::new(
+                Kind::NoTransactionGroupedMigration(migration),
+                None,
+            ));
+        }
 
         migration.set_applied();
         let query = insert_migration_query(&migration, migration_table_name);

refinery_core/src/traits/sync.rs

@@ -1,4 +1,4 @@
-use crate::error::WrapMigrationError;
+use crate::error::{Kind, WrapMigrationError};
 use crate::traits::{
     insert_migration_query, verify_migrations, ASSERT_MIGRATIONS_TABLE_QUERY,
     GET_APPLIED_MIGRATIONS_QUERY, GET_LAST_APPLIED_MIGRATION_QUERY,
@@ -41,6 +41,13 @@ fn migrate_grouped<T: Executor>(
             }
         }
 
+        if !migration.flags().run_in_transaction {
+            return Err(Error::new(
+                Kind::NoTransactionGroupedMigration(migration),
+                None,
+            ));
+        }
+
         migration.set_applied();
         let query = insert_migration_query(&migration, migration_table_name);