USE [ddb] GO SET ANSI_NULLS OFF GO SET QUOTED_IDENTIFIER ON GO CREATE TRIGGER [dbo].[requeststrigger] ON [dbo].[requests] AFTER INSERT,UPDATE AS BEGIN DECLARE @email VARCHAR(400); DECLARE @firstname VARCHAR(400); DECLARE @requestno VARCHAR(400); DECLARE @lastname VARCHAR(400); DECLARE @statusid INT; DECLARE thecursor CURSOR FOR SELECT inserted.requestno,contacts.firstname,contacts.lastname,contacts.email FROM request_contacts,contacts,inserted WHERE request_contacts.requestid=inserted.requestid AND contacts.contactid=request_contacts.contactid AND request_contacts.notification=1 AND contacts.notification=1; SET @statusid = (SELECT statusid FROM inserted); IF @statusid = 4 AND @statusid <> (SELECT statusid FROM deleted) BEGIN SET NOCOUNT ON SET ARITHABORT ON OPEN thecursor; FETCH NEXT FROM thecursor INTO @requestno,@firstname,@lastname,@email WHILE @@FETCH_STATUS = 0 BEGIN EXEC MAIL_SEND @email,@firstname,@requestno,@lastname; FETCH NEXT FROM thecursor INTO @requestno,@firstname,@lastname,@email END CLOSE thecursor; DEALLOCATE thecursor SET NOCOUNT OFF END END This simply makes the whole UPDATE/INSERT not work. When I remove the cursor declaration, it works. The cursor is just selecting a field from a table that is existing in the same database called "contacts". What is wrong?
A trigger should ideally do something very fast, so that it can run "incognito". If what you want is to loop through a child table and send a mail for each row, I agree with the suggestion to change the trigger to instead just update a table with the information that should be e-mailed, and have a job run every n seconds/minutes/hours/whateverintervalyouwant. Here's how I would change your current trigger. First a table to store the information to send: CREATE TABLE NotificationsToSend(ID int IDENTITY(1,1) PRIMARY KEY, email varchar(400), firstname varchar(400), lastname varchar(400), requestno varchar(400), date_sent datetime DEFAULT null) GO CREATE INDEX ix_date_sent ON NotificationsToSend(date_sent) GO And then some changes to the trigger: CREATE TRIGGER [dbo].[requeststrigger] ON [dbo].[requests] AFTER INSERT,UPDATE AS BEGIN insert into NotificationsToSend(requestno, firstname, lastname, email) SELECT inserted.requestno,contacts.firstname,contacts.lastname,contacts.email FROM inserted inner join request_contacts on inserted.requestid=request_contacts.requestid inner join contacts on request_contacts.contactid=contacts.contactid WHERE request_contacts.notification=1 AND contacts.notification=1 and inserted.statusid=4 and inserted.statusid not in (select statusid from deleted) END Finally I would create a stored procedure which actually sends the emails: CREATE PROC SendNotificationEmails AS DECLARE @email varchar(400), @firstname varchar(400), @lastname varchar(400), @requestno varchar(400), @id int DECLARE TRIGGER tr FOR select email, firstname, lastname, requestno, id from NotificationsToSend where date_sent is null OPEN tr FETCH NEXT FROM tr INTO @email, @firstname, @lastname, @requestno, @id WHILE @@FETCH_STATUS=0 BEGIN EXEC MAIL_SEND @email,@firstname,@requestno,@lastname UPDATE NotificationsToSend SET date_sent=CURRENT_TIMESTAMP where
ID=@ID FETCH NEXT FROM tr INTO @email, @firstname, @lastname, @requestno, @id END An alternative to this would be to include the logic to send e-mails into the original stored procedure which performs the updates. But that would mean the application has to wait for an e-mail to be sent for each child-row, and that's not very efficient either. My suggestion is written totally without even the simplest compilation tests, so it probably needs a few tweaks...
I'm not sure, entirely, but I have a feeling it's SQL Server's way of telling you that a cursor inside a trigger is not a good way to go. Triggers, ok (some people will try and avoid them like the plague, but I accept that they have their place) - but a cursor within a trigger is going to be a performance issue. Try and do what you wanted to do in a set-based manner, rather than using the cursor.