[5.5] Use transaction in migrations using Sql Server

[lucassch]

No description provided.

[sisve]

1. What functionality does this add to the migration system?
2. With which versions of SQL Server have you tested this change?

[lucassch]

1. This makes migration run in a transaction.

• The follow migration will fail because the table wrongUserTable doesnt exists:

Schema::table('packinglists', function (Blueprint $table) {
    $table->integer('user_id')->nullable();
    $table->foreign('user_id')->references('id')
               ->on('wrongUserTable')->onDelete('no action');
 });

• After that, i'll notice that the column user_id has been created, even though the migration failed.
• So i have to delete the column manually and run the migration again, after adjust the name of table.
• Changing transactions to true, the migration is performed in a transaction, and when the migration fail, a rollback runs, no need to remove the field manually.

2. SQL Server 2012.

[sisve]

Is this supported on older SQL Server versions? Does this apply to all DDL statements that the migration system can execute, or only selective to some of them?

[lucassch]

Is this supported on older SQL Server versions? Does this apply to all DDL statements that the migration system can execute, or only selective to some of them?

• This apply to all statements.
• I have tried several commands manually using BEGIN TRANSACTION and ROLLBACK TRANSACTION in a SQL Server 2008 database and it seems to work.
• SQL Server 2005 was not supported by Microsoft anymore.
• Docs: https://docs.microsoft.com/en-us/sql/t-sql/language-elements/rollback-transaction-transact-sql

[laurencei]

Why would you want to wrap your migration in a transaction?

If it fails (in development) - you just use migrate:fresh and start over again?

[imbrish]

@laurencei not speaking for everyone, but I have nothing against making my life as developer easier :) Sometimes you don't what to eradicate whole database only because minor migration failed.

[lucassch]

Why would you want to wrap your migration in a transaction?
If it fails (in development) - you just use migrate:fresh and start over again?

Because it:

Sometimes you don't what to eradicate whole database only because minor migration failed.

Why not? The PostgresGrammar works at this way. For me it does not make sense every time a migration fails to have to go into the database and drop a table manually.

[sisve]

@laurencei A refresh will not help if the problem is a badly written migration that crashes. A scenario would be when developing new migrations and you've messed up a column name somewhere and the migrations fails when trying it out.

I do not see any downsides with this change; a failing migration will leave the database in the original state instead of a half-way processed migration that you have to clean-up by hand.

[laurencei]

I said migrate:fresh, not migrate:refresh. This is *exactly* why migrate:fresh exists, and solves this problem. Anyway - for everyone who supports it (I don’t); I don’t understand why this PR is on for SQL Server then. It should be for all drivers, or none. Having transactions on one driver only seems strange...

[sisve]

Both migrate:refresh and migrate:fresh runs all migrations. If the latest one you're developing is having issues it would still crash, every time, and leave the database in a weird state where you have to clean it manually.

This change does technically introduce the transactions capability to another driver, so we now have two drivers with transactional support. ;)

Regarding implementing other drivers;

Most of these statements also cause an implicit commit after executing. The intent is to handle each such statement in its own special transaction because it cannot be rolled back anyway.
[...]

• Data definition language (DDL) statements that define or modify database objects. ALTER DATABASE ...

Source: https://dev.mysql.com/doc/refman/5.7/en/implicit-commit.html

I've not looked into what sqlite supports. I usually just have to look at the mysql implementation to find something weird enough for my arguments. ;)

[paulofreitas]

Good catch, I also don't see any downsides with this - in fact, would be very useful. 👍

SQL Server foreign keys are being created in a separated ALTER TABLE statement, so without transactions the CREATE TABLE would always get executed although ALTER TABLE fails.

Unfortunately although transactions are supported in MySQL, those DDL statements use implicit atomic commits as @sisve pointed out - so it would be useless to use in this schema grammar. Same for MariaDB.

It seems to also be useful when using SQLite - although SQLite foreign keys are created inline, it could be used when creating tables with indexes or changing multiple columns. 😉

[elynnaie]

I know this this has already been merged and closed, but I just wanted to add my 2 cents to the above discussion to let you know about some alternate use cases.

Why would you want to wrap your migration in a transaction?

If it fails (in development) - you just use migrate:fresh and start over again?

Not everyone is building microservices and fresh, small apps. migrate:fresh (or refresh) is not a valid option for my team. We have been developing in Laravel since version 3 and have several years of migrations. On top of that, our development database is necessarily >50GB, so dropping everything and reloading takes many minutes on an SSD and fast CPU...not an option when we are creating 10+ migrations a week. Plus, every time a new version of Laravel is released, we would have had to go back all the way to the first migration and update the code to work with new versions of Laravel. With almost 3000 migrations and counting, this becomes utterly impossible to manage. Transactions in migrations really are the only way to go.

Unfortunately for us, sometimes we have to do things in migrations that we cannot do inside a transaction, so the above solution wouldn't work for us (though we don't use SQL server so this PR does not directly affect us). Instead, we modified the migration stub to the following to give ourselves flexibility:

    /** 
     * Run the migrations.
     *
     * @return void
     */
    public function up()
    {   
        DB::transaction(function () {
            // Code goes here
        });
    }   
 
    /** 
     * Reverse the migrations.
     *
     * @return void
     */
    public function down()
    {   
        DB::transaction(function () {
            // Code goes here
        });
    }

This has worked great for us for years. If an exception is thrown (or a dd() statement in the migration for debugging particularly complicated logic), no changes are applied and we can simply fix the problem and run migrate to try again.

Edit: Thought of another use case for migrations where a (re)fresh won't work: Production. If I write a migration that works on my local but for some reason breaks on production, I would prefer that the migration be cleanly rolled back so that it is all-or-nothing. I don't want a half-run migration to have to sort out. It's easy enough to roll back the other migrations in the batch and reset the code until the migration can be debugged.