Script-Migration command should begin/commit transactions

[Zysce]

Script-Migration command currently generate creation (or drop) script without checking objects existence.
If an error happens during script deployment (network, server error, ...), you can't redeploy generated script.

A check is done for table __EFMigrationsHistory :

IF OBJECT_ID(N'__EFMigrationsHistory') IS NULL
BEGIN
    CREATE TABLE [__EFMigrationsHistory] (
        [MigrationId] nvarchar(150) NOT NULL,
        [ProductVersion] nvarchar(32) NOT NULL,
        CONSTRAINT [PK___EFMigrationsHistory] PRIMARY KEY ([MigrationId])
    );
END;

GO

It should be the same for user tables, ex of generated creation script :

IF NOT EXISTS(SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'20170217083056_EFEntityInitialCreate')
BEGIN
    CREATE TABLE [SomeTable] (
        [Id] int NOT NULL IDENTITY,
        [Name] nvarchar(max) NOT NULL,
        CONSTRAINT [PK_SomeTable] PRIMARY KEY ([Id])
    );
END;

GO

The modification should be done in creation script (check if not exists) and drop script (check if exists).

[ErikEJ]

Did you use the

-Idempotent 

command swith?

https://docs.microsoft.com/en-us/ef/core/miscellaneous/cli/powershell

[Zysce]

Yes,
IdemPotent check if __EFMigrationsHistory table contains migration row.
IF NOT EXISTS(SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'20170217083056_EFSomeTableInitialCreate')

However, the row with MigrationId = '20170217083056_EFSomeTableInitialCreate' is added at the end of the script.

If an error occurs on script deployment before the row is inserted, you cannot redeploy the script since the table __EFMigrationsHistory does not contains migration row.

[rowanmiller]

When we execute migrations from tools/code, we use transactions for most operations (though there are some DDL operations that can't be executed in a transaction). @bricelam should we be emitting BEGIN TRANSACTION etc. in the script we generate?

[bricelam]

@rowanmiller I think it depends on whether most tools (e.g. MSDeploy, SSMS, SSDT, etc.) give you the option to run it inside a transaction...

[ajcvickers]

We discussed this at length and have decided that at this time we will not start generating any transactions code in the script. We want to leave the scripts generated to be useful in a variety of scenarios, some of which don't involve use of transactions, and some of which will do transactions themselves separately from the script. We considered emitting a comment saying this at the start of the script, but for now we don't think there is enough value in doing that. We will re-consider this if there is more feedback and/or specific pit-of-failure scenarios that we see people falling into.

[bricelam]

Based on feedback, we're re-considering whether the script should include the BEGIN TRANSACTION and COMMIT statements corresponding to where transactions occur during Update-Database.

[jonsagara]

Hi Brice,

I see this is attached to milestone 3.0.0. Will this definitely make it into EF Core 3.0?

Thanks,

Jon

[bricelam]

@jonsagara Our 3.0 planning happend before I took on porting EF6 to .NET Core. I need to do another pass of punting my 3.0 issues. That said, I still hope I can get to this one during the 3.0 milestone.

[bricelam]

I recently discovered that this is important for suppressTransaction: true. Without the BEGIN and END statements, you don't know which parts of the script are supposed to run outside of a transaction. Today, Web Publish wraps the whole script inside a transaction causing these operations to fail.

[ChristopherHaws]

@esquino I honestly don't understand why, but it looks like evaluating XACT_STATE() with an AND in the IF statement doesn't work, however it does work if it is the only thing evaluated in the if statement or if you set it as a variable and evaluate that. I modified my original post's code so that it outputs like this now and it does appear to be working:

DECLARE @XACT_STATE SMALLINT = XACT_STATE();
IF @XACT_STATE = 1 AND NOT EXISTS(SELECT * FROM [__EFMigrationsHistory] WHERE [MigrationId] = N'20200409230613_Second')
BEGIN
	-- Migrate Data
END;

[bricelam]

I looked into MSDeploy (used by Web Publish) and it's safe to include the BEGIN/COMMIT statements in the script. I was worried we might run into nested transaction errors because it automatically wraps the entire script inside a transaction, but it simply ignores the statements. I filed dotnet/sdk#12676 to, once this is implemented, disable the automatic transaction and start using the script's transactions instead.

[bricelam]

Design meeting notes:

• Start doing this by default
• Add a --no-transactions option to revert to the previous behavior in the off chance that it breaks existing workflows

[woeterman94]

Any update on this?

[bricelam]

@woeterman94 Will be available in 5.0.0-rc.1

[ErroneousFatality]

@bricelam I have an issue with this.
I'm using YugabyteDB (through the npgsql framework), which doesn't support DDL code inside transactions.
I'm using the DbContext.Database.MigrateAsync method in code to run my migrations through my application for migrating and seeding our databases.
There's no parameter to tell the Migrate method to not use transactions.

I opened a feature request issue about it here.