3

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.

5

@@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;
  • Sorry about that, I didn't realise hitting "Enter" would save and post the comment. Here is what I meant to say: I tried the below to no avail: BEGIN TRANSACTION insert intos, alter tables, etc. GO IF @@ERROR=0 ALTER first procedure GO IF @@ERROR=0 ALTER 2nd procedure GO IF @@ERROR=0 COMMIT TRANSACTION ELSE ROLLBACK TRANSACTION GO And I get the error below: Msg 156, Level 15, State 1, Line 4 Incorrect syntax near the keyword 'PROCEDURE'. And then a Msg 137 error for each variable inside the procedures – Daniel F Apr 23 '15 at 20:43
  • 4
    You can't say IF <something> ALTER PROCEDURE. The ALTER needs to be in its own batch. So you can use labels and GOTO like I suggested in my answer, or you can maintain the error status of each action in a #temp table, and just roll back the whole thing at the end (this is how Red Gate SQL Compare does it IIRC). Please read sommarskog.se/error-handling-I.html and sommarskog.se/error-handling-II.html and sommarskog.se/error_handling_2005.html – Aaron Bertrand Apr 23 '15 at 20:46
1

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.

  • Thanks for all the help thus far. Per @Aaron's advice, below is a pastebin of the script I just tried: pastebin.com/CNHVzuFD I'm still seeing the edits go through on the 2nd proc when the first one fails. I really appreciate the help you guys are willing to provide. This is actually the first time I couldn't find a solution by reading SO :) – Daniel F Apr 24 '15 at 14:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.