Half of a transaction completes, when the other half fails?

Question

I have done some searching on SO, MSDN, and various other sources, and I found plenty of questions about transactions, but none that seem to be exactly what I'm dealing with.

Admittedly I'm still very much a junior DBA, though I do understand the concept of ACID and transactions. I'm working on a SQL script that:

1. Adds a row to 2 different tables, provided the values aren't already there
2. Drops a column if it exists
3. Adds a column if it doesn't exist
4. Alters 2 stored procedures that generate dynamic SQL to build a report query

All of the above operations work on their own, and they also work together when wrapped up in (what I believe to be) a single transaction. What I'm trying to understand is how "part" of the transaction completes (and presumably commits) when another part of it fails.

For the sake of brevity, assume I have working procedures called "ProcessReport" and "RetrieveReport." The dynamic SQL in both of these procedures starts with "SELECT" and then builds from there. Test cases:

CREATE TABLE ReportTable (FirstName VARCHAR(100), LastName VARCHAR(100), OrderId INT);
GO
CREATE PROCEDURE ProcessReport
AS
BEGIN
  DECLARE @SQL VARCHAR(4000)

  -- start creating dynamic sql   
  SET @SQL = 'SELECT FirstName, LastName, '
  --create rest of dynamic SQL here...

  INSERT INTO ReportTable (FirstName, LastName) EXEC(@SQL)
END
GO 

CREATE PROCEDURE RetrieveReport
AS
BEGIN
  DECLARE @SQL VARCHAR(4000)

  -- start creating dynamic sql   
  SET @SQL = 'SELECT FirstName, LastName '
  --create rest of dynamic SQL here...

  SET @SQL = @SQL + ' FROM ReportTable '
  EXEC (@SQL)
END
GO   

Now that the procedures exist, here's what has me kerfuffled: when I purposely enter the wrong name for the first ALTER PROCEDURE, the edits I made to the second procedure are completed. Below is the short version of the transaction I've written:

BEGIN TRANSACTION 
GO 

ALTER PROCEDURE ProcessReport2  -- this procedure does not exist, causing an error
AS
BEGIN
  DECLARE @SQL VARCHAR(4000)

  -- start creating dynamic sql   
  SET @SQL = 'SELECT FirstName, LastName, '
  --create rest of dynamic SQL here...

  INSERT INTO ReportTable(FirstName, LastName) EXEC(@SQL)
END
GO
ALTER PROCEDURE RetrieveReport
AS
BEGIN
  DECLARE @SQL VARCHAR(4000)

  -- start creating dynamic sql   
  SET @SQL = 'SELECT DISTINCT FirstName, LastName '  --DISTINCT added
  --create rest of dynamic SQL here...

  SET @SQL = @SQL + ' FROM ReportTable '
  EXEC (@SQL)

END
GO 

IF @@ERROR = 0
  COMMIT TRANSACTION;
ELSE
  ROLLBACK TRANSACTION;

When running the above statement, I get the below error:

Msg 208, Level 16, State 6, Procedure ProcessReport2, Line 2 Invalid object name 'ProcessReport2'.

Msg 3902, Level 16, State 1, Line 3 The COMMIT TRANSACTION request has no corresponding BEGIN TRANSACTION.

With regard to ACID and transactions, it was my understanding that either everything between BEGIN TRANSACTION and COMMIT TRANSACTION completes, or nothing does. In SSMS, when I click "Modify" on the RetrieveReport procedure, I now see the dynamic SQL starting with "SELECT DISTINCT..."

Can anyone tell me where I'm going wrong here? Is it something in my

IF @@ERROR = 0
COMMIT TRANSACTION;

ELSE
ROLLBACK TRANSACTION;

My intent with that was "if there are no errors, commit the whole thing, if there are errors, don't do anything."

I'm using SQL Server 2012 Enterprise.

Any help and/or insight will be greatly appreciated.

Answer 1

@@ERROR is reset after every statement. The error from your attempt to alter the first procedure is no longer detectable via @@ERROR after you successfully altered the second procedure. Here is an even simpler repro:

SELECT 1/0;
GO
SELECT @@ERROR; -- 8134

However if I put a successful statement in between:

SELECT 1/0;
GO
SELECT 1/1;
GO
SELECT @@ERROR; -- 0

In your case, you are doing this:

ALTER dbo.p1 ... -- fails
GO
-- @@ERROR here would be <> 0, but you're not checking it here!

ALTER dbo.p2 ... -- succeeds
GO
-- You're checking @@ERROR here, but success is 0

The only way your current logic would roll everything back is if the last ALTER failed. You need to check for @@ERROR after every ALTER. Here is an example.

USE tempdb;
GO
CREATE TABLE #x(err INT);
GO

BEGIN TRANSACTION;
GO

CREATE PROCEDURE dbo.foo -- fails
AS 
  SELECT foo FROM sys.objects;
GO
IF @@ERROR <> 0 
  INSERT #x(err) SELECT 1;
GO

CREATE PROCEDURE dbo.blat -- succeeds, but will get rolled back
AS 
  SELECT 1;
GO
IF @@ERROR <> 0 
  INSERT #x(err) SELECT 1;
GO

IF EXISTS (SELECT 1 FROM #x)
  ROLLBACK TRANSACTION;
ELSE
  COMMIT TRANSACTION;

DROP TABLE #x;

Answer 2

As Aaron has explained, @@ERROR needs to be checked after each ALTER PROCEDURE. There is no avoiding that, but as for using a transaction, you could replace it with a different approach, if you are open to suggestions on that.

You could manipulate the SET NOEXEC setting to achieve the same result you are using a transaction for: either both procedures altered or both left unchanged. This is how:

SET NOEXEC OFF  -- usually the default, but just in case
GO

ALTER PROCEDURE dbo.proc1
AS
  . . .
GO

IF @@ERROR <> 0  -- if an error occurred altering dbo.proc1
  SET NOEXEC ON  -- then skip altering the following one(s)
;
GO

ALTER PROCEDURE dbo.proc2
AS
  . . .
GO

IF @@ERROR <> 0  -- apply the same after each procedure that you want
  SET NOEXEC ON  -- to be either altered or skipped as a single batch
;
GO

. . .

ALTER PROCEDURE dbo.procN
AS
  . . .
GO

IF @@ERROR <> 0  -- strictly, no need to use it after the last one
  SET NOEXEC ON  -- but it is no harm if you do
;
GO

SET NOEXEC OFF  -- just reset to proceed with the rest of the script normally
GO
. . .

SET NOEXEC ON causes SQL Server to stop executing the subsequent statements until the end of the script or until SET NOEXEC OFF is encountered, the latter setting the execution back on.