1

Having read this page I having built a couple of tables and a trigger. The idea is that when an INSERT, UPDATE or DELETE is performed on the first table Matt the data operated upon will be inserted into the second, audit, table MattAudit.

The trigger must be failing and I don't know why; The evidence is that there is no entry made in the audit table, though the CREATE TRIGGER and subsequent ALTER TRIGGER statements complete successfully.

Main table Matt:

CREATE TABLE [dbo].[Matt](
    [MattID] [int] IDENTITY(1,1) NOT NULL,
    [Text] [nchar](10) NULL,
 CONSTRAINT [PK_Matt] PRIMARY KEY CLUSTERED 
(
    [MattID] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]

Audit table MattAudit:

CREATE TABLE [dbo].[MattAudit](
    [MattAuditID] [int] IDENTITY(1,1) NOT NULL,
    [MattID] [int] NOT NULL,
    [Text] [nchar](10) NULL,
    [Action] [int] NOT NULL,
    [InsertedDate] [datetime] NOT NULL,
 CONSTRAINT [PK_MattAudit] PRIMARY KEY CLUSTERED 
(
    [MattAuditID] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]

Trigger on Matt:

ALTER TRIGGER TrgMattAudit ON Matt
AFTER INSERT, UPDATE, DELETE
AS
BEGIN
SET NOCOUNT ON;

    INSERT INTO [dbo].[MattAudit]
        ( MattID, [Text], [Action], InsertedDate )
    SELECT
        ISNULL(i.MattID, d.MattID) as MattID,
        ISNULL(i.Text, d.Text) as Text,
        CASE ISNULL(i.MattID,0) WHEN 0 THEN 0 ELSE 1 END
        +
        CASE ISNULL(d.MattID,0) WHEN 0 THEN 0 ELSE -1 END
        as [Action],
        GETDATE() as InsertedDate
    FROM
        inserted i
        INNER JOIN deleted d ON i.MattID = d.MattID;

END

The following insert statement will insert rows into the Matt table but nothing appears in the MattAudit table.

INSERT INTO Matt ([Text]) VALUES ('Test4')

What am I missing or getting wrong in the trigger?

Matt W
  • 11,753
  • 25
  • 118
  • 215
  • Using triggers for audit tables might not be the best idea. There are built in tools in SQL Server like capture data change or temporal tables that can give you a better solution. – Zohar Peled Sep 05 '19 at 11:13
  • @ZoharPeled Temporal tables also work for deleted rows? – Radu Gheorghiu Sep 05 '19 at 11:16
  • 1
    Yes. Temporal tables (A.K.A system-versioned tables) was introduced in SQL Server 2016 - the idea is that you have one table that stores the current data, and one table that stores the history of that data - and they are automatically linked - any change made in the main table will result with the current row automatically pushed to the history table - including updates and deletes. https://learn.microsoft.com/en-us/sql/relational-databases/tables/temporal-tables?view=sql-server-2017 – Zohar Peled Sep 05 '19 at 11:20
  • If you're working with an older version, you can use Change Data Capture (which IIRC was available since 2008) https://learn.microsoft.com/en-us/sql/relational-databases/track-changes/about-change-data-capture-sql-server?view=sql-server-2017 – Zohar Peled Sep 05 '19 at 11:22
  • Yep, [I remembered correctly...](https://www.red-gate.com/simple-talk/sql/learn-sql-server/introduction-to-change-data-capture-cdc-in-sql-server-2008/) – Zohar Peled Sep 05 '19 at 12:32
  • @zohar-peled This might be a dumb question, but something in the way the doc you linked to describes 'current' tables makes me think they need to be defined as temporal at point of creation. I want to store auditing data for a table which has been in use for years - is that possible using a new application of a temporal history table? – Matt W Sep 05 '19 at 13:00
  • @MattW you mean you have a table containing historical data and you want to use that as the history table of a temporal set? I honestly don't know if that's possible. I know you can manually create the history table, but I don't know if it can be pre-populated. I do know that once it's referenced by another table as it's history table, it is readonly - you can't update or delete records from it, nor can you insert records directly to it. – Zohar Peled Sep 05 '19 at 13:04
  • 1
    Aside: As a rule a logging trigger should be set to fire _last_ using [`sp_settriggerorder`](https://learn.microsoft.com/en-us/sql/relational-databases/system-stored-procedures/sp-settriggerorder-transact-sql?view=sql-server-2017). There's no point in logging actions that a validation trigger might rollback. – HABO Sep 05 '19 at 14:20

2 Answers2

2

I think the problem is because of this:

FROM
    inserted i
    INNER JOIN deleted d ON i.MattID = d.MattID;
  • When inserting you only have records in INSERTED.
  • When you delete records you only have records in DELETED.
  • When you update records you will have records in both INSERTED (new value) and DELETED (old value).

Joining the two will always result in 0 rows for INSERT or DELETE, because when doing an INSERT you will have 1 or more rows in INSERTED, but 0 rows in DELETED. And vice versa for the DELETE statement.

A suggestion I would make is to split the single trigger into a trigger for each situation (INSERT, UPDATE and DELETE) and have a single query in each of your new triggers.

A small caveat is that an AFTER UPDATE trigger will add rows in both INSERTED and DELETED table.

The value in INSERTED will be the value which was put in place, and the value in DELETED will be the old value, before the UPDATE query ran.

Radu Gheorghiu
  • 20,049
  • 16
  • 72
  • 107
  • 1
    When you are updating records, both `inserted` and `deleted` tables will contain these records - `inserted` will contains the records after the update, and `deleted` will contain the records before the update.Also, a `union all` operator will work on select statements, but not on insert statements. – Zohar Peled Sep 05 '19 at 11:30
  • Also, an after update trigger will be fired even if the update statement updated the values in the row to the same values already existing. Suppose your `Text` column contains `Matt` in the row where the id is 1, and you run `update Matt set text = 'Matt' where id = 1` - that update statement would still fire the update trigger. – Zohar Peled Sep 05 '19 at 11:46
  • I see you've edited your answer, but for the most parts of it it's still wrong... I do absolutely agree that this shouldn't be handled in a single trigger, though. – Zohar Peled Sep 05 '19 at 12:00
  • @ZoharPeled I've trimmed it down. I'll proceed to cleanup the comments, since there's no need for them. When the last comment is gone, this one will go too. – Radu Gheorghiu Sep 05 '19 at 12:03
  • 1
    "When inserting or updating you only have records in INSERTED." should be "When inserting you only have records in INSERTED." Also, "Joining the two will always result in 0 rows" should be "Joining the two will result in 0 rows for insert or delete"... – Zohar Peled Sep 05 '19 at 12:28
  • Now it's better :-) +1. – Zohar Peled Sep 05 '19 at 12:31
0

I think the error you are doing there is to do an inner join between inserted and deleted tables. As never should be same MattId in the 2 tables. I mean, you should do outer joins like left or right (Or load variables in 2 different selects instead of doing 1). Try that, hope you understood me!

Jon
  • 166
  • 6
  • When you are updating records, both `inserted` and `deleted` tables will contain these records - `inserted` will contains the records after the update, and `deleted` will contain the records before the update. – Zohar Peled Sep 05 '19 at 11:30
  • Thats true but he wanted to audit everytime he inserts,updates or deletes so in the inserted and deletes cases his audit will fail. – Jon Sep 06 '19 at 07:15