CODEWITHSUNDEEP
sql-server
JoHa
JoHa

Reputation: 2039

Cursor inside Trigger not working

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?

Upvotes: 0

Views: 1415

Answers (1)

Ed Harper
Ed Harper

Reputation: 21505

Are you prepared to consider amending your design? There appear to be a couple of issues with what you're attempting here.

A trigger isn't necessarily the best place to be doing this kind of row-by-row operation since it executes in-line with the changes to the source table, and will affect the performance of the system negatively.

Also, your existing code evaluates statusid only for a single row in the batch, although logically it could be set to more than one value in a single batch of updates.

A more robust approach might be to insert the rows which should generate a MAIL_SEND operation to a queuing table, from which a scheduled job can pick rows up and execute MAIL_SEND, setting a flag so that each operation is carried out only once.

This would simplify your trigger to an insert - no cursor would be required there (although you will still need a loop of some kind in the sechduled job).

Upvotes: 2

Related Questions