Transactions and @@Error function in SQL Server

Question

I just want to check if the place where I put the @@Error and Begin/Commit tran is correct?

I am unsure if I should you the Begin Tran over the DELETE statement instead? And does the @@ERROR make any sense at all?

Thanks!

CREATE PROCEDURE spDeleteAnInactiveEmployee
    @TrainerID int,
    @EActive char (1)
AS
    BEGIN TRY
    BEGIN TRAN

        IF (SELECT COUNT(*) FROM EmployeeDetails ed 
            WHERE TrainerID = @TrainerID) = 0
          RAISERROR ('Trainer details were not deleted. Trainer ID does not exist.', 16, 1)

        IF EXISTS (SELECT * FROM EmployeeDetails ed 
                   WHERE TrainerID = @TrainerID AND EActive = 'Y')
            RAISERROR ('Trainer details were not deleted. Trainer is still active.', 16, 1)

        DELETE FROM [EmployeeDetails]    
        WHERE TrainerID = @TrainerID AND EActive = 'N'

        IF @@ERROR = 0
            COMMIT TRAN

        BEGIN
            PRINT 'Employee ID' + CAST (@TrainerID AS VARCHAR) + ' was successfully deleted.'
        END
    END TRY
    BEGIN CATCH
        SELECT
            ERROR_NUMBER() AS ErrorNumber,
            ERROR_STATE() AS ErrorState,
            ERROR_SEVERITY() AS ErrorSeverity,
            ERROR_PROCEDURE() AS ErrorProcedure,
            ERROR_LINE() AS ErrorLine,
            ERROR_MESSAGE() AS ErrorMessage;

        IF (XACT_STATE()) = -1
        BEGIN  
            PRINT 'Transaction was not committed' 
            ROLLBACK TRANSACTION;  
        END;  

        IF (XACT_STATE()) = 1
        BEGIN 
            PRINT 'Transaction was committed'
            COMMIT TRANSACTION;
        END;
    END CATCH;
GO

Why are you selecting the error here? Wouldn't you be better off THROWing it? Also why COMMIT the transaction on error? Generally you don't want to COMMIT on error. — Thom A
– Thom A ♦, Commented Jun 3, 2020 at 12:38
I see no @@ERROR check in the code, only normal structured handling. I suggest 1) specify SET XACT_ABORT ON; at the beginning of the proc to ensure immediate rollback in the case of a query timeout and 2) change the catch block to the simplified IF @@TRANCOUNT > 0 ROLLBACK; THROW; (assuming SQL 2012 or later) to rollback the tran and re-raise the original error. — Dan Guzman
– Dan Guzman, Commented Jun 3, 2020 at 12:38
does the @@ERROR make any sense at all - no, because in a try block, raiserror will immediately jump to catch. Outside of a try block it would carry on and reach your @@error. — GSerg
– GSerg, Commented Jun 3, 2020 at 15:59

David Browne - Microsoft · Accepted Answer · 2020-06-03 16:14:28Z

@@ERROR is unnecessary when you use TRY/CATCH. Before TRY/CATCH you had to check @@ERROR after each statement that might fail and use GOTO to force control flow to an error label.

So this should be something like:

CREATE PROCEDURE spDeleteAnInactiveEmployee
    @TrainerID int,
    @EActive char (1)
AS
BEGIN
    SET XACT_ABORT ON;
    BEGIN TRY
        BEGIN TRAN

        IF (SELECT COUNT(*) FROM EmployeeDetails ed 
            WHERE TrainerID = @TrainerID) = 0
          RAISERROR ('Trainer details were not deleted. Trainer ID does not exist.', 16, 1)

        IF EXISTS (SELECT * FROM EmployeeDetails ed 
                   WHERE TrainerID = @TrainerID AND EActive = 'Y')
            RAISERROR ('Trainer details were not deleted. Trainer is still active.', 16, 1)

        DELETE FROM [EmployeeDetails]    
        WHERE TrainerID = @TrainerID AND EActive = 'N'

        COMMIT TRAN
        PRINT 'Employee ID' + CAST (@TrainerID AS VARCHAR) + ' was successfully deleted.'
    END TRY
    BEGIN CATCH
        IF @@TRANCOUNT > 0 ROLLBACK; 
        THROW;
    END CATCH;
END

Collectives™ on Stack Overflow

Transactions and @@Error function in SQL Server

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related