Efcore: SqlServer Migrations: Scripts do not fail on failure

[ChristopherHaws]

One of the new features added in 5.0-rc1 was that transactions are now used in migrations, however if there is a failure in your migration, the transaction does not work properly because of the use of GO batch terminators. The transaction will not rollback changes prior to the error and will continue executing after the error'ed batch and will proceed to commit the successful batches of the failed migration.

In short, the current behaviors of suppressTransaction = true and suppressTransaction = false are pretty much the same. While a transaction is added to the script, it doesn't add any protection and the results of executing the script are the same.

Steps to reproduce

1. Create a new project with the following DbContext setup:
   ``` C#
   // Startup.cs
   services.AddDbContext(x => {
   x.UseSqlServer(@"Server=(localdb)mssqllocaldb; Database=ef; Trusted_Connection=True; MultipleActiveResultSets=true");
   });

public class ApplicationContext : DbContext {
public ApplicationContext(DbContextOptions options) : base(options) {}

public DbSet<Blog> Blogs { get; set; }

protected override void OnModelCreating(ModelBuilder model) {
    model.Entity<Blog>(x => {
        x.Property(x => x.Name).HasMaxLength(10);
        x.HasData(new Blog { BlogId = 1, Name = "Blog 1" });
        x.HasData(new Blog { BlogId = 2, Name = "Blog 2 - Name is too long" });
    });
}

}

public class Blog {
public int BlogId { get; set; }
public string Name { get; set; }
}

2. Create a migration:

dotnet migrations add Initial

```C#
public partial class Initial : Migration {
    protected override void Up(MigrationBuilder migrationBuilder) {
        migrationBuilder.CreateTable(
            name: "Blogs",
            columns: table => new {
                BlogId = table.Column<int>(type: "int", nullable: false)
                    .Annotation("SqlServer:Identity", "1, 1"),
                Name = table.Column<string>(type: "nvarchar(10)", maxLength: 10, nullable: true)
            },
            constraints: table => {
                table.PrimaryKey("PK_Blogs", x => x.BlogId);
            });

        migrationBuilder.InsertData(
            table: "Blogs",
            columns: new[] { "BlogId", "Name" },
            values: new object[] { 1, "Blog 1" });

        migrationBuilder.InsertData(
            table: "Blogs",
            columns: new[] { "BlogId", "Name" },
            values: new object[] { 2, "Blog 2 - Name is too long" });
    }
}

1. Create a migration script:

dotnet ef migrations script -o "migrate.sql"
````
```sql
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

BEGIN TRANSACTION;
GO

CREATE TABLE [Blogs] (
    [BlogId] int NOT NULL IDENTITY,
    [Name] nvarchar(10) NULL,
    CONSTRAINT [PK_Blogs] PRIMARY KEY ([BlogId])
);
GO

IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [name] IN (N'BlogId', N'Name') AND [object_id] = OBJECT_ID(N'[Blogs]'))
    SET IDENTITY_INSERT [Blogs] ON;
INSERT INTO [Blogs] ([BlogId], [Name])
VALUES (1, N'Blog 1');
IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [name] IN (N'BlogId', N'Name') AND [object_id] = OBJECT_ID(N'[Blogs]'))
    SET IDENTITY_INSERT [Blogs] OFF;
GO

IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [name] IN (N'BlogId', N'Name') AND [object_id] = OBJECT_ID(N'[Blogs]'))
    SET IDENTITY_INSERT [Blogs] ON;
INSERT INTO [Blogs] ([BlogId], [Name])
VALUES (2, N'Blog 2 - Name is too long');
IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [name] IN (N'BlogId', N'Name') AND [object_id] = OBJECT_ID(N'[Blogs]'))
    SET IDENTITY_INSERT [Blogs] OFF;
GO

INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion])
VALUES (N'20200919002238_Initial', N'5.0.0-rc.1.20451.13');
GO

COMMIT;
GO

1. Run the migration.sql script in an empty SQL database and notice that you get the following error:

(1 row affected)
Msg 8152, Level 16, State 13, Line 31
String or binary data would be truncated.
The statement has been terminated.

(1 row affected)

Completion time: 2020-09-18T17:29:20.1685190-07:00

The script continued to run even though there was an error and inserted the first blog entry and the migration entry but not the second blog entry.

Possible Solutions

You can see a current working solution for EF Core 3.1 here: https://github.com/dotnet/efcore/issues/7681#issuecomment-602878706

The basic details of that solution are:

1. Execute SET XACT_ABORT ON; at the beginning of the script
2. Only execute the batch IF XACT_STATE() = 1
3. Only commit a transaction IF XACT_STATE() = 1

Something to be aware of, using XACT_STATE() within an IF statement with more than 1 condition causes it to not work properly so you will need to make a variable to capture the state. See: https://github.com/dotnet/efcore/issues/7681#issuecomment-613017864

Following these steps, the migration would look something like this:

SET XACT_ABORT ON;
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

BEGIN TRANSACTION;
GO

IF XACT_STATE() = 1
BEGIN
    CREATE TABLE [Blogs] (
        [BlogId] int NOT NULL IDENTITY,
        [Name] nvarchar(10) NULL,
        CONSTRAINT [PK_Blogs] PRIMARY KEY ([BlogId])
    );
END
GO

IF XACT_STATE() = 1
BEGIN
    IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [name] IN (N'BlogId', N'Name') AND [object_id] = OBJECT_ID(N'[Blogs]'))
        SET IDENTITY_INSERT [Blogs] ON;
    INSERT INTO [Blogs] ([BlogId], [Name])
    VALUES (1, N'Blog 1');
    IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [name] IN (N'BlogId', N'Name') AND [object_id] = OBJECT_ID(N'[Blogs]'))
        SET IDENTITY_INSERT [Blogs] OFF;
END
GO

IF XACT_STATE() = 1
BEGIN
    IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [name] IN (N'BlogId', N'Name') AND [object_id] = OBJECT_ID(N'[Blogs]'))
        SET IDENTITY_INSERT [Blogs] ON;
    INSERT INTO [Blogs] ([BlogId], [Name])
    VALUES (2, N'Blog 2 - Name is too long');
    IF EXISTS (SELECT * FROM [sys].[identity_columns] WHERE [name] IN (N'BlogId', N'Name') AND [object_id] = OBJECT_ID(N'[Blogs]'))
        SET IDENTITY_INSERT [Blogs] OFF;
END
GO

IF XACT_STATE() = 1
BEGIN
    INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion])
    VALUES (N'20200919002238_Initial', N'5.0.0-rc.1.20451.13');
END
GO

IF XACT_STATE() = 1
BEGIN
    COMMIT;
END
GO

Which results in the __EFMigrationsHistory table being created, but it stays empty and all the rest of the script is rolled back.

Further technical details

EF Core version: 5.0.0-rc.1.20451.13
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 5.0
Operating system: Windows 10
IDE: Visual Studio 2019 16.8.0 Preview 3.0

[roji]

/cc @bricelam

@ChristopherHaws thanks for flagging this. Note that if you wrap the COMMIT in an IF XACT_STATE() = 1, for failed transactions we neither commit nor roll back, and so the next migration would create a so-called "nested transaction". This may be mitigated by having an ELSE clause that rolls back the migration's transaction instead of committing when XACT_STATE isn't 1.

I'm also not sure why it would be necessary to wrap batches if XACT_STATE() = 1 (assuming XACT_ABORT is ON) - isn't wrapping the commit enough?

For a cross-database perspective, in PostgreSQL the moment any error occurs the transaction is failed and can only be rolled back. None of the statements (before after after the error) can ever be committed (unless savepoints are used), so this doesn't occur there.

[ChristopherHaws]

@roji I do not believe that it creates nested transactions. I tested this theory with this script and found that before each BEGIN TRANSACTION the current transaction count was 0, even though none of the COMMIT's were called. There should be no need to rollback in an ELSE because with XACT_ABORT set to ON, if a Transact-SQL statement raises a run-time error, the entire transaction is terminated and rolled back.

SET XACT_ABORT ON;

BEGIN TRANSACTION;
GO

IF XACT_STATE() = 1
BEGIN
    CREATE TABLE [Blogs] ([Name] nvarchar(10) NULL);
END
GO

IF XACT_STATE() = 1
BEGIN
    INSERT INTO [Blogs] VALUES (N'Blog 1');
END
GO

IF XACT_STATE() = 1
BEGIN
    INSERT INTO [Blogs] VALUES (N'Blog 2 - Name is too long');
END
GO

IF XACT_STATE() = 1
BEGIN
    COMMIT;
END
GO


PRINT 'Before Second Transaction'
PRINT @@TRANCOUNT
BEGIN TRANSACTION;
GO

IF XACT_STATE() = 1
BEGIN
    INSERT INTO [Blogs] VALUES (N'Blog 3');
END
GO

IF XACT_STATE() = 1
BEGIN
    COMMIT;
END
GO


PRINT 'Before Third Transaction'
PRINT @@TRANCOUNT

BEGIN TRANSACTION;
GO

IF XACT_STATE() = 1
BEGIN
    INSERT INTO [Blogs] VALUES (N'Blog 4');
END
GO

IF XACT_STATE() = 1
BEGIN
    COMMIT;
END
GO

(1 row affected)
Msg 8152, Level 16, State 13, Line 20
String or binary data would be truncated.
Before Second Transaction
0
Msg 208, Level 16, State 1, Line 38
Invalid object name 'Blogs'.
Before Third Transaction
0
Msg 208, Level 16, State 1, Line 57
Invalid object name 'Blogs'.

Completion time: 2020-09-19T01:30:17.9541196-07:00

As for only wrapping the COMMIT's, I tried that too, but it still inserts data into the __EFMigrationsHistory table:

SET XACT_ABORT ON;
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

BEGIN TRANSACTION;
GO

CREATE TABLE [Blogs] ([Name] nvarchar(10) NULL);
GO

INSERT INTO [Blogs] VALUES (N'Blog 1');
GO

INSERT INTO [Blogs] VALUES (N'Blog 2 - Name is too long');
GO

INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion])
VALUES (N'20200919002238_Initial1', N'5.0.0-rc.1.20451.13');
GO

IF XACT_STATE() = 1
BEGIN
    COMMIT;
END
GO


BEGIN TRANSACTION;
GO

INSERT INTO [Blogs] VALUES (N'Blog 3');
GO

INSERT INTO [__EFMigrationsHistory] ([MigrationId], [ProductVersion])
VALUES (N'20200919002239_Initial2', N'5.0.0-rc.1.20451.13');
GO

IF XACT_STATE() = 1
BEGIN
    COMMIT;
END
GO

PRINT 'Transaction Count'
PRINT @@TRANCOUNT

(1 row affected)
Msg 8152, Level 16, State 13, Line 21
String or binary data would be truncated.

(1 row affected)
Msg 208, Level 16, State 1, Line 38
Invalid object name 'Blogs'.

(1 row affected)
Transaction Count
0

Completion time: 2020-09-19T01:36:06.1506185-07:00

You also cant wrap the whole transaction in an IF because SQL Server doesn't allow for IF statements to span multiple batches. (i.e. the following is invalid SQL):

IF 1=1
BEGIN
    SELECT GETUTCDATE()
    GO

    SELECT GETUTCDATE()
    GO
END

If the batch terminators (GO statements) were removed, then a normal transaction as it exists today, with the addition of a TRY/CATCH, would be sufficient. I don't know if there is a specific reason that each command is batched vs each migration/commit.

[roji]

Looks right, thanks for testing. Am a PostgreSQL guy so my SQL Server knowledge is limited - the interaction of transactions, XACT_ABORT and GO batches seems really odd, @bricelam what do you think?

[ChristopherHaws]

@roji GO is not a SQL keyword but rather a way for tools like SSMS, sqlcmd, etc to split up the execution of statements. Each "batch" is run in sequence and errors will not prevent the next batch from executing. The first batch with an error will trigger the transaction to rollback, thus making the subsequent batches not take part of the transaction. Usually you would handle rolling back a transaction in a TRY/CATCH block, but you cannot do that when using batches. The only way I have come up with to use transactions across batches is to verify that the transaction is still active and valid (via XACT_STATE) before every batch.

Another way this could work if we don't want to wrap each batch in an IF, we might be able to add a simple IF to the beginning of each batch:

DROP TABLE IF EXISTS dbo.Blogs
GO

SET XACT_ABORT ON;

BEGIN TRANSACTION;
GO

IF XACT_STATE() <> 1 RETURN
CREATE TABLE [Blogs] ([Name] nvarchar(10) NULL);
GO

IF XACT_STATE() <> 1 RETURN
INSERT INTO [Blogs] VALUES (N'Blog 1');
GO

IF XACT_STATE() <> 1 RETURN
INSERT INTO [Blogs] VALUES (N'Blog 2 - Name is too long');
GO

IF XACT_STATE() <> 1 RETURN
INSERT INTO [Blogs] VALUES (N'Blog 3');
GO


IF XACT_STATE() <> 1 RETURN
COMMIT;
GO


SELECT * FROM [Blogs]

(1 row affected)
Msg 8152, Level 16, State 13, Line 18
String or binary data would be truncated.
Msg 208, Level 16, State 1, Line 31
Invalid object name 'Blogs'.

Completion time: 2020-09-19T12:37:04.6429332-07:00

[bricelam]

Is there a way to tell the tool to stop on error? PowerShell has the same terrible default behavior, and it can really screw up your machine sometimes.

[bricelam]

When I was testing the scripts with MSDeploy (which powers VS Publish) it seemed to use the stop on error behavior.

[ChristopherHaws]

@bricelam It probably depends on the tool, for example sqlcmd has an :on error exit, but if we went this route, we would need to know about how each tool works and generate a specific script for each of those tools. I think that checking the transaction state before each batch and skipping that batch (via RETURN or wrapping the batch in an IF statement) if it the state is invalid would be a more universal solution.

Here is a blog post from red-gate on how they handle this problem:
https://www.red-gate.com/hub/product-learning/sql-change-automation/transaction-handling-techniques-in-t-sql-deployments

If we are ok with scripted migrations being tied to sqlcmd, then the following would work:

-- Abort execution if a error is encoundered in a batch
:on error exit

-- Automatically rollback the current transaction when a Transact-SQL statement raises a run-time error
SET XACT_ABORT ON;

BEGIN TRANSACTION;
GO

CREATE TABLE [Blogs] ([Name] nvarchar(10) NULL);
GO

INSERT INTO [Blogs] VALUES (N'Blog 1');
GO

INSERT INTO [Blogs] VALUES (N'Blog 2 - Name is too long');
GO

INSERT INTO [Blogs] VALUES (N'Blog 3');
GO

COMMIT;
GO

BTW, I am happy to do the work to fix this once we finalize a solution. I created a PR last night with the XACT_STATE implementation, but if you would prefer the sqlcmd solution, I am happy to change it to use that.

[ajcvickers]

We discussed this at length in triage. It's unfortunate that some tools keep executing the script even when it fails. In the future we can mitigate this through non-scripted migrations such as migrations bundles.

However, this is not really a new issue. The script would continue to execute after an error even before we used transactions. In addition, making changes that may or not work in various tools that run the scripts seems to risky to do for 5.0 at this point. Therefore, we're not going to change the code here in 5.0.

I have filed https://github.com/dotnet/EntityFramework.Docs/issues/2671 to better document this.

Also, adding this to the overall migrations experience in #19587. Also see #22616, since a single transaction for all migrations may be preferable if it can be done cleanly.

[bricelam]

Ooo, I really like this Stack Overflow answer. Try to use :ON ERROR EXIT and if it fails, direct users to switch SSMS to SQLCMD mode (Query > SQLCMD Mode) and run again.

[ChristopherHaws]

@bricelam Yeah, I mention that in my previous comment, that is also how red-gate does it for their migration scripts. That would lock the migration scripts in to only work on SQLCMD however. If that is acceptable, I could do modify my PR to do this instead. I don't believe it would be very hard.

Maybe if transactions were not enabled by default (to prevent this from being a breaking change), using :ON ERROR EXIT would be an acceptable solution for servicing and then a possible 6.0 feature would allow you to pass a flag to dotnet-ef to tell it to output a script that doesn't require SQLCMD which uses XACT_STATE.

@ajcvickers I agree that it would be very helpful to have the entire migration script be wrapped in the transaction instead of each migration. This is something I had to make work via a custom IMigrator.

I would really like this to be in the 5.0 release as it will completely block me from switching to EF 5.0 which I am desperately waiting for due to the SplitQuery functionality. :(

[ChristopherHaws]

I suppose it might be possible for me to make it do what I want without servicing, but it's requires me to still use the internal IMigrator type in my code. If I disable transactions in the dotnet ef migrations script command, I could probably use the following migrator implementation:

[SuppressMessage("Usage", "EF1001:Internal EF Core API usage.", Justification = "The built in migrator doesn't wrap the script in a transaction...")]
public class TransactionMigrator : Migrator
{
    public TransactionMigrator(
        IMigrationsAssembly migrationsAssembly,
        IHistoryRepository historyRepository,
        IDatabaseCreator databaseCreator,
        IMigrationsSqlGenerator migrationsSqlGenerator,
        IRawSqlCommandBuilder rawSqlCommandBuilder,
        IMigrationCommandExecutor migrationCommandExecutor,
        IRelationalConnection connection,
        ISqlGenerationHelper sqlGenerationHelper,
        ICurrentDbContext currentContext,
        IDiagnosticsLogger<DbLoggerCategory.Migrations> logger,
        IDiagnosticsLogger<DbLoggerCategory.Database.Command> commandLogger,
        IDatabaseProvider databaseProvider
    ) : base(
        migrationsAssembly,
        historyRepository,
        databaseCreator,
        migrationsSqlGenerator,
        rawSqlCommandBuilder,
        migrationCommandExecutor,
        connection,
        sqlGenerationHelper,
        currentContext,
        logger,
        commandLogger,
        databaseProvider
    ) {}

    public override string GenerateScript(string? fromMigration = null, string? toMigration = null, bool idempotent = false) => $@"
-- Abort execution if a error is encountered in a batch
:on error exit

-- Automatically rollback the current transaction when a Transact-SQL statement raises a run-time error
SET XACT_ABORT ON;

BEGIN TRANSACTION;
GO

{base.GenerateScript(fromMigration, toMigration, idempotent)}

COMMIT;
GO";
}

[bricelam]

Design proposal

SET CONTEXT_INFO 0x0;
GO

:On Error exit
SET CONTEXT_INFO 0xEF; -- NB: Not set when the previous statement fails
GO

IF CONTEXT_INFO() <> 0xEF
BEGIN
    PRINT CONCAT(
        'ERROR: Entity Framework scripts must be executed in SQLCMD Mode.', CHAR(10), CHAR(10),
        'To enable SQLCMD mode in Visual Studio, select "SQL > Execution Settings > SQLCMD Mode".', CHAR(10),
        'In SQL Server Management Studio, select "Query > SQLCMD Mode".');
    SET NOEXEC ON; -- Stops executing (but still continues parsing)
END
GO

SET XACT_ABORT ON;
GO

BEGIN TRANSACTION;
GO

-- Migration script here.
--     - No transactions around individual migrations (issue #22616)
--     - Batched statements (issue #3928)
--     - Begin batches with "IF XACT_STATE() <> 1 RETURN;"

COMMIT;
GO

[bricelam]

Not sure if both THROW and NOEXEC are needed, but you get the idea...

[ChristopherHaws]

@bricelam One possible concern is that the THROW 50000 might not stop the script if they aren't running in SQLCMD mode. There will be an exception but it will continue running because :On Error exit did nothing (since it isnt running SQLCMD). I don't really think there is much that can be done about that thought... Thoughts?

[bricelam]

If I understand correctly, THROW will put a message in the error window and SET NOEXEC ON will stop executing (but continue parsing) the rest of the script.

[ChristopherHaws]

If I run the following in SSMS with SQLCMD mode turned off

SET CONTEXT_INFO 0x0
GO

:On Error exit
SET CONTEXT_INFO 0xEF
GO

IF CONTEXT_INFO() <> 0xEF
BEGIN
    THROW 50000, 'Entity Framework scripts should be run in SQLCMD mode. Enable SQL > Execution Settings > SQLCMD Mode and try again.', 0;
    SET NOEXEC ON;
END;
GO

SET XACT_ABORT ON;
GO

BEGIN TRANSACTION;
GO

-- Migration script here (without individual transactions)
PRINT 'Running migration';

COMMIT;
GO

SET NOEXEC OFF;
GO

The result is:

Msg 102, Level 15, State 1, Line 4
Incorrect syntax near ':'.
Msg 50000, Level 16, State 0, Line 10
Entity Framework scripts should be run in SQLCMD mode. Enable SQL > Execution Settings > SQLCMD Mode and try again.
Running migration

Completion time: 2020-09-21T14:49:25.8295878-07:00

I would expect for "Running migration" to not show up in the output.

[bricelam]

Ugg

[bricelam]

Maybe print statements are special? 😇

[bricelam]

lol, maybe just a big fat warning comment at the top...

-- !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
-- ! ⚠ WARNING: This script must be executed in SQLCMD Mode. !
-- !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
--
-- To enable SQLCMD mode in Visual Studio, select "SQL > Execution Settings > SQLCMD Mode".
-- In SQL Server Management Studio, select "Query > SQLCMD Mode".

:On Error exit

[ChristopherHaws]

I dont think there is much else you can do. Maybe in a future version, if there is demand for it, we could have a way to change the output type to not use SQLCMD and it could check the XACT_STATE at the beginning of each batch, but this would be an opt-in feature.

[bricelam]

Yeah, I'd also want to do #3928 at the same time so it's not so invasive.

[bricelam]

Ah, the THROW prevents the SET NOEXEC from running. This works:

PRINT 'ERROR: Entity Framework scripts should be run in SQLCMD mode.';
SET NOEXEC ON;

[ChristopherHaws]

So here is the final proposal, which seems to be working!

SET CONTEXT_INFO 0x0
GO

-- Abort execution if a error is encountered in a batch
:ON ERROR EXIT

SET CONTEXT_INFO 0xEF
GO

IF CONTEXT_INFO() <> 0xEF
BEGIN
    PRINT 'ERROR: Entity Framework scripts should be run in SQLCMD mode. Enable SQL > Execution Settings > SQLCMD Mode and try again.';
    SET NOEXEC ON;
END;
GO

-- Automatically rollback the current transaction when a Transact-SQL statement raises a run-time error
SET XACT_ABORT ON;

-- Must be ON when you are creating a filtered index.
SET QUOTED_IDENTIFIER ON;
GO

BEGIN TRANSACTION;
GO

-- Migration script here (without individual transactions)
PRINT 'Running Migration...'

COMMIT;
GO

SET NOEXEC OFF;
GO

From an implementation point of view, would a new helper live in SqlServerSqlGenerationHelper and be exposed on ISqlGenerationHelper as something like AutomaticRollbackOnFailureStatement?

[ChristopherHaws]

I just thought of one more issue. SQLCMD won't report errors (since we are using PRINT to display the error), so in something like an automated deployment scenario, it will think the migration succeeded.

EDIT:
Actually maybe this is ok because the error would be:

Msg 102, Level 15, State 1, Line 5
Incorrect syntax near ':'.

on the :ON ERROR EXIT not working

[bricelam]

We should also include SET QUOTED_IDENTIFIER ON

[ChristopherHaws]

Would we also be changing the behavior to run the entire migration script in a single transaction (#22616) or keep the behavior of each migration in independent transaction?

[bricelam]

Single transaction

[ChristopherHaws]

I updated the final proposal to include SET QUOTED_IDENTIFIER ON

@ajcvickers With the changes proposed, can we consider this for the 5.0 release? I can submit a PR tonight with these changes.

[bricelam]

Feel free to send a PR whenever so we can start iterating on it.

This is unlikely to go into 5.0. This is not a new problem. You can go back to the previous transaction behavior using —no-transactions. RC is not an appropriate time to be introducing new behaviors; I think we’ll need a full release cycle to gather feedback on this and iron out any kinks users will inevitably find.

[ChristopherHaws]

@bricelam I created a PR. One thing that I am not sure how to handle is having the entire migration script run in a single transaction. Currently, MigrationCommands can suppress transactions in the middle of the migration (for example CREATE DATABASE cannot be run in a transaction). Any thoughts on how this could be handled?