Reputation: 1817
I created a trigger to fire AFTER UPDATE
of a table -- SOP_Head
. This is supposed to monitor whenever a sales order has its status changed to "picked" so that a request for an advanced shipping notice to be created is generated at this stage.
CREATE TRIGGER OrderPickedCreateASN ON SOP_Head AFTER UPDATE
AS
BEGIN
-- Check the new status of the order
DECLARE @sopstatus SMALLINT
SELECT @sopstatus=SOPSTATUS FROM inserted
-- Get the SOPNUMBE
DECLARE @sopnumbe CHAR(21)
SELECT @sopnumbe=SOPNUMBE FROM inserted
-- A SOPSTATUS of 4 means the order has been set to picked
-- At this stage, create an ASN export request
IF @sopstatus=4
BEGIN
-- Create the ASN export record
EXEC AddOrderASNExportRequest @in_sopnumbe=@sopnumbe
END
-- If any other status, remove any existing requests
ELSE
BEGIN
EXEC RemoveOrderASNExportRequest @in_sopnumbe=@sopnumbe
END
END
GO
When I attempt to fire this trigger using the following update, it only fires for the first and last order numbers (ORD117196 AND ORD117199).
UPDATE SOP_Head SET SOPSTATUS=4
WHERE SOPNUMBE IN ('ORD117196','ORD117197','ORD117198','ORD117199')
If I adjust the above statement to only include the two remaining orders for which an ASN export request is not generated (ORD117197 and ORD117198), they are created without issue.
Why is it that the trigger doesn't appear to be fired for all four updates of SOP_Head
?
Upvotes: 0
Views: 60
Reputation: 2992
I can think of a two general approaches to your problem.
One of them is actually the one that you have implemented - using a stored procedure. However, I would recommend firing off one execution of a stored procedure for bulk of records from SOP_Head. The power of SQL is when working on a set of records, not looping them one by one. This, of course, requires some redesigning of your SPs. A possible implementation could be the following:
--this UDTT could be defined in a more generic way for a good reuse
CREATE TYPE dbo.SopNrArray AS TABLE
(
SopNumber CHAR(21) PRIMARY KEY
)
GO
CREATE TRIGGER OrderPickedCreateASN ON SOP_Head AFTER UPDATE
AS
BEGIN
DECLARE @sopstatusADD dbo.SopNrArray;
INSERT @sopstatusADD
SELECT DISTINCT SOPNUMBE FROM inserted WHERE SOPSTATUS=4;
--consider removing DISTINCT if all SOPNUMBE would be unique
DECLARE @sopstatusREM dbo.SopNrArray;
INSERT @sopstatusREM
SELECT DISTINCT SOPNUMBE FROM inserted WHERE SOPSTATUS!=4;
--consider removing DISTINCT if all SOPNUMBE would be unique
-- This check could eventually be removed if considered to be more optimal
IF EXISTS (SELECT NULL FROM @sopstatusADD)
BEGIN
-- Create the ASN export record
EXEC AddOrderASNExportRequest @sopstatusADD
END
-- This check could eventually be removed if considered to be more optimal
IF EXISTS (SELECT NULL FROM @sopstatusREM)
BEGIN
EXEC RemoveOrderASNExportRequest @sopstatusREM
END
END
GO
The other approach would be to implement some kind of FIFO queue using another dedicated table and then a frequently-enough scheduled SQL Agent Job that would process the records. This has the following main pros:
it is a lot extensible for future implementations
does not affect the performance of the main flow.
Upvotes: 1
Reputation: 1271241
Your code assumes that inserted
has only one row. inserted
is a view that can contain multiple rows.
This is simply wrong. You are going to need to loop through inserted
to do one exec per update.
Probably the safest method is to use a cursor -- because you are using stored procedures in the trigger. That makes the code much more complicated than it would otherwise need to be.
If you have a unique id, I would probably do:
declare @i int;
declare @n int;
select @n = count(*), @i = 1 from inserted;
while @i <= n
begin
with i as (
select i.*, row_number() over (order by id) as seqnum
from inserted
)
select @sopstatus = SOPSTATUS, @sopnumbe = SOPNUMBE
from inserted
where seqnum = @i;
set @i := @i + 1;
IF @sopstatus=4
BEGIN
-- Create the ASN export record
EXEC AddOrderASNExportRequest @in_sopnumbe=@sopnumbe
END
-- If any other status, remove any existing requests
ELSE
BEGIN
EXEC RemoveOrderASNExportRequest @in_sopnumbe=@sopnumbe
END
end; -- while
This is actually a bit more expensive than a cursor if may rows are inserted at the same time, but I find it easier to code.
Upvotes: 2