Database trigger to monitor insert/update events and update another table

Question

I am working on a MSSQL Database (for a program called Sage 200). There are many tables in the database, however, I want to be notified of changes (new record inserted or existing record updated) via Trigger on a specific table.

I also want to support multiple rows on this table being updated at same time also.

When a record is inserted or updated, I want to take a specific field from the table and insert/update another table with that field's value.

So, to put it into perspective; the helper stored prodc & sql trigger looks like this:

/**
 * New Stored Procedures
 */

IF EXISTS (SELECT * FROM sysobjects WHERE  id = object_id(N'[dbo].[SyncCustomerUpdateQueue]') AND OBJECTPROPERTY(id, N'IsProcedure') = 1)
BEGIN
    DROP PROCEDURE [dbo].[SyncCustomerUpdateQueue]
END
GO
CREATE PROCEDURE [dbo].[SyncCustomerUpdateQueue] 
    @SLCustomerAccountID INT,
    @TriggerSource VARCHAR(30)
AS 
    -- Init
    SET NOCOUNT ON;
    DECLARE @EventType VARCHAR(8);

    -- Check If Customer Doesn't Already Exist In Queue Table
    IF ((SELECT COUNT(*) FROM IC_CustomerUpdates WHERE SLCustomerAccountID = @SLCustomerAccountID) = 0)
    BEGIN

        -- Insert new record
        SET @EventType = '[INSERT]';
        INSERT INTO IC_CustomerUpdates (SLCustomerAccountID, Synced) VALUES
        (@SLCustomerAccountID, 0);

    END
    ELSE
    BEGIN

        -- Update existing record
        SET @EventType = '[UPDATE]';
        UPDATE IC_CustomerUpdates SET Synced = 0 
        WHERE SLCustomerAccountID = @SLCustomerAccountID;

    END

    -- Debug
    print @EventType + ' Customer Updates Queue Entry | SLCustomerAccountID : ' + CAST(@SLCustomerAccountID AS VARCHAR(255)) + ' | Trigger Source : ' + @TriggerSource;
GO

/*
 * New SAGE DB Triggers
 */

IF ((SELECT COUNT(*) FROM sys.triggers WHERE name = 'IC_CustomerChanges') > 0)
    DROP TRIGGER [dbo].[IC_CustomerChanges]
GO
CREATE TRIGGER [dbo].[IC_CustomerChanges] ON [dbo].[SLCustomerAccount]
AFTER INSERT, UPDATE
AS
BEGIN

    DECLARE @TriggerSource VARCHAR(30) = 'IC_CustomerChanges';

    SELECT RowNum = ROW_NUMBER() OVER(ORDER BY SLCustomerAccountID) , SLCustomerAccountID
    INTO #CustomerUpdates
    FROM INSERTED;

    DECLARE @MaxRownum INT;
    SET @MaxRownum = (SELECT MAX(RowNum) FROM #CustomerUpdates);

    DECLARE @Iter INT;
    SET @Iter = (SELECT MIN(RowNum) FROM #CustomerUpdates);

    WHILE @Iter <= @MaxRownum
    BEGIN

        -- Sync Customer Update Queue Entry
        DECLARE  @SLCustomerAccountID INT = (SELECT SLCustomerAccountID FROM #CustomerUpdates WHERE RowNum = @Iter);
        EXEC SyncCustomerUpdateQueue @SLCustomerAccountID, @TriggerSource;

        -- Next Row
        SET @Iter = @Iter + 1;

    END

    DROP TABLE #CustomerUpdates;

END
GO

IF ((SELECT COUNT(*) FROM sys.triggers WHERE name = 'IC_CustomerContactChanges') > 0)
    DROP TRIGGER [dbo].[IC_CustomerContactChanges]
GO
CREATE TRIGGER [dbo].[IC_CustomerContactChanges] ON [dbo].[SLCustomerContactValue]
AFTER INSERT, UPDATE
AS
BEGIN

    DECLARE @TriggerSource VARCHAR(30) = 'IC_CustomerContactChanges';

    SELECT RowNum = ROW_NUMBER() OVER(ORDER BY SLCustomerContactID) , SLCustomerContactID
    INTO #CustomerContactUpdates
    FROM INSERTED;

    DECLARE @MaxRownum INT;
    SET @MaxRownum = (SELECT MAX(RowNum) FROM #CustomerContactUpdates);

    DECLARE @Iter INT;
    SET @Iter = (SELECT MIN(RowNum) FROM #CustomerContactUpdates);

    WHILE @Iter <= @MaxRownum
    BEGIN

        -- Sync Customer Update Queue Entry
        DECLARE @SLCustomerAccountID INT = (SELECT TOP 1 CC.SLCustomerAccountID
                                            FROM SLCustomerContactValue AS CCV
                                            INNER JOIN SLCustomerContact AS CC ON CC.SLCustomerContactID = CCV.SLCustomerContactID
                                            WHERE CCV.SLCustomerContactID = (SELECT SLCustomerContactID FROM #CustomerContactUpdates WHERE RowNum = @Iter));
        EXEC SyncCustomerUpdateQueue @SLCustomerAccountID, @TriggerSource;

        -- Next Row
        SET @Iter = @Iter + 1;

    END

    DROP TABLE #CustomerContactUpdates;

END
GO

IF ((SELECT COUNT(*) FROM sys.triggers WHERE name = 'IC_CustomerLocationChanges') > 0)
    DROP TRIGGER [dbo].[IC_CustomerLocationChanges]
GO
CREATE TRIGGER [dbo].[IC_CustomerLocationChanges] ON [dbo].[SLCustomerLocation]
AFTER INSERT, UPDATE
AS
BEGIN

    DECLARE @TriggerSource VARCHAR(30) = 'IC_CustomerLocationChanges';

    SELECT RowNum = ROW_NUMBER() OVER(ORDER BY SLCustomerAccountID) , SLCustomerAccountID
    INTO #CustomerLocationChanges
    FROM INSERTED;

    DECLARE @MaxRownum INT;
    SET @MaxRownum = (SELECT MAX(RowNum) FROM #CustomerLocationChanges);

    DECLARE @Iter INT;
    SET @Iter = (SELECT MIN(RowNum) FROM #CustomerLocationChanges);

    WHILE @Iter <= @MaxRownum
    BEGIN

        -- Sync Customer Update Queue Entry
        DECLARE  @SLCustomerAccountID INT = (SELECT SLCustomerAccountID FROM #CustomerLocationChanges WHERE RowNum = @Iter);
        EXEC SyncCustomerUpdateQueue @SLCustomerAccountID, @TriggerSource;

        -- Next Row
        SET @Iter = @Iter + 1;

    END

    DROP TABLE #CustomerLocationChanges;

END
GO

This works as expected, however I have been informed that I am doing things incorrectly. How can I improve this?

Answer 1

Checking for existence

You check for existence by aggregating a COUNT() of values given a certain filter, and then continue if the count is zero:

-- Check If Customer Doesn't Already Exist In Queue Table
IF ((SELECT COUNT(*) FROM IC_CustomerUpdates WHERE SLCustomerAccountID = @SLCustomerAccountID) = 0) BEGIN...

All you really need to do is check that one row or more exists in this case, it shouldn't matter the count of rows if it's not zero. So, there are 2 ways of checking this more efficiently.

With EXISTS (my personal preference for clarity):

IF NOT EXISTS (SELECT 1 FROM IC_CustomerUpdates WHERE SLCustomerAccountID = @SLCustomerAccountID) BEGIN...

This works just as well by checking for NULL result set:

IF (SELECT 1 FROM IC_CustomerUpdates WHERE SLCustomerAccountID = @SLCustomerAccountID) IS NULL BEGIN...

Note the use of NOT EXISTS and IS NULL, the logic can also of course be reversed if you need to do something only if a row exists.

---

Nitpicks

This:

DECLARE @TriggerSource VARCHAR(30) = 'IC_CustomerChanges';

This never changes so it is a constant. T-SQL doesn't have a naming standard for constants, but in many other languages constants are written LIKE_THIS instead of LikeThis or likeThis (as variables might be). It's not a bad idea to also use this in SQL scripts to give a visual cue that this is not going to change. Also, since you know what the value is, make its type match that.

--In [dbo].[IC_CustomerChanges]
DECLARE @TRIGGER_SOURCE CHAR(18) = 'IC_CustomerChanges';
--In [dbo].[IC_CustomerContactChanges]
DECLARE @TRIGGER_SOURCE CHAR(25) = 'IC_CustomerContactChanges';

---

In your loops, T-SQL allows the use of += operator (this surprised me at first, too). Try this in a blank query:

DECLARE @foo INT = 1;
PRINT @foo;     --> 1
SET @foo += 1;
PRINT @foo;     --> 2
DECLARE @bar VARCHAR(20) = 'Hello';
PRINT @bar;     --> Hello
SET @bar += 'World';
PRINT @bar;     --> HelloWorld

So you can simply change your SET @Iter = @Iter + 1; to SET @Iter += 1;.

---

In your loops you declare a new variable on each iteration. There is no need to do that, just declare it once outside the loop, then just assign it a new value in the loop using SET (longer form) or SELECT (shorter form) like this:

DECLARE @SLCustomerAccountID INT;

WHILE @Iter <= @MaxRownum
BEGIN

    -- Sync Customer Update Queue Entry
    SELECT @SLCustomerAccountID = SLCustomerAccountID FROM #CustomerLocationChanges WHERE RowNum = @Iter;
    EXEC SyncCustomerUpdateQueue 
        @SLCustomerAccountID  = @_SLCustomerAccountID, 
        @TriggerSource = @TRIGGER_SOURCE;

    -- Next Row
    SET @Iter += 1;
END

---

Use explicit parameters with EXEC

For maintainability, readability, and future-proofing, I find it better to not implicitly rely on the order of the argument in the procedure's declaration, and rather to name them when it is called.

So for example this:

EXEC SyncCustomerUpdateQueue @SLCustomerAccountID, @TriggerSource;

Could be this:

EXEC SyncCustomerUpdateQueue 
    @SLCustomerAccountID  = @SLCustomerAccountID, 
    @TriggerSource = @TriggerSource;

Except, there's a little problem now, as both your variables and your parameters have the same name, and this will raise an error when you try to execute it this way. Now this simply becomes a question of naming the variables differently (by as little as one character difference). C# (and other) programmers often type an underscore in front of the variable name to indicate private variables.

DECLARE @_SLCustomerAccountID INT; --see underscore after the @ symbol

So doing this (as well as applying the above naming scheme for constants), our call now looks like this:

EXEC SyncCustomerUpdateQueue 
    @SLCustomerAccountID  = @_SLCustomerAccountID, 
    @TriggerSource = @TRIGGER_SOURCE;

So now if someone (including your future self who has to maintain this code) went and changed the procedure declaration at some point down the road, instead of possibly returning wrong results or writing wrong records, it will likely just raise an error and let you know right away that there is something wrong. It also makes the procedure calls easier to read/understand.