efcore: SqlServer Migrations: Scripts do not fail on failure

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:
// Startup.cs
services.AddDbContext<ApplicationContext>(x => {
    x.UseSqlServer(@"Server=(localdb)\mssqllocaldb; Database=ef; Trusted_Connection=True; MultipleActiveResultSets=true");
});

public class ApplicationContext : DbContext {
    public ApplicationContext(DbContextOptions<ApplicationContext> 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; }
}
  1. Create a migration:
dotnet migrations add Initial
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"
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. image

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

About this issue

  • Original URL
  • State: open
  • Created 4 years ago
  • Reactions: 8
  • Comments: 37 (32 by maintainers)

Commits related to this issue

Most upvoted comments

You can get this behavior right now like this:

new DbContextOptionsBuilder<TContext>().ReplaceService<IMigrator, TransactionalMigrator>()
public class TransactionalMigrator : Migrator
{
    public TransactionalMigrator(IMigrationsAssembly migrationsAssembly, IHistoryRepository historyRepository, IDatabaseCreator databaseCreator, IMigrationsSqlGenerator migrationsSqlGenerator, IRawSqlCommandBuilder rawSqlCommandBuilder, IMigrationCommandExecutor migrationCommandExecutor, IRelationalConnection connection, ISqlGenerationHelper sqlGenerationHelper, ICurrentDbContext currentContext, IConventionSetBuilder conventionSetBuilder, IDiagnosticsLogger<DbLoggerCategory.Migrations> logger, IDiagnosticsLogger<DbLoggerCategory.Database.Command> commandLogger, IDatabaseProvider databaseProvider)
        : base(migrationsAssembly, historyRepository, databaseCreator, migrationsSqlGenerator, rawSqlCommandBuilder, migrationCommandExecutor, connection, sqlGenerationHelper, currentContext, conventionSetBuilder, logger, commandLogger, databaseProvider)
    {
    }

    public override string GenerateScript(string fromMigration = null, string toMigration = null, MigrationsSqlGenerationOptions options = MigrationsSqlGenerationOptions.Default)
    {
        var result = base.GenerateScript(fromMigration, toMigration, options | MigrationsSqlGenerationOptions.NoTransactions);
        return string.IsNullOrEmpty(result) ? result :
$@"SET CONTEXT_INFO 0x0;
GO

-- Abort execution if an 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.';

    -- Disable execution if not running in SQLCMD mode
    SET NOEXEC ON;
END;
GO

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

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

BEGIN TRANSACTION;
GO

{result}

COMMIT;
GO

SET NOEXEC OFF;
GO

SET QUOTED_IDENTIFIER OFF;
GO
";
    }
}

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.

Single transaction

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?

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;

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

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

@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