1

I am using the following trigger to track inserts and updates on multiple tables and log it in a log table.

CREATE TRIGGER tr_TestTable1]
ON [TestTable_1]
AFTER INSERT, UPDATE
AS
DECLARE @keyid int, @tn nvarchar(50), @recEditMode nvarchar(50), @trstat nvarchar(50)
BEGIN
    SET NOCOUNT ON;   
    SET @tn = 'TestTable_1'
    IF EXISTS(SELECT 1 FROM INSERTED)
    BEGIN
    SET @recEditMode = (Select REC_EDIT_MODE FROM inserted)
    SET @trstat = 'PENDING'
    SET @keyid = (Select prkeyId FROM inserted)

    IF (@recEditMode = 'MANUAL')
    BEGIN
    IF NOT EXISTS (SELECT * FROM [logTable_1] WHERE SourceKeyId = @keyid AND TrStatus = 'PENDING' AND SourceTableName = @tn)
        BEGIN
            INSERT INTO [logTable_1](SourceKeyId,SourceTableName,TrStatus)
                VALUES (@keyid, @tn, @trstat)       
        END
    END
        END

END

This works fine on single row insert and single row update. I am unable to optimize this code to handle multi row inserts and updates. Looking for some help in handling this.

Thanks.

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
user2208460
  • 11
  • 1
  • 2
  • Which RDBMS is this for? For triggers, it makes a **huge** difference whether you're using MySQL, PostgreSQL, Oracle, SQL Server or IBM DB2 - or something else even. Please add a relevant tag to your question! – marc_s Jul 08 '16 at 04:56
  • 2
    If this is for **SQL Server**, then your trigger has a **MAJOR** flaw: you assume it'll be called **once per row** - that is **NOT** the case. The trigger will fire **once per statement**, so if your `INSERT` statement inserts 25 rows, the trigger fires **once** and `Inserted` pseudo table will contain 25 rows. Which of those 25 rows will your code select here?? `Select prkeyId FROM inserted` - it's non-deterministic, you'll get **one arbitrary row** and you will be **ignoring all other rows**. You need to rewrite your trigger to take this into account! – marc_s Jul 08 '16 at 04:56
  • Sorry for incomplete info. The database is SQL Server and i did realise the trigger will fire per statement. I was trying to track all the rows which meet a certain criteria (recEditMode = 'MANUAL') and log them into a separate table. I also need to capture the table name as i will be using the same trigger on multiple tables. Being new to this, i was looking for some help to rewrite the trigger to account for all the affected rows. – user2208460 Jul 08 '16 at 15:16
  • "We do not recommend using cursors in triggers because they could potentially reduce performance. To design a trigger that affects multiple rows, use rowset-based logic instead of cursors." https://msdn.microsoft.com/en-us/library/ms190752.aspx – Ryan Gavin Jan 25 '17 at 10:30

1 Answers1

0

I modified the trigger as below and it seems to be working fine now...

 CREATE TRIGGER tr_TestTable1]
    ON [TestTable_1]
    AFTER INSERT, UPDATE
    AS
    DECLARE @keyid int, @tn nvarchar(50), @trstat nvarchar(50)
    BEGIN
        IF @@ROWCOUNT = 0 
        RETURN
        SET NOCOUNT ON;   
        IF EXISTS(SELECT * FROM INSERTED)
        BEGIN
        SET @tn = 'TestTable_1'
        SET @trstat = 'PENDING'
        BEGIN
        INSERT INTO LogTable_1 (SourceKeyId, SourceTableName, TrStatus)
        SELECT I.prKeyId, @tn, @trStat FROM INSERTED AS I
        WHERE (I.REC_EDIT_MODE = 'MANUAL' AND NOT EXISTS(SELECT * FROM LogTable_1 WHERE SourceKeyId =   I.prKeyId AND SourceTableName = @tn AND TrStatus = 'PENDING'))

        END
    END
END
user2208460
  • 11
  • 1
  • 2