[Idea] Wrapping migrations in transactions?

[conradkleinespel]

It seems like at the moment, migrations aren't wrapped in transactions. This means if you try to run a migration that breaks half way through because you forgot some foreign key or whatever, you end up with half the database updated. Which is kind of a pain.

I can't think of a reason why this wouldn't be merged, but since it isn't in the core, I'm thinking there may actually be a reason. Could anyone tell me more about this please?

It would end the need for this kind of boilerplate code in migration files:

try {
    DB::getPdo()->beginTransaction();
    Schema::create('users', function($table) {

    });

    // some more queries or whatever
} catch (PDOException $e) {
    DB::getPdo()->rollBack();
    throw $e;
}

[bencorlett]

We totally should. And the database has a transaction method which I use a lot. Seems like an easy pull request.

FYI, usage is:

DB::transaction(function($connection)
{
    $connection->table('foo')->where('bar', 'baz'); // etc
});

[conradkleinespel]

Oh, I wasn't aware of that helper, thanks :-)

[taylorotwell]

Unfortunately, wrapping migrations in a transaction doesn't really solve this problem. Some operations can't be rolled back, even from a transaction, such as a table creation, etc. So you can still end up in a broken state, which stinks. If anyone has any alternative ideas, let me know.

[bencorlett]

(from the migrator itself) could we wrap it in a transaction, catch ANY exceptions and then run the down() method on any exception? In the down method we're going to delete our created tables

On 13/02/2013, at 10:06 AM, Taylor Otwell notifications@github.com wrote:

Unfortunately, wrapping migrations in a transaction doesn't really solve this problem. Some operations can't be rolled back, even from a transaction, such as a table creation, etc. So you can still end up in a broken state, which stinks. If anyone has any alternative ideas, let me know.

—
Reply to this email directly or view it on GitHub.

[robclancy]

I have run into issues like this a fair bit. What @bencorlett said should work? Or another alternative is wrapping in the transaction for what supports them and what doesn't manually log what is done and reverse it manually after doing the normal db transaction rollback?

[conradkleinespel]

👍 @Robbo- & @bencorlett

[JoostK]

Calling down on failure may work if it only contains one statement. For multiple statements it may fail because you have no idea in what state the DB actually is.

[taylorotwell]

Yeah, there is still no really solid solution to this. My best advice is to do a single operation per migration so that your migrations stay very granular.

[erindru]

FYI (Hope this doesnt repoen the issue but im posting this here because others may find it useful), PostgreSQL supports Transactional DDL statements - http://wiki.postgresql.org/wiki/Transactional_DDL_in_PostgreSQL:_A_Competitive_Analysis

[HonzaMac]

What about to enable transaction just for DB layers, which support REAL transactions? included DDL? @taylorotwell @conradkleinespel

[conradkleinespel]

@HonzaMac Hey ! I haven't been working with Laravel for some time, so I'm afraid I won't have any valuable input on migration atomicity. I do hope this will get fixed eventually though. @taylorotwell 's suggestion on making migrations more granular seems like a good workaround though.

[gacelita]

Disclaimer: I very possibly do not know what I'm talking about

While it is true that some operations can't be rolled back (thus making transactions alone not universally useful), this could be solved if migrations were made by defining 2-tuples representing an "up" operation and its corresponding "down" operation. This would allow us to roll back whatever did not fail before the failed "up" operation.

Example transformation:

public function up() {
	Schema::table('example', function($table) {
		$table->increments('id');
		$table->string('value');
	});
	Schema::table('second_example', function($table) {
		throw new Exception("Oops!");
	});
}
public function down() {
	Schema::drop('second_example');
	Schema::drop('example');
}

This will always go bad because we can't run either methods afterwards (down expects both tables to exist, and up expects both tables to not exist), nor can we use transactions to avoid it.

My proposed solution:

public function run() {
	Schema::submigration('example', function() {
		Schema::table('example', function($table) {
			$table->increments('id');
			$table->string('value');
		});
	}, function() {
		Schema::drop('example');
	});

	Schema::submigration('secondExample', function() {
		Schema::table('second_example', function($table) {
			throw new Exception("Oops!");
		});
	}, function() {
		Schema::drop('second_example');
	});
}

First closure is up, second closure is down. Now we can roll back the first submigration after the second submigration fails, because the down closure specifies how to do so.

[simonhamp]

@JoelSanchez interesting approach... but if you're going to go to the trouble of writing two separate functions for two submigrations why not simply create two migrations and use the native up() and down() methods for each?

Also, this doesn't really solve the problem in an elegant way. Even when only one table is being acted on (one submigration), if something fails, you will end up possibly writing two down statements: one in case something fails and one as an actual migration rollback. That's not ideal IMO.

Ideally, the migrator keeps track of the statements that are run and ONLY rolls those back if the migration doesn't complete.

The trick is knowing what 'rolling back' means for each command - it may not simply be the apparent opposite of the command being used; the migrator needs to know the previous schema state to roll back predictably.

[HonzaMac]

Just imagine, that someone will use raw SQL query and all sub-migrations are not the well designed concept. All normal programmer want is just to write one up, and one down. No IF EXISTS, and no other detections of the current state.
I hope that better solution is to allow DDL transactional migrations just for Postgresql and keep other databases behind.

[simonhamp]

@HonzaMac I don't think that's very likely.

I believe a better migrator is needed that compares current state to desired state and calculates the changes necessary. That could then calculate changes in both directions.

This would change migrations from 'a set of commands that achieve the change you want' to 'the desired structure'.

[crazy4chrissi]

I think for the time being, the main issue is that the documentation does not mention that transactions are not transactional and that using only a single command per migration is therefore advisable.

In my opinion, if the DB does not support this transactional, it is not the job of laravel. But I also think the developer should have the possibility to code it manually by catching exceptions and coding the rollback. I tried this, but it does not work:

    public function up()
    {
        Schema::table('posts', function (Blueprint $table) {
            $table->integer('user_id')->unsigned()->nullable();
            try {
                $table->foreign('user_id')
                    ->references('id')->on('userDoesNotExist')
                    ->onDelete('set null');
            } catch(Exception $e) {
                // first command succeeded, second one did not, so roll back the first one
                $table->dropColumn('user_id');
                // throw an Exception so that the migrator still knows something went wrong
                throw new Exception('Foreign Key creation for user_id failed');
            }
        });
    }

I expected that I could catch the exception that occurs and roll back what I did so far. But the catch gets never called, even though an Illuminate\Database\QueryException (and a PDOException in my case) occur.

[simonhamp]

@crazy4chrissi That's because the Exception isn't thrown at the point where the closure you pass to Schema::table() is executed. That set of commands is used to bundle an entire blueprint together which can be executed in a single logical command.

Perhaps try wrapping your entire Schema::table() in a try-catch block:

try {
  Schema::table('posts', function(Blueprint $table){ ... });
}
catch(Exception $e) {
}

[HonzaMac]

I found, that in version 5.3 and 5.4 from commit 7a35d72 are transactions for databases with transactional DDL supported (PostgresSQL)! Thanks @ameliaikeda for that!

[TomHAnderson]

I've created a coding pattern to roll back failed migrations: https://blog.tomhanderson.com/2021/09/transactional-laravel-migrations.html