CODEWITHSUNDEEP
sqlt-sqlsql-server-2005stored-procedurestransactions
jon333
jon333

Reputation: 831

Why does this SQL Server transaction always fail and how to fix the stored procedure structure?

I'm having some trouble with a stored procedure with a transaction always failing. I think it's because of IF EXISTS(SELECT 1 FROM TargetTable WHERE BaseTableId = @BaseTableId) or the way the code around that is structured but I am not sure how to fix the stored procedure so the transaction does not always fail. When I remove all the transaction code and do the same activities without a transaction in the stored procedure it works. All the updates and/or inserts succeed.

CREATE PROCEDURE [dbo].[proc_BaseTable_TargetTable_Update] 
(
@BaseTableId        bigint,
@BaseTableTypeId        bigint,
@Alias      nvarchar(max),
@TargetTableNonPkItemId bigint,
@OtherField     nvarchar(max) = NULL
)
AS
-- NOTE: There is a problem with enabling tractions on this, the trans always fails
-- with the current structure, seemingly because of the SELECT 1 FROM TARGETTABLE
BEGIN
BEGIN TRAN

    UPDATE BaseTable 
        SET 
            Alias = @Alias,
            BaseTableTypeId = @BaseTableTypeId,
            UpdatedOn = GETUTCDATE()
    WHERE BaseTableId = @BaseTableId

    IF @@ERROR <> 0
    BEGIN
        RAISERROR('Error updating BaseTable', 16, 1)
        ROLLBACK TRAN
        RETURN
    END

    IF EXISTS(SELECT 1 FROM TargetTable WHERE BaseTableId = @BaseTableId)
        UPDATE TargetTable
            SET
                TargetTableNonPkItemId = @TargetTableNonPkItemId,
                OtherField = @OtherField
        WHERE BaseTableId = @BaseTableId

        IF @@ERROR <> 0
        BEGIN
            RAISERROR('Error updating TargetTable', 16, 1)
            ROLLBACK TRAN
            RETURN
        END
    ELSE
        INSERT INTO TargetTable(BaseTableId, TargetTableNonPkItemId, OtherField)
        VALUES (@BaseTableId, @TargetTableNonPkItemId, @OtherField)

        IF @@ERROR <> 0
        BEGIN
            RAISERROR('Error inserting TargetTable', 16, 1)
            ROLLBACK TRAN
            RETURN
        END

COMMIT TRAN
END

Upvotes: 0

Views: 230

Answers (3)

HLGEM
HLGEM

Reputation: 96552

I prefer to use TRY CATCH for error handling. The example below will alos give a more useful error message.

CREATE PROCEDURE [dbo].[proc_BaseTable_TargetTable_Update]  
( @BaseTableId        bigint
, @BaseTableTypeId        bigint
, @Alias      nvarchar(max)
, @TargetTableNonPkItemId bigint
, @OtherField     nvarchar(max) = NULL ) 
AS  

BEGIN TRY
BEGIN TRAN      
    UPDATE BaseTable          
    SET Alias = @Alias
    , BaseTableTypeId = @BaseTableTypeId
    , UpdatedOn = GETUTCDATE()     
    WHERE BaseTableId = @BaseTableId           

IF EXISTS(SELECT 1 FROM TargetTable WHERE BaseTableId = @BaseTableId)         
    UPDATE TargetTable             
    SET TargetTableNonPkItemId = @TargetTableNonPkItemId
        , OtherField = @OtherField         
    WHERE BaseTableId = @BaseTableId         
ELSE         
    INSERT INTO TargetTable(BaseTableId, TargetTableNonPkItemId, OtherField)         
    VALUES (@BaseTableId, @TargetTableNonPkItemId, @OtherField)  

IF @@TRANCOUNT > 0
    BEGIN
        COMMIT TRAN
    END 
END TRY

BEGIN CATCH

    SELECT 
            ERROR_NUMBER() AS ErrorNumber
            ,ERROR_SEVERITY() AS ErrorSeverity
            ,ERROR_STATE() AS ErrorState
            ,ERROR_PROCEDURE() AS ErrorProcedure
            ,ERROR_LINE() AS ErrorLine
            ,ERROR_MESSAGE() AS ErrorMessage;

IF @@TRANCOUNT > 0
    BEGIN
        ROLLBACK TRAN
    END

END CATCH

Upvotes: 1

Justin Pihony
Justin Pihony

Reputation: 67075

You need to wrap your IF. What is happening in your code is that you expect something like this

IF EXISTS
{
   UPDATE
   RAISE ERROR IF BAD
}
ELSE
{
    INSERT
    RAISE ERROR IF BAD
}

But instead you end up with this

IF EXISTS
    UPDATE


IF ERROR
    RAISE ERROR
ELSE 
    INSERT


IF ERROR
    RAISE ERROR

So, here is what your code should look like:

CREATE PROCEDURE [dbo].[proc_BaseTable_TargetTable_Update] 
(
@BaseTableId        bigint,
@BaseTableTypeId        bigint,
@Alias      nvarchar(max),
@TargetTableNonPkItemId bigint,
@OtherField     nvarchar(max) = NULL
)
AS
-- NOTE: There is a problem with enabling tractions on this, the trans always fails
-- with the current structure, seemingly because of the SELECT 1 FROM TARGETTABLE
BEGIN
BEGIN TRAN

    UPDATE BaseTable 
        SET 
            Alias = @Alias,
            BaseTableTypeId = @BaseTableTypeId,
            UpdatedOn = GETUTCDATE()
    WHERE BaseTableId = @BaseTableId

    IF @@ERROR <> 0
    BEGIN
        RAISERROR('Error updating BaseTable', 16, 1)
        ROLLBACK TRAN
        RETURN
    END

    IF EXISTS(SELECT 1 FROM TargetTable WHERE BaseTableId = @BaseTableId)
    BEGIN
        UPDATE TargetTable
            SET
                TargetTableNonPkItemId = @TargetTableNonPkItemId,
                OtherField = @OtherField
        WHERE BaseTableId = @BaseTableId

        IF @@ERROR <> 0
        BEGIN
            RAISERROR('Error updating TargetTable', 16, 1)
            ROLLBACK TRAN
            RETURN
        END
    END
    ELSE
    BEGIN
        INSERT INTO TargetTable(BaseTableId, TargetTableNonPkItemId, OtherField)
        VALUES (@BaseTableId, @TargetTableNonPkItemId, @OtherField)

        IF @@ERROR <> 0
        BEGIN
            RAISERROR('Error inserting TargetTable', 16, 1)
            ROLLBACK TRAN
            RETURN
        END
    END

COMMIT TRAN
END

Upvotes: 3

Patrick
Patrick

Reputation: 5836

You should add a BEGIN/END block after the IF and after the ELSE because you have multiple statements in those blocks.

Upvotes: 2

Related Questions