Repository: diesel-rs/diesel
File: diesel_cli/src/migrations/mod.rs (Rust)

Code before:
) where
    DB: Backend,
    Conn: MigrationHarness<DB> + Connection<Backend = DB> + 'static,
{
    let migrations = MigrationSource::<DB>::migrations(&migrations_dir)
        .unwrap_or_else(handle_error)
        .into_iter()
        .map(|m| (m.name().version().as_owned(), m))
        .collect::<HashMap<_, _>>();
    let applied_migrations = conn.applied_migrations().unwrap_or_else(handle_error);
    let versions_to_revert = if args.get_flag("REDO_ALL") {
        &applied_migrations
    } else {
        let number = args.get_one::<u64>("REDO_NUMBER").unwrap();
        let number = std::cmp::min(*number as usize, applied_migrations.len());
        &applied_migrations[..number]
    };
    let should_use_not_use_transaction = versions_to_revert.iter().any(|v| {
        migrations
            .get(v)
            .map(|m| !m.metadata().run_in_transaction())
            .unwrap_or_default()
    });

    let migrations_inner =
        |harness: &mut HarnessWithOutput<Conn, _>|
         -> Result<(), Box<dyn Error + Send + Sync + 'static>> {
            let reverted_versions = if args.get_flag("REDO_ALL") {
                harness.revert_all_migrations(migrations_dir.clone())?
            } else {
                let number = args.get_one::<u64>("REDO_NUMBER").unwrap();

                (0..*number)
                    .filter_map(|_| {
                        match harness.revert_last_migration(migrations_dir.clone()) {
                            Ok(v) => Some(Ok(v)),
                            Err(e) if e.is::<MigrationError>() => {
                                match e.downcast_ref::<MigrationError>() {
                                    // If n is larger then the actual number of migrations,
                                    // just stop reverting them
                                    Some(MigrationError::NoMigrationRun) => None,
                                    _ => Some(Err(e)),
                                }
                            }
                            Err(e) => Some(Err(e)),
                        }
                    })
                    .collect::<Result<Vec<_>, _>>()?
            };

             let mut migrations = MigrationSource::<DB>::migrations(&migrations_dir)

Review comment (bug):
A single migration having `metadata().run_in_transaction()` as false can cause all the migrations to not be run as a transaction. So if there's a failure in the middle of the redo operation, how are we restoring the DB to a consistent state?

Code after:
    conn: &mut Conn,
    migrations_dir: FileBasedMigrations,
    args: &ArgMatches,
) where
    DB: Backend,
    Conn: MigrationHarness<DB> + Connection<Backend = DB> + 'static,
{
    let migrations = MigrationSource::<DB>::migrations(&migrations_dir)
        .unwrap_or_else(handle_error)
        .into_iter()
        .map(|m| (m.name().version().as_owned(), m))
        .collect::<HashMap<_, _>>();
    let applied_migrations = conn.applied_migrations().unwrap_or_else(handle_error);
    let versions_to_revert = if args.get_flag("REDO_ALL") {
        &applied_migrations
    } else {
        let number = args.get_one::<u64>("REDO_NUMBER").unwrap();
        let number = std::cmp::min(*number as usize, applied_migrations.len());
        &applied_migrations[..number]
    };
    let should_use_not_use_transaction = versions_to_revert.iter().any(|v| {
        migrations
            .get(v)
            .map(|m| !m.metadata().run_in_transaction())
            .unwrap_or_default()
    });

    let migrations_inner =
        |harness: &mut HarnessWithOutput<Conn, _>|
         -> Result<(), Box<dyn Error + Send + Sync + 'static>> {
            // revert all the migrations
            let reverted_versions = if args.get_flag("REDO_ALL") {
                harness.revert_all_migrations(migrations_dir.clone())?
            } else {
                let number = args.get_one::<u64>("REDO_NUMBER").unwrap();
                (0..*number)
                    .filter_map(|_| {
                        match harness.revert_last_migration(migrations_dir.clone()) {
                            Ok(v) => Some(Ok(v)),
                            Err(e) if e.is::<MigrationError>() => {
                                match e.downcast_ref::<MigrationError>() {
                                    // If n is larger then the actual number of migrations,
                                    // just stop reverting them
                                    Some(MigrationError::NoMigrationRun) => None,
                                    _ => Some(Err(e)),
                                }
                            }
                            Err(e) => Some(Err(e)),
                        }
                    })
                    .collect::<Result<Vec<_>, _>>()?