Reputation: 2147
When using Flyway in combination with a Microsoft SQL Server, we are observing the issue described on this question.
Basically, a migration script like this one does not rollback the successful GO
-delimited batches when another part failed:
BEGIN TRANSACTION
-- Create a table with two nullable columns
CREATE TABLE [dbo].[t1](
[id] [nvarchar](36) NULL,
[name] [nvarchar](36) NULL
)
-- add one row having one NULL column
INSERT INTO [dbo].[t1] VALUES(NEWID(), NULL)
-- set one column as NOT NULLABLE
-- this fails because of the previous insert
ALTER TABLE [dbo].[t1] ALTER COLUMN [name] [nvarchar](36) NOT NULL
GO
-- create a table as next action, so that we can test whether the rollback happened properly
CREATE TABLE [dbo].[t2](
[id] [nvarchar](36) NOT NULL
)
GO
COMMIT TRANSACTION
In the above example, the table t2
is being created even though the preceding ALTER TABLE
statement fails.
On the linked question, the following approaches (outside of the flyway context) were suggested:
A multi-batch script should have a single error handler scope that rolls back the transaction on error, and commits at the end. In TSQL you can do this with dynamic sql
With SQLCMD you can use the
-b
option to abort the script on error
Or roll your own script runner
EDIT: alternative example
BEGIN TRANSACTION
CREATE TABLE [a] (
[a_id] [nvarchar](36) NOT NULL,
[a_name] [nvarchar](100) NOT NULL
);
CREATE TABLE [b] (
[b_id] [nvarchar](36) NOT NULL,
[a_name] [nvarchar](100) NOT NULL
);
INSERT INTO [a] VALUES (NEWID(), 'name-1');
INSERT INTO [b] VALUES (NEWID(), 'name-1'), (NEWID(), 'name-2');
COMMIT TRANSACTION
BEGIN TRANSACTION
ALTER TABLE [b] ADD [a_id] [nvarchar](36) NULL;
UPDATE [b] SET [a_id] = [a].[a_id] FROM [a] WHERE [a].[a_name] = [b].[a_name];
ALTER TABLE [b] ALTER COLUMN [a_id] [nvarchar](36) NOT NULL;
ALTER TABLE [b] DROP COLUMN [a_name];
COMMIT TRANSACTION
This results in the error message Invalid column name 'a_id'.
for the UPDATE
statement.
Possible solution: introduce GO
between statements
BEGIN TRANSACTION
SET XACT_ABORT ON
GO
ALTER TABLE [b] ADD [a_id] [nvarchar](36) NULL;
GO
UPDATE [b] SET [a_id] = [a].[a_id] FROM [a] WHERE [a].[a_name] = [b].[a_name];
GO
ALTER TABLE [b] ALTER COLUMN [a_id] [nvarchar](36) NOT NULL;
GO
ALTER TABLE [b] DROP COLUMN [a_name];
GO
COMMIT TRANSACTION
[b]
have a matching entry in table [a]
.Cannot insert the value NULL into column 'a_id', table 'test.dbo.b'; column does not allow nulls. UPDATE fails.
The COMMIT TRANSACTION request has no corresponding BEGIN TRANSACTION.
ALTER TABLE [b] DROP COLUMN [a_name]
statement was actually executed, committed and not rolled back. I.e. one cannot fix this up afterwards as the linking column is lost.This behaviour is actually independent of flyway and can be reproduced directly via SSMS.
Upvotes: 3
Views: 1666
Reputation: 420
Edited 20201102 -- learned a lot more about this and largely rewrote it! So far have been testing in SSMS, do plan to test in Flyway as well and write up a blog post. For brevity in migrations, I believe you could put the @@trancount check / error handling into a stored procedure if you prefer, that's also on my list to test.
For error handling and transaction management in SQL Server, there are three things which may be of great help:
Two changes:
In this case the @@TRANCOUNT check clause will work even if XACT_ABORT is off, but I believe you want it on for other cases. (Need to read up more on this, but I haven't come across a downside to having it ON yet.)
BEGIN TRANSACTION;
SET XACT_ABORT ON;
GO
-- Create a table with two nullable columns
CREATE TABLE [dbo].[t1](
[id] [nvarchar](36) NULL,
[name] [nvarchar](36) NULL
)
-- add one row having one NULL column
INSERT INTO [dbo].[t1] VALUES(NEWID(), NULL)
-- set one column as NOT NULLABLE
-- this fails because of the previous insert
ALTER TABLE [dbo].[t1] ALTER COLUMN [name] [nvarchar](36) NOT NULL
GO
IF @@TRANCOUNT <> 1
BEGIN
DECLARE @ErrorMessage AS NVARCHAR(4000);
SET @ErrorMessage
= N'Transaction in an invalid or closed state (@@TRANCOUNT=' + CAST(@@TRANCOUNT AS NVARCHAR(10))
+ N'). Exactly 1 transaction should be open at this point. Rolling-back any pending transactions.';
RAISERROR(@ErrorMessage, 16, 127);
RETURN;
END;
-- create a table as next action, so that we can test whether the rollback happened properly
CREATE TABLE [dbo].[t2](
[id] [nvarchar](36) NOT NULL
)
GO
COMMIT TRANSACTION;
I added a bit of code at the top to be able to reset the test database. I repeated the pattern of using XACT_ABORT ON and checking @@TRANCOUNT after each batch terminator (GO) is sent.
/* Reset database */
USE master;
GO
IF DB_ID('transactionlearning') IS NOT NULL
BEGIN
ALTER DATABASE transactionlearning
SET SINGLE_USER
WITH ROLLBACK IMMEDIATE;
DROP DATABASE transactionlearning;
END;
GO
CREATE DATABASE transactionlearning;
GO
/* set up simple schema */
USE transactionlearning;
GO
BEGIN TRANSACTION;
CREATE TABLE [a]
(
[a_id] [NVARCHAR](36) NOT NULL,
[a_name] [NVARCHAR](100) NOT NULL
);
CREATE TABLE [b]
(
[b_id] [NVARCHAR](36) NOT NULL,
[a_name] [NVARCHAR](100) NOT NULL
);
INSERT INTO [a]
VALUES
(NEWID(), 'name-1');
INSERT INTO [b]
VALUES
(NEWID(), 'name-1'),
(NEWID(), 'name-2');
COMMIT TRANSACTION;
GO
/*******************************************************/
/* Test transaction error handling starts here */
/*******************************************************/
USE transactionlearning;
GO
BEGIN TRANSACTION;
SET XACT_ABORT ON;
GO
IF @@TRANCOUNT <> 1
BEGIN
DECLARE @ErrorMessage AS NVARCHAR(4000);
SET @ErrorMessage
= N'Check 1: Transaction in an invalid or closed state (@@TRANCOUNT=' + CAST(@@TRANCOUNT AS NVARCHAR(10))
+ N'). Exactly 1 transaction should be open at this point. Rolling-back any pending transactions.';
RAISERROR(@ErrorMessage, 16, 127);
RETURN;
END;
ALTER TABLE [b] ADD [a_id] [NVARCHAR](36) NULL;
GO
IF @@TRANCOUNT <> 1
BEGIN
DECLARE @ErrorMessage AS NVARCHAR(4000);
SET @ErrorMessage
= N'Check 2: Transaction in an invalid or closed state (@@TRANCOUNT=' + CAST(@@TRANCOUNT AS NVARCHAR(10))
+ N'). Exactly 1 transaction should be open at this point. Rolling-back any pending transactions.';
RAISERROR(@ErrorMessage, 16, 127);
RETURN;
END;
UPDATE [b]
SET [a_id] = [a].[a_id]
FROM [a]
WHERE [a].[a_name] = [b].[a_name];
GO
IF @@TRANCOUNT <> 1
BEGIN
DECLARE @ErrorMessage AS NVARCHAR(4000);
SET @ErrorMessage
= N'Check 3: Transaction in an invalid or closed state (@@TRANCOUNT=' + CAST(@@TRANCOUNT AS NVARCHAR(10))
+ N'). Exactly 1 transaction should be open at this point. Rolling-back any pending transactions.';
RAISERROR(@ErrorMessage, 16, 127);
RETURN;
END;
ALTER TABLE [b] ALTER COLUMN [a_id] [NVARCHAR](36) NOT NULL;
GO
IF @@TRANCOUNT <> 1
BEGIN
DECLARE @ErrorMessage AS NVARCHAR(4000);
SET @ErrorMessage
= N'Check 4: Transaction in an invalid or closed state (@@TRANCOUNT=' + CAST(@@TRANCOUNT AS NVARCHAR(10))
+ N'). Exactly 1 transaction should be open at this point. Rolling-back any pending transactions.';
RAISERROR(@ErrorMessage, 16, 127);
RETURN;
END;
ALTER TABLE [b] DROP COLUMN [a_name];
GO
COMMIT TRANSACTION;
There is a wonderful free resource online which digs into error and transaction handling in great detail. It is written and maintained by Erland Sommarskog:
One common question is why XACT_ABORT is still needed/ if it is entirely replaced by TRY/CATCH. Unfortunately it is not entirely replaced, and Erland has some examples of this in his paper, this is a good place to start on that.
Upvotes: 1
Reputation: 2805
The problem is fundamental to the GO command. It's not a part of the T-SQL language. It's a construct in use within SQL Server Management Studio, sqlcmd, and Azure Data Studio. Flyway is simply passing the commands on to your SQL Server instance through the JDBC connection. It's not going to be dealing with those GO commands like the Microsoft tools do, separating them into independent batches. That's why you won't see individual rollbacks on errors, but instead see a total rollback.
The only way to get around this that I'm aware of would be to break apart the batches into individual migration scripts. Name them in such a way so it's clear, V3.1.1, V3.1.2, etc. so that everything is under the V3.1* version (or something similar). Then, each individual migration will pass or fail instead of all going or all failing.
Upvotes: 1