Reputation: 127563
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
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