0

I have the following trigger:

ALTER TRIGGER .[dbo].[trgAfterInsertComment] 
   ON  .[dbo].[Comment] 
   AFTER INSERT
AS 
BEGIN
    -- SET NOCOUNT ON added to prevent extra result sets from
    -- interfering with SELECT statements.
    SET NOCOUNT ON;
    declare @Id int;
    declare @LoanId int;
    declare @CopyId int;
    declare @CustomerId int;
    declare @Comment nvarchar(255);

    --DECLARE cur CURSOR FOR 
    select @Id = Id from inserted
    select @LoanId = LoanId from inserted
    --select @CopyId = CopyId from deleted
    --select @CustomerId = CustomerId from deleted
    select @Comment = Comment from inserted
    -- OPEN cur

    --FETCH NEXT FROM cur INTO @Id, @ISBN, @Purchase_Date

    --WHILE @@FETCH_STATUS = 0 BEGIN

        -- your business logic 
    Declare @Title nvarchar(255);
    select @Title = (Select Title from Book where ISBN = (select ISBN from copy where Id = (select CopyId from Loan where Id = @LoanId)))
    select @CustomerId = (Select CustomerId from Loan where Id = @LoanId)
    select @CopyId = (Select CopyId from Loan where Id = @LoanId)
    insert into Activity("Heading", "Date")

    values(Concat('New Comment added - Id: ', @Id, ' Title: ', @Title, ' Copy Id: ', @CopyId, ' Customer Id: ', @CustomerId, ' Comment: ', @Comment), GETDATE())

        --FETCH NEXT FROM cur INTO @Id, @ISBN, @Purchase_Date

    --END

    --CLOSE cur
    --DEALLOCATE cur
    end

As you can see I have commented out a cursor that I was using to handle multiple inserts. Could someone tell me how I can handle multiple inserts without the cursor, as after reading around I see that using a cursor is a bad idea?

With the above trigger, if I try to insert multiple lines like this:

USE [Library]
GO

INSERT INTO [dbo].[Comment]
           ([LoanId]
           ,[Comment])
     VALUES
           (47, 'test'),
           (48, 'test'),
           (50, 'test')
GO

Only the first row is inserted into my Activity table. Thanks for any help

2
  • "the first row is inserted" - you're not even guaranteed that. Because you're running independent selects to populate your variables, there's no guarantee that they all correspond to the same row from inserted. Commented Oct 16, 2017 at 10:32
  • Any suggestions on how to do it differently, Damien? Commented Oct 16, 2017 at 10:33

2 Answers 2

3

You need to shift it to be set based, using variables and a loop will cause you issues. Can't test the below, but something like:

INSERT INTO Activity
        (   
            Heading , 
            [Date]
        )
SELECT      CONCAT('New Comment added - Id: ', I.id, ' Title: ', COALESCE(B.Title,''), ' Copy Id: ', COALESCE(L.CopyID,''), ' Customer Id: ', COALESCE(L.CustomerID,''))    ,
            GETDATE()
FROM        inserted    AS  I
LEFT JOIN   Loan        AS  L   ON  I.loanId    =   L.loanId
LEFT JOIN   Copy        AS  C   ON  C.Id        =   L.CopyId
LEFT JOIN   Book        AS  B   ON  B.ISBN      =   C.ISBN;
Sign up to request clarification or add additional context in comments.

Comments

2

Do this querying inserted table directly.

insert into [dbo].[Comment] (LoanId, Comment)
select LoanId, Comment from inserted

You can change the select query to more complex to achieve the result using query only.

2 Comments

Hi Arkadiusz, thanks for your post. But for each row in inserted, I want to add one row to my activity table. I'm not sure if your answer fits this?
@DevDave it does, SQL server works in sets. If you are thinking from a programming POV you will think of foreach loops to add to a table, but SQL doesn't need to work like that. If you use INSERT with a SELECT, a row will be inserted for each row the SELECT brings back. This is the way to use SQL, loops and cursors have a place, but should be avoided where they can be.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.