SQL Trigger Execution problem

Question

I have following trigger. On Insertion of a new row, the Stored Procedure is unable to get the parameter value of variable @ItemID. I am trying to pass the value of ItemID Column of newly inserted Row to the stored procedure CalculateCurrentStock

ALTER TRIGGER UpdateCurrentStock
   ON StockIn
   AFTER INSERT
AS 
BEGIN
    SET NOCOUNT ON;
    EXEC CalculateCurrentStock (SELECT ItemID From INSERTED)
END

The error text reads

Procedure or function 'CalculateCurrentStock' expects parameter '@ItemID', which was not supplied. The statement has been terminated.

Thank you for help.

EDIT: Answer

I have altered the trigger as per Derek Kromm's and KM's suggestion

ALTER TRIGGER UpdateCurrentStock
   ON StockIn
   AFTER INSERT
AS 
BEGIN
    SET NOCOUNT ON;

    DECLARE @CSV varchar(max)
    SELECT @CSV=ISNULL(@CSV+',','')+ItemID From INSERTED
    EXEC CalculateCurrentStock @CSV


END

Thanks for help :)

Marshal, that isn't what I suggested. Your modified trigger will only work if you update a single row. If you update multiple rows, it will not work. — Derek, Aug 24 '11 at 12:30
**your `edit` will fail if your `INSERTED` table contains more than one row.** It is best practice to always write triggers to support multiple rows in `INSERTED` and/or `DELETED`. It is fairly easy and common to insert/update/delete multiple rows in a single statement, and any trigger that only handles a single row will fail in a "logical" manner without an error message (your data gets messed up). try something this to see your trigger fails: INSERT INTO StockIn (col1, col2,...) SELECT TOP 3 col1, col2, ... FROM StockIn ORDER BY ItemID` this assumes ItemID is an identity PK. — KM., Aug 24 '11 at 12:31
I re-edited the answer. Thanks Derek and KM for valuable inputs — Marshal, Aug 25 '11 at 04:04

Derek · Answer 1 · 2011-08-24T12:29:35.630

3

Edit: As Martin pointed out, you can pass a table-valued parameter if you're using SQL Server 2008. If you're using 2005 or prior, you can't do that. KM suggests using a comma-delimited value in this case, which I personally disagree with.

In reality, you should probably copy the stored procedure logic directly into the trigger and avoid the whole mess altogether.

edited Aug 24 '11 at 12:29

answered Aug 24 '11 at 11:49

Derek

21,828
7
53
61

I am sorry I am a newbie to sql trigger. I dont understand what I should put for `` and ``. Thanks again for help – Marshal Aug 24 '11 at 11:52
Have you seen my edit ? That is works currently. Is your code more safe in execution ? – Marshal Aug 24 '11 at 12:10
1

In SQL Server 2008 you **can** pass a set of values via a TVP. will be more efficient to try and rewrite whatever `CalculateCurrentStock` is to do it in a set based way and do it in the trigger itself though. – Martin Smith Aug 24 '11 at 12:11
`You can't pass in a set of values to a procedure - it has to be a single scalar value.` you can pass in a table valued parameter or a CSV list. – KM. Aug 24 '11 at 12:22
I also want to add that passing in a TVP or CSV list is going to undoubtedly require a re-coding of the stored procedure to handle these parameters. Rather than doing that, you should simply move (or copy if the SP is used elsewhere) the SP logic into the trigger. Odds are, there would be a set-friendly solution if you do that. – Derek Aug 24 '11 at 12:33
1

if the procedure doesn't handle a set of ItemId values, then simply copying it into the trigger won't make it work either. You'll still need to make it work in a set based way on the ItemId values. – KM. Aug 24 '11 at 13:05
1

obviously, but the SP may be used elsewhere for single scalar purposes – Derek Aug 24 '11 at 13:08
@Marshal, what you don;t appear to understand is that your current code DOES NOT work. It only works for a limited case. You need to improve your testing procedures to include mulitple inserts anytime you test a trigger. Then you would see that your code will not have the expected result in all cases. – HLGEM Aug 24 '11 at 17:19
@HLGEM: I of course understood the points kept by regarding SET of values, and hence also changed the code accordingly. – Marshal Aug 25 '11 at 04:03

score 2 · Accepted Answer · edited May 23 '17 at 11:55

I'd personally poke out my eye before using a cursor, especially when you can pass in a set of values (table values parameter, or CSV) all at one time in a single procedure call.

if you are running SQL Server 2008 you can create your procedure using a Table-Valued Parameters and should be able to pass in the INSERTED table.

If your are using SQL Server 2005 you can create a comma separated value (csv) list of values within a varchar(max) variable and pass that in to your procedure.

EDIT, here is how to pass in a CSV

ALTER TRIGGER UpdateCurrentStock
   ON StockIn
   AFTER INSERT
AS 
BEGIN
    SET NOCOUNT ON;
    DECLARE @CSV varchar(max)
    SELECT @CSV=ISNULL(@CSV+',','')+ItemID From INSERTED
    EXEC CalculateCurrentStock @CSV
END

now, within you'll need to split apart the @CSV values, so look here: Pass a list-structure as an argument to a stored procedure

@KM: I am using SQL Server 2005. So, in my particular case, can I convert `(SELECT ItemID FROM INSERTED)` to CSV Values ? — Marshal, Aug 24 '11 at 12:27

SQL Trigger Execution problem

2 Answers2