Scott Chamberlain
Scott Chamberlain

Reputation: 127563

Is it better to error and catch or check for existing?

I have two tables defined as

CREATE TABLE [dbo].[Foo](
    FOO_GUID uniqueidentifier NOT NULL PRIMARY KEY CLUSTERED,
    SECOND_COLUMN nvarchar(10) NOT NULL,
    THIRD_COLUMN nvarchar(10) NOT NULL)

CREATE TABLE [dbo].[FooChanged](
    [FooGuid] uniqueidentifier NOT NULL PRIMARY KEY CLUSTERED,
    [IsNewRecord] bit NOT NULL)

And a trigger

CREATE TRIGGER dbo.[trg_FooChanged] 
   ON  [dbo].[Foo] 
   AFTER INSERT,UPDATE
   NOT FOR REPLICATION
AS 
BEGIN
    SET NOCOUNT ON;

    DECLARE @isNewRecord as bit;

    IF EXISTS(SELECT * FROM DELETED)
    BEGIN
       --We only want to make a record if the guid or one other column in Foo changed.
        IF NOT(UPDATE(FOO_GUID) OR UPDATE(SECOND_COLUMN))
            RETURN;

        SET @isNewRecord = 0
    END
    ELSE
        SET @isNewRecord = 1;

    insert into FooChanged(FooGuid, IsNewRecord) SELECT FOO_GUID, @isNewRecord from INSERTED

END

The following simple test script fails on the last batch with a primary key constraint violation (as expected)

INSERT INTO [dbo].[Foo] ([FOO_GUID],[SECOND_COLUMN],[THIRD_COLUMN]) VALUES (cast(0x1 as uniqueidentifier), '1', '1')
GO
INSERT INTO [dbo].[Foo]([FOO_GUID],[SECOND_COLUMN],[THIRD_COLUMN]) VALUES (cast(0x2 as uniqueidentifier), '2', '2')
GO
UPDATE Foo SET THIRD_COLUMN = '1a' WHERE FOO_GUID = cast(0x1 as uniqueidentifier)
GO
UPDATE Foo SET SECOND_COLUMN = '1a' WHERE FOO_GUID = cast(0x1 as uniqueidentifier)
GO

My question is, which is the "proper" way to not have that error propagate to the user and not mess up any current transactions the user may have (with potentially settings like XACT_ABORT ON).

The two options I see is either check before inserting

    declare @guid uniqueidentifier
    select @guid = FOO_GUID from INSERTED

    BEGIN TRANSACTION       
    if NOT EXISTS(select * from FooChanged WITH (UPDLOCK, HOLDLOCK) where FooGuid = @guid)
        insert into FooChanged(FooGuid, IsNewRecord) SELECT @guid, @isNewRecord from INSERTED
    COMMIT TRANSACTION

But I will need to lock on the table to prevent any race conditions which I think will cause performance problems on busy tables

or catch the error in a TRY/CATCH

    BEGIN TRY
        insert into FooChanged(FooGuid, IsNewRecord) SELECT @guid, @isNewRecord from INSERTED
    END TRY
    BEGIN CATCH
        DECLARE @ErrorMessage NVARCHAR(4000);
        DECLARE @ErrorSeverity INT;
        DECLARE @ErrorState INT;

        SELECT 
            @ErrorMessage = ERROR_MESSAGE(),
            @ErrorSeverity = ERROR_SEVERITY(),
            @ErrorState = ERROR_STATE();

        if(ERROR_NUMBER() != 2627)
            RAISERROR (@ErrorMessage, -- Message text.
                       @ErrorSeverity, -- Severity.
                       @ErrorState -- State.
           );
    END CATCH

But I am concerned about code running inside a XACT_ABORT transaction getting it's XACT_STATE mucked with.

What is the correct approach to use?

Upvotes: 0

Views: 59

Answers (1)

D Stanley
D Stanley

Reputation: 152566

What is the correct approach to use?

Check for existence, then insert or update accordingly.

Error handling is expensive - and once an error is thrown there's no way to go back and do something different.

EDIT

Honestly, I may be wrong on the "expensive" part (and don't have any evidence to back it up) as I'm not a SQL expert, but the principle of not being able to go back sill applies.

Here's an article from Aaron Bertrand, who is a much better source on SQL than I am. At first glance it seems to indicate that neither approach is significantly better than the other performance wise.

Upvotes: 3

Related Questions