Transaction Syntax Best Practice

  • When you are writing your stored procedures that perform multiple tasks on different tables and check for errors to rollback or continue with the transaction, do you use nested selection structures like the following:

    BEGIN TRANSACTION

     INSERT INTO Table (Column1, Column2,...) VALUES ('something', 'something else')

     IF (@@ERROR = 0)

      BEGIN

       UPDATE Table2 SET ForeignKey =@@IDENTITY WHERE PrimaryKey = @primarykey-2

       IF (@@ERROR = 0)

        BEGIN

         COMMIT TRANSACTION

        END

       ELSE

        BEGIN

         INSERT INTO ErrorLog (Table, Error) VALUES ('Table2', @@ERROR)

         ROLLBACK TRANSACTION

        END

      END

     ELSE

      BEGIN

       INSERT INTO ErrorLog (Table, Error) VALUES ('Table', @@ERROR)

       ROLLBACK TRANSACTION

      END

    Or something like the following:

    BEGIN TRANSACTION

     INSERT INTO Table (Column1, Column2,...) VALUES ('something', 'something else')

     IF (@@ERROR> 0)

      BEGIN

       INSERT INTO ErrorLog (Table, Error) VALUES ('Table', @@ERROR)

       ROLLBACK TRANSACTION

      END

     UPDATE Table2 SET ForeignKey =@@IDENTITY WHERE PrimaryKey = @primarykey-2

     IF (@@ERROR> 0)

      BEGIN

       INSERT INTO ErrorLog (Table, Error) VALUES ('Table2', @@ERROR)

       ROLLBACK TRANSACTION

      END

    COMMIT TRANSACTION

    It seems like both methods do the same thing, I am just looking for a best practice opinion, as well as an answer as to which one is the most efficient.  Any ideas?

  • I usually have a standard error handler that looks something like this (simplified):

    BEGIN TRANSACTION

    INSERT mytable (val1, val2)

    VALUES (@val1, @val2)

    SELECT @err = @@ERROR

    IF @err 0

    BEGIN

    SET @err_string = 'Insert to mytable failed.'

    GOTO ERROR_HANDLER

    END

    WHILE @@TRANCOUNT > 0 COMMIT TRAN

    RETURN(0)

    ERROR_HANDLER:

    IF @@TRANCOUNT > 0 ROLLBACK TRAN

    RAISERROR (@err_string, @err, 1);

    RETURN(-1)

    In SQL Server 2005 I modify it a little bit because of TRY/CATCH blocks making my life easier. In your case if you want to continue on non-critical errors I like the second syntax better than the nesting, but it's a matter of personal preference. I find the second example a little easier to follow and maintain.

  • BTW... couldn't help but notice you're using @@IDENTITY... if a trigger is present, that could mess things up because @@IDENTITY returns the last id of ANY table updated.  Recommend you you SCOPE_IDENTITY() instead.

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

    Change is inevitable... Change for the better is not.


    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)

  • From your example:

    IF (@@ERROR> 0)

    BEGIN

    INSERT INTO ErrorLog (Table, Error) VALUES ('Table2', @@ERROR)

    ROLLBACK TRANSACTION

    END

    As there is a rollback, this will also rollback the insert into the ErrorLog table and you will find that this table is always empty. Also, after the rollback, you need to exit the stored procedure, so add a RETURN +1

    Personally, I just write my stored procedures without any error checking by aborting on any error. You also need to begin/commit only when the SP is not within a transaction.

    create procedure mysp

    (@PrimaryKey integer )

    as

    set nocount on

    set xact_abort on

    declare @Trancount integer

    set @Trancount = @@trancount

    if @Trancount = 0 BEGIN TRANSACTION

    INSERT INTO Table1 (Column1, Column2)

    VALUES ('something', 'something else')

    UPDATE Table2

    SET ForeignKey = SCOPE_IDENTITY()

    WHERE PrimaryKey = @primarykey-2

    if @Trancount = 0 COMMIT

    go

    SQL = Scarcely Qualifies as a Language

  • Jim,

    check you flow.

    Error in INSERT does not prevent execution of UPDATE.

    Moreover, because your transaction is already rolled back either ROLLBACK or COMMIT will cause an error.

    And I don't see where you're catching that error.

    _____________
    Code for TallyGenerator

  • Jim,

    There also is a misuse of @@ERROR function - it's value is reset by ANY next statement executed including conditional statements (in your example it is IF statement). If you want to log execution errors in ErrorLog table you need to store the value of @@ERROR in a local variable IMMEDIATELY after the action being checked (i.e. INSERT or UPDATE) and use that variable later for logging. Also as Sergiy stated you need to perform logging after rollback

  • OK.  Thanks.

  • I use Aaron's method.

    I only ever have transactions in top level procedures. All nested procedure calls write an error to a temporary error table. My top level procedures retrieve the error description from temporary table prior to rolling back the transaction. I then write to a permanent error table.

  • Question - doesn't your second method update the table regardless of outcome due to the insert? It seems to me that the two options are not logically equivalent! What am I missing?

Viewing 9 posts - 1 through 8 (of 8 total)

You must be logged in to reply to this topic. Login to reply