commandbox-modules / commandbox-migrations

MIT License
7 stars 9 forks source link

When a migration containing multiple tables fails all tables... #5

Closed jclausen closed 4 years ago

jclausen commented 6 years ago

..created up to the point of failure are orphaned

This is because the migration entry does not exist.

Since the down() function may use dropIfExists, I would suggest storing an entry for the migration with a completion boolean which marks it as incomplete, in order for the down() function to be able to clean up failed migrations in development. Upon migration completion the boolean flag is set to true.

elpete commented 6 years ago

Is this MySQL? If so, the issue is that DDL statements are not transactionable. There’s not really a good workaround for this. Either we create the migration entry and it is incomplete or we don’t create it and we have orphan tables. Running down doesn’t help if the migration is using any methods that can fail (like alter). I personally use the migrate fresh command in this case to reset the database in development.

I should not that other grammars may support transactions with DDL statements.

elpete commented 6 years ago

The only thing I can think of with your suggestion is that we’d be able to print out some message about being in an inconsistent state when running the migrations up again.

jclausen commented 6 years ago

In development with a new database migrate fresh might be the way to go, but if you've got a migration on a existing database that you're writing and it fails, then you're in trouble.

The down() command might not cover all cases, but having the ability to run migrate down --once, which would trigger the most recent migration in the database, could be really invaluable. This might be even more so the case, when looking at a production migration that fails, for whatever reason.

The only way I can see to make that work is pre-inserting the migration record with an incomplete flag ( which you could also use to denote an inconsistent state ).

elpete commented 6 years ago

migrate fresh is only to be used in development against a local database. You wouldn't actually run it against a production database.

I'm not sure I follow what running migrate down --once would do if we had an incomplete record? The state still could be 1/2 migrated or 3/4 migrated or whatever.

jclausen commented 6 years ago

migrate fresh is only to be used in development against a local database

Right

I'm not sure I follow what running migrate down --once would do if we had an incomplete record?

If the down function was written write, and an entry was in the database, then the migration attempt could be reverted.

public function down()
{
    if (schema.hasColumn('users', 'FK_avatar'))
    {
        schema.table('users', function( table )
        {
             table.dropForeign(['FK_avatar']);
             table.dropColumn('FK_avatar');
        });
    }

}
elpete commented 4 years ago

Coming back to this one.

migrate down --once exists. migrate fresh will work for all grammars as of qb 7.0.0 Mistakes in production migrations should be fixed with a new migration.