Pimp my Trigger? 1

[KirbyWallace]

Can anyone tell me if the following update trigger is the most efficient way to do what I'm doing? or is there a better way? I'm particularly concerned that the way I'm doing this requires me to make three selects to get three columns from the same table (deleted).

ALTER TRIGGER tbl_User_Update_SAC
ON tbl_User
AFTER UPDATE
AS

IF NOT UPDATE(Security_Access_Code)
RETURN

IF EXISTS (SELECT * FROM inserted a JOIN deleted b ON a.UserRecordID = b.UserRecordID
WHERE a.Security_Access_Code <> b.Security_Access_Code)

BEGIN

INSERT tbl_Audit_Log (UserSessionID, UserRecordID, Action_Type, Action_Desc)

VALUES (
Null, Null, 'Internal Audit',
'UserRecordID:' + cast((SELECT UserRecordID FROM inserted) as varchar(16)) +

' SAC CHANGED FROM: ' + (SELECT Security_Access_Code FROM deleted) +

' TO:' + (SELECT Security_Access_Code FROM inserted)
)
END


I've entered some blank lines for readability that are not there in the actual trigger code. This is working just fine, but I wonder if I'm going about it wrong or inefficiently.

Is there a better way? Or is this the standard way?

Thanks,

Kirby

 

[SQLSister]

This is a very bad way to write the trigger. If there is a multiple insert, you will not get all records in your audit table. You need to use a select stament instead of the values clause. Never use the values clause in a trigger becasue it only processes the values for one record and multiple records can be in a batch.
select would be something like:

select Null, Null, 'Internal Audit',
'UserRecordID:' + cast(i.UserRecordID as varchar(16)) + 
' SAC CHANGED FROM: ' + (d.Security_Access_Code) + 
' TO:' + (i.Security_Access_Code)
from inserted i 
join deleted d on i.UserRecordID = d.UserRecordID
WHERE d.Security_Access_Code <> d.Security_Access_Code

"NOTHING is more important in a database than integrity." ESquared

 

[KirbyWallace]

Thanks,

A couple of q's about your suggestion, if I may.

The UserRecordID is not available during this operation. It's used int he application when the application is writing to the audit log. I supply a ZERO for this trigger's purposes. I could just as well leave it NULL. So I can't use UserRecordID in the WHERE clause.

That brings me to this other question: What ARE the inserted and deleted table refs? Are they the ENTIRE new and old tables? or are they just the inserted and deleted ROW?

And finally, how would I go about taking the select that you have suggested and inserting that into the audit log? I'm thinking, something like:

INSERT tbl_Audit_Log SELECT (your select recommendation here)

Thanks, Much

 

[KirbyWallace]

To partially answer my own question, here's what I did:

I changed the trigger (just for a moment) to insert the rowcount from "inserted" and "deleted" in place of the zeros I was supplying:


INSERT tbl_Audit_Log (UserSessionID, UserRecordID, Action_Type, Action_Desc)

VALUES ((select count(*) from inserted), (select count(*) from deleted),... etc etc...)

These two table references always have exactly ONE row.

So the inserted and deleted references contain the one ROW that is being modified.

So, I guess my question has now become, "What's the use of a WHERE clause in an update trigger when selecting from the inserted or deleleted table references?"

So many questions. So little Time. Thanks SQLSister.

 

[SQLSister]

No no no. Do not use the values clause. It will not work in a multi record change. The two table references may not always have exactly one row. You cannot ever under any circumstances write a trigger that depends on there being only one row. Suppose someone needed to change a group of users at the same time. This will happen sooner or later and must be planned for. This is not a nice to have; this is a requirement.

Never ever use the values clause in a trigger.

the general syntax to use a select instead is
insert table (filed1, field2)
select field1, 'test' from table2

Try my select and then modify it until you get what you need.

"NOTHING is more important in a database than integrity." ESquared