Need a spot-check on this trigger

  • I wrote this trigger (be gentle, I'm a newb) for our business application, to alert the "order taker" when the enter a "required date" that is the same as the "order date"

    The trigger executes correctly with no errors reported. When I do something in the system that should set off the trigger I don't get any error messages or notices on the screen, it just never sends the email.

    I know my DBMail is working because other triggers are sending emails.

    Here's my code - any assistance is greatly appreciated!!!

    CREATE TRIGGER [No_REQ_DATE_TRIGGER] ON [dbo].[oe_hdr]

    FOR UPDATE

    AS

    DECLARE

    @OrderNo as varchar(8),

    @CustomerID as varchar(19),

    @TakerID as varchar(15),

    @TakerEmail as varchar(40),

    @OrderDate datetime,

    @RequiredDate datetime,

    --email variables

    @Recipients as varchar(50),

    @Subject as varchar(80),

    @BillToName as varchar(80)

    SELECT @OrderNo = order_no,

    @CustomerID = customer_id,

    @OrderDate = order_date,

    @RequiredDate = required_date

    FROM p21_order_view

    SELECT @TakerID = taker

    FROM p21_order_view

    WHERE order_no = @OrderNo

    SELECT @TakerEmail = email_address

    FROM users

    WHERE id = @TakerID

    SELECT @Subject = 'Order #' + @OrderNo + ' for ' + @CustomerID + ' does not have a required date set'

    IF @@ROWCOUNT = 0 RETURN

    IF @TakerEmail = '' or @TakerEmail is null RETURN

    IF @OrderDate = @RequiredDate

    BEGIN

    EXEC msdb.dbo.sp_send_dbmail

    @profile_name = 'Alerts',

    @Recipients = @TakerEmail,

    @importance ='High',

    @Subject = @Subject;

    END

  • I noticed a couple things that might help

    The trigger is on oe_hdr, but you never reference that table, nor the INSERTED or DELETED tables. So the trigger will execute when a record is updated in oe_hdr, but the action will not be specific to the records changed. It will also only fire when a record is changed, not when a new one is inserted (that may still be what you want).

    The SELECT from p21_order_view that set local variables may return multiple rows, but the variables will only hold information from the LAST row selected. Since you don't have an order by nor a where clause, you'll never be sure exactly which row that is. This is my guess as to why the email is not being sent... you are probably updating a row in the middle of the table, and the variables are being set to the values for a row at the end (keep in mind that without an order by, the row at the "end" isn't necessarily the last one you entered - it depends entirely on how the optimizer decided to scan through the table).

    I'm going to take a wild guess here... I'm guessing that p21_order_view is a view that includes the oe_hdr table. If that's the case, what you really need to do is either use the INSERTED and DELETED tables directly, or at least join them back to the view so you only get the rows you specifically wanted. Since more than one row might be updated, you need to take that into account as well and send the mail out for each one (as it is written now, if you updated every record in the table, you would only get one email, not one for each), or send a query result instead of a message.

    Good Luck!

    Chad

  • Thanks for the feedback Chad, I'm going to try re-writing the trigger to point at the tables rather than the view and see what happens.

  • Chad has the right idea. I'd actually not send email from a trigger as that puts the email inside the transaction, which isn't something you might want to do. Typically I'd write these values in a table and then have a job run every minute or two to pick them up and send the emails.

    That being said, chad has the right advice. First, don't use local variables. If 2 rows are updated, you might not get what you want.

    I think you'd want to do soemthing like

    select orderno

    from inserted

    where required_date = enter_date

    to get the orders that need to be flagged. You can use this list to send to the people that need to know.

  • Actually it turns out that my original trigger was too precise. Our app includes hr/min/sec in the date records so 01/01/08 really becomes 01/01/08 12:00:00

    To get around that problem I was given a new query:

    select order_no, left(cast(order_date as varchar(26)),11), left(cast(requested_date as varchar(26)),11) from oe_hdr

    where left(cast(order_date as varchar(26)),11) = left(cast(requested_date as varchar(26)),11)

    This one returns the correct results so I'd need to adjust the trigger to use this - I have no idea what the correct format is on the DECLARE statement.

    Here's my most recent trigger revisions, if anyone could help me hack it up to use the new query it would be greatly appreciated!

    CREATE TRIGGER [No_REQ_DATE_TRIGGER] ON [dbo].[oe_hdr]

    FOR UPDATE

    AS

    DECLARE

    @OrderNo as varchar(8),

    @CustomerID as varchar(19),

    @TakerID as varchar(15),

    @TakerEmail as varchar(40),

    @OrderDate datetime,

    @RequiredDate datetime,

    --email variables

    @Recipients as varchar(50),

    @Subject as varchar(80),

    @BillToName as varchar(80)

    SELECT @OrderNo = order_no,

    @CustomerID = customer_id,

    @OrderDate = order_date,

    @RequiredDate = requested_date

    FROM oe_hdr

    SELECT @TakerID = taker

    FROM oe_hdr

    WHERE order_no = @OrderNo

    SELECT @TakerEmail = email_address

    FROM users

    WHERE id = @TakerID

    SELECT @Subject = 'Order #' + @OrderNo + ' for ' + @CustomerID + ' does not have a required date set'

    IF @@ROWCOUNT = 0 RETURN

    IF @TakerEmail = '' or @TakerEmail is null RETURN

    IF @OrderDate = @RequiredDate

    BEGIN

    EXEC msdb.dbo.sp_send_dbmail

    @profile_name = 'Alerts',

    @Recipients = @TakerEmail,

    @importance ='High',

    @Subject = @Subject;

    END

  • Again - look at some of the comments Steve put in. You REALLY don't want to assume you will be updating just one row at a time. You never can tell what might happen, and if you get multiple rows involved - you script will fail. Remember - triggers fire once per BATCH of updates, not once per ROW.

    So - if it should happen that 4 records at the same time, the trigger would fire just once, and your trigger will fire up an error.

    Also - sending mail from within a trigger is a bad, bad idea. If at any time, your e-mail server starts having a bad day - the sp_send_dbmail will FAIL, causes the TRIGGER to fail, which will force the update to be rolled back or cancelled. No - not just the email, the changes to the records will be reversed.

    If your e-mail server has a SLOW day (or rather - it happens to be slow because it's having a BUSY day), the lock on that row will be maintained WHILE the e-mail is processed, which might take a while.

    ----------------------------------------------------------------------------------
    Your lack of planning does not constitute an emergency on my part...unless you're my manager...or a director and above...or a really loud-spoken end-user..All right - what was my emergency again?

  • I see what you mean about not running the email directly from the trigger. I'm not sure how to go about setting up temp tables and running it as a job.... I'm still too much of a SQL newb 😛

  • well, some of this advice assumes that sp_send_dbmail communicates with your mailserver in real-time; this is not the case. There are certainly issues about transactions, the virtual inserted & deleted tables and passing a query to DBMail, but DBMail will queue up a message even if your SMTP server is on fire!

    Just as a test, I changed my default mail account to use a bogus SMTP server & sent a few test messages. One via. the built-in Database Mail test routine, another via. a call to sp_send_dbmail. Both succeeded, or in the case of the proc, returned a normal "Mail queued." message.

    That said, it seems DBMail doesn't attempt to resend failed messages either (which I thought it did...), so that's not so good either.

Viewing 8 posts - 1 through 7 (of 7 total)

You must be logged in to reply to this topic. Login to reply