Thread: Insert/Delete Trigger Misfires
01-06-06, 16:54 #1Registered User
- Join Date
- Jan 2003
- Pittsburgh, PA
Unanswered: Insert/Delete Trigger Misfires
I am having problems with a trigger that is designed to audit changes to a particular field in a table. If that field is updated, then the old record is inserted into an audit table.
This trigger never fails when I run test data against it from Query Analyzer. It works some of the time when the web application updates it, fails other times.
Typically, multiple records are updated at the same time. Any ideas?
Here is the Trigger:
create trigger t_u_product_rate_detail
for insert, update, delete
--Set values so function isn't executed a bunch of times
@auditdate = getdate(),
@audituser = suser_sname()
if exists (select * from inserted)
if exists (select * from deleted)
insert into product_rate_detail_audit_log
@auditdate, @audituser, 'U'
from deleted d
join inserted i on i.product_rate_detail_id = d.product_Rate_detail_id
where (d.rate <> 0 and d.rate is not null)
and i.rate <> d.rate -- this determines if the rate has changed.
SET QUOTED_IDENTIFIER OFF
SET ANSI_NULLS ON
01-07-06, 00:01 #2World Class Flame Warrior
Provided Answers: 1
- Join Date
- Jun 2003
Lots of stuff wrong with this. Heres some things to consider:
Your trigger is for insert/update/delete, but this criteria:Code:
if exists (select * from inserted) begin if exists (select * from deleted) begin
You really don't need to check for the existence of data in the inserted and deleted tables anyway. If you reference them as a source of data in a statement and they are empty, then your statement will just not do anything. So drop the "if exists" clauses completely.
If you just want to capture updates and deletes then you need only reference the deleted table. The inserted table contains the new values, and looks like you are not archiving those (until they themselves are updated).
You can also drop the @AuditDate and @AuditUser variables, and just reference getdate() and suser_sname() directly in your update statement. suser_sname() is constant throughout the transaction, and unless you have a truly massive update then getdate() will return a consistent value across all affected records as well.
Dropping your exists clauses and your unnecessary variable declarations will simplify your code, and simpler is always better.
01-07-06, 16:41 #3Registered User
- Join Date
- Jul 2003
- San Antonio, TX
Well, let me bite ...
First, check for existence is always a good idea, simply because attempting to perform an operation on an empty set also has its cost and contributes to resource contention. Besides the trigger will get fired even if 0 rows are updated.
Second, if you all want to use best practices, - do not perform mass updates, so that the trigger does not have to kill the server while processing millions of rows. Also, (and this is truly the best practice point) - read your virtual tables only once, because this opration in itself is extremely expensive. In order to satisfy this requirement, - select * into #tmp from deleted!
Third, - remove references to INSERT and DELETE in the trigger definition. UPDATE occurs only when a record is updated, not when it is deleted or inserted (unless your app performs UPDATE by issueing DELETE+INSERT)."The data in a record depends on the Key to the record, the Whole Key, and
nothing but the Key, so help me Codd."