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:

// 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; }
}

2. 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" });
    }
}

3. 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

4. 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: #7681 (comment)

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: #7681 (comment)

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 dotnet/EntityFramework.Docs#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

[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]

@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?

[Chris-Huzza]

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

Wouldn't a slight tweak to the above (adding the ELSE statement) be kinder to the users - saves them having to run the NOEXEC OFF command themselves if they had to turn SQLCMD mode on.

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;
ELSE
    SET NOEXEC OFF;
GO

[gabrielmaldi]

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
";
    }
}

[clement911]

We encounter this issue. I see this issue is still open.

We generate sql migration scripts and run them manually.
However, we found that even though the script encountered a failure, a new row is inserted to the migration table, as if the script had ran succesfully.

I was surprised by this behaviour given that the whole thing is wrapped in a transaction.
As it's been almost 2 years since the last activity on this issue, is this still pending a fix? Is there a recommended work around?

[ajcvickers]

@clement911 There are a few options:

• Use migration bundles rather than executing the raw script.
• Run the script in a tool that will stop when errors are encountered.
• Modify the script creation as shown in SqlServer Migrations: Scripts do not fail on failure #22613 (comment) or automatically as shown in SqlServer Migrations: Scripts do not fail on failure #22613 (comment).

[clement911]

Thank you for the quick response.

Using bundles is a bit scary for us because it's a black box and we like to review the script and run it manually in SSMS in case something unexpected occurs.

Is there any reason why the final proposal above is not yet merged in?

[ajcvickers]

@clement911 Because it hasn't been implemented yet.

[clement911]

@ajcvickers fair enough, I thought there might be some issues left to be worked out...

[clement911]

Wouldn't simply not emitting the GO separator fix the issue?

[roji]

@clement911 IIRC there are various DDL statement which SQL Server only supports at the beginning of a "batch" (i.e. right after GO); IIRC that's at least one reason why we have them.

[clement911]

I see, thanks @roji

For future reference, according to this doc on batches (old but still relevant I believe):

1. CREATE DEFAULT, CREATE FUNCTION, CREATE PROCEDURE, CREATE RULE, CREATE SCHEMA, CREATE TRIGGER, and CREATE VIEW statements cannot be combined with other statements in a batch. The CREATE statement must start the batch. All other statements that follow in that batch will be interpreted as part of the definition of the first CREATE statement.

2. A table cannot be changed and then the new columns referenced in the same batch.

3. If an EXECUTE statement is the first statement in a batch, the EXECUTE keyword is not required. The EXECUTE keyword is required if the EXECUTE statement is not the first statement in the batch.

As far as 1, migrations can definitely generate some 'CREATE SCHEMA' commands.
Point 2 is also relevant.
I don't think 3 is relevant for migrations.

[Charlieface]

@roji

The real solution here seems to me to indeed leave out the GO, and just use SET XACT_ABORT ON. And put each migration command into their own batch using EXEC ('command here'). This means that effectively your script is the tool, so you can rely on your own error handling rather than whatever the user decide to use. XACT_ABORT then stops the whole batch and rolls back as soon as the first error occurs.

So the final script looks like this

SET XACT_ABORT ON;

BEGIN TRANSACTION;

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

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

EXEC ('
INSERT INTO [Blogs] VALUES (N''Blog 2 - Someone''''s name is too long'');
');

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

COMMIT;

Note that you obviously need to double up all the apostrophes to escape them. Any IF EXISTS checks for object existence etc can still go in the main part of the batch, just the DDL and INSERT commands need to go in the dynamic parts.