Fix exec_with_connection ignoring per-migration use_transaction config

[tyt2y3]

Fixes #3001

• Remove exec_with_connection! macro that unconditionally wrapped all Postgres operations in a transaction
• Inline the into_database_executor() / SchemaManager::new() pattern at each call site (fresh, refresh, reset, uninstall), matching the approach already used by up() and down()
• Move the Postgres transaction for drop_everything into the function itself so atomicity is preserved

[yuriy-yarosh]

Not sure about this.
Only database role/user extension and tablespace ops can not be put into a transaction.

I'm expecting it to tolerate use_transaction() for both apply and rollback ops, disregarding what exact database being used PostgreSQL or MySQL. So, that check is a bit pointless... We may get partial support for transactional DDL in MySQL one day, as well. So it worth respecting, when transaction wrap is explicitly enabled, or explicitly disabled.

[tyt2y3]

Not sure about this. Only database role/user extension and tablespace ops can not be put into a transaction.

I'm expecting it to tolerate use_transaction() for both apply and rollback ops, disregarding what exact database being used PostgreSQL or MySQL. So, that check is a bit pointless... We may get partial support for transactional DDL in MySQL one day, as well. So it worth respecting, when transaction wrap is explicitly enabled, or explicitly disabled.

sorry, I don't quite get the verdict. is the new behaviour in this PR good enough? note that before this change, everything is wrapped in transaction on Postgres which is a bit strange

[tyt2y3]

I think this is a more sensible mechanism, will merge for now

[yuriy-yarosh]

@tyt2y3 yes, it's feasible.
I'd skip DB type check and applied implicit transaction wrap, only when migration itself has not declared explicit use_transaction() block. But, I presume, this would require parsing and custom proc-macro use, which might be an overkill.

Right now I'm splitting CLI's into multiple admin accounts and run them separately, to fix these types of issues.

const MIGRATION_PHASE_ENV: &str = "APP_MIGRATION_PHASE";

pub fn set_database_url(url: &str) {
    unsafe {
        env::set_var("DATABASE_URL", url);
    }
}

async fn run_bootstrap() {
    let url = migration::conn::bootstrap_connection_url();
    set_database_url(&url);

    cli::run_cli_with_connection(BootstrapMigrator, |_| async move { Database::connect(url).await }).await;
}

async fn run_app() {
    let url = migration::conn::admin_connection_url();
    set_database_url(&url);

    cli::run_cli_with_connection(AppMigrator, |_| async move { Database::connect(url).await }).await;
}

fn run_phase_subprocess(phase: &str) -> Result<(), Box<dyn std::error::Error + 'static>> {
    let exe = env::current_exe()?;
    let args: Vec<_> = env::args_os().skip(1).collect();

    let status = Command::new(exe).args(args).env(MIGRATION_PHASE_ENV, phase).status()?;

    if status.success() {
        Ok(())
    } else {
        Err(format!("migration phase '{phase}' failed with status {status}").into())
    }
}

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error + 'static>> {
    match env::var(MIGRATION_PHASE_ENV).ok().as_deref() {
        Some("bootstrap") => {
            run_bootstrap().await;
            return Ok(());
        }
        Some("app") => {
            run_app().await;
            return Ok(());
        }
        _ => {}
    }

    if conn::bootstrap_required().await {
        run_phase_subprocess("bootstrap")?;
        if !conn::bootstrap_required().await {
            run_phase_subprocess("app")?;
        }
    }

    if !conn::bootstrap_required().await {
        run_phase_subprocess("app")?;
        if conn::applied_migrations_count().await == 0 {
            run_phase_subprocess("bootstrap")?;
        }
    }
    
    Ok(())
}

A bit hacky way, but I didn't figure out anything better atm.