Problem with Trigger in SQL 2005

Problem with Trigger in SQL 2005

Old forum URL: forums.lhotka.net/forums/t/4138.aspx


eslater posted on Wednesday, January 09, 2008

I have two classes Invoice and InvoiceItem which are parent/child relationship and therefore are part of a transaction.  I have a Trigger on AFTER INSERT on InvoiceItem that updates a Rate table with the avg monthly usage.  This trigger is not updating the table and I cant figure out why.  Here is the Trigger:

ALTER TRIGGER [tr_LDInvoiceItem_Insert]
   ON  dbo.LDInvoiceItem
   AFTER INSERT
AS
BEGIN
    -- SET NOCOUNT ON added to prevent extra result sets from interfering with SELECT statements.
    SET NOCOUNT ON;

    -- Bail on this if there is nothing in the inserted table
    if not exists(select * from inserted)
        return

    DECLARE @InvoiceID uniqueidentifier
    DECLARE @LDAccountID uniqueidentifier
    DECLARE @CountryCode nvarchar(2)
    DECLARE @AvgInboundSeconds int
    DECLARE @AvgOutboundSeconds int
    DECLARE @AvgMobileSeconds int

    SELECT @InvoiceID = InvoiceID FROM INSERTED
    SELECT @CountryCode = CountryCode FROM INSERTED

    SELECT @LDAccountID = (SELECT LDAccountID FROM LDInvoice WHERE InvoiceID = @InvoiceID)

    SELECT @AvgInboundSeconds = (SELECT SUM(InboundSeconds) / COUNT(*) As [AvgUsage] FROM LDInvoiceItem INNER JOIN LDInvoice ON LDInvoiceItem.InvoiceID = LDInvoice.InvoiceID WHERE LDAccountID = @LDAccountID AND CountryCode = @CountryCode)
    SELECT @AvgMobileSeconds = (SELECT SUM(MobileSeconds) / COUNT(*) As [AvgUsage] FROM LDInvoiceItem INNER JOIN LDInvoice ON LDInvoiceItem.InvoiceID = LDInvoice.InvoiceID WHERE LDAccountID = @LDAccountID AND CountryCode = @CountryCode)
    SELECT @AvgOutboundSeconds = (SELECT SUM(OutboundSeconds) / COUNT(*) As [AvgUsage] FROM LDInvoiceItem INNER JOIN LDInvoice ON LDInvoiceItem.InvoiceID = LDInvoice.InvoiceID WHERE LDAccountID = @LDAccountID AND CountryCode = @CountryCode)
   
    UPDATE
        LDRate
    SET
        AvgInboundSeconds = @AvgInboundSeconds,
        AvgMobileSeconds = @AvgMobileSeconds,
        AvgOutboundSeconds = @AvgOutboundSeconds
    WHERE
        LDAccountID = @LDAccountID
    AND CountryCode = @CountryCode
END

The only thing I can see that might be the problem is the line
SELECT @LDAccountID = (SELECT LDAccountID FROM LDInvoice WHERE InvoiceID = @InvoiceID) which relies on the Invoice table already having been updated.  Could this be the problem and if so how can I get around it?

Thanks for any help.

Elliot

JoeFallon1 replied on Wednesday, January 09, 2008

Could the transaction be interfering with the trigger?

Can you remove the transaction code and then see if the trigger works?

Joe

 

DavidDilworth replied on Thursday, January 10, 2008

Triggers!

Ick! [+o(]

esteban404 replied on Friday, January 11, 2008

The assignment may not be working as expected. Try this type of setting instead of assignment:

SET @InvoiceID = (SELECT InvoiceID FROM INSERTED)

I've noted some differences in getting to the inserted table which sometimes required a JOIN on the main table to the inserted. I don't think that's the prob here.

All the select statements are returning something also, but they aren't OUT type parameters, so are they being used outside of the trig? May just bloat the result sets that way.

HTH,

_E

JonnyBee replied on Tuesday, January 15, 2008

Are you sure that there will always be an existing row in LDRate for all given AccountID and CountryCode at the time the trigger is executed? If not the update will rn OK but 0 records is updated.

And remember the following too:
- triggers may be executed with more than 1 affected row. If your INSERTED table has  more than one - 1 row your trigger will fail.
- performance should also be considered

    SELECT
          @AvgInboundSeconds =  AVG(InboundSeconds),
          @AvgMobileSeconds = AVG(MobileSeconds),
          @AvgOutboundSeconds = AVG(OutboundSeconds)
    FROM LDInvoiceItem
        INNER JOIN LDInvoice ON LDInvoiceItem.InvoiceID = LDInvoice.InvoiceID
    WHERE LDAccountID = @LDAccountID
      AND CountryCode = @CountryCode

will run just once instead of 3 times on potentially large tables.

Jonny

xplinscott replied on Thursday, January 17, 2008

If I understand your schema correctly you might be better of doing something more like this:

IF NOT EXISTS (SELECT 1 FROM Inserted I)
   RETURN

BEGIN TRAN

UPDATE LDRate
SET 
      AvgInboundSeconds
= T.AvgInboundSeconds
,     AvgMobileSeconds = T.AvgMobileSeconds
,     AvgOutboundSeconds = T.AvgOutboundSecond
FROM LDRate L
JOIN (
   
SELECT 
      I
.InvoiceID
   ,  I.CountryCode
   ,  L.LDAccountID
   ,  [AvgInboundSeconds] = SUM(LI.InboundSeconds) / COUNT(LI.InvoiceID)
   ,  [AvgMobileSeconds] = SUM(LI.MobileSeconds) / COUNT(LI.InvoiceID)
   ,  [AvgOutboudSeconds] = SUM(LI.OutboundSeconds) / COUNT(LI.InvoiceID)
   
FROM Inserted I
   
JOIN LDInvoice L (NOLOCK)
         
ON I.InvoiceID = L.InvoiceID
         
AND I.CountryCode = L.CountryCode
   
JOIN LDInvoiceItem LI (NOLOCK)
         
ON L.InvoiceID = LI.InvoiceID
   
GROUP BY I.InvoiceID, I.CountryCode, L.LDAccountID
) AS T
      
ON L.LDAccountID = T.LDAccountID
      
AND L.CountryCode = T.CountryCode

IF (@@ERROR = 0)
      
COMMIT TRAN
ELSE
   
BEGIN
      
RAISERROR('Some Error Message...',16,1)
      
ROLLBACK TRAN
   
END

This should allow for more than one row to be in the inserted table and should be a bit of better performance. Definitely test it before using it.

Copyright (c) Marimer LLC