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
on product_rate_detail
for insert, update, delete

as

/--Local variable
declare
@.auditdate datetime,
@.audituser sysname

--Set values so function isn't executed a bunch of times
select
@.auditdate = getdate(),
@.audituser = suser_sname()

if exists (select * from inserted)
begin
if exists (select * from deleted)
begin
insert into product_rate_detail_audit_log
select d.product_rate_detail_id,
d.product_rate_id,
d.day_of_week_id,
d.ad_size_id,
d.rate,
d.plan_vol,
d.plan_freq,
@.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.
end
end

GO
SET QUOTED_IDENTIFIER OFF
GO
SET ANSI_NULLS ON
GOLots of stuff wrong with this. Heres some things to consider:
Your trigger is for insert/update/delete, but this criteria:if exists (select * from inserted)
begin
if exists (select * from deleted)
begin...will cause your insert statement to execute only on updates, because that is the only occasion when data exists in both inserted and deleted tables.
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.|||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).