Sql-server – Update trigger issue throwing errors on IF statements

sql server

I have created a trigger, basically what I want it to do is update or insert a record to another table.

Two table I have got:

MVF_Transport_Register: Main table
MVF_Transport_Acc: table trigger need to update.

When a record is updated on the Main table, I want to the trigger to check if that records exists on the MVF_Transport_Acc.

If it does only update the Record_status column.
If the record is not in MVF_Transport_Acc then insert the new record.

This is what I have written but I am getting the following error when trying to deploy it:

Msg 156, Level 15, State 1, Procedure UpdateTransport_TEST1, Line 36

Incorrect syntax near the keyword 'END'.

Here's the trigger:

SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO

ALTER TRIGGER [dbo].[UpdateTransport_TEST1]
   ON  [dbo].[MVF_Transport_Register]
   AFTER UPDATE
AS
IF UPDATE (Record_Status) 

BEGIN
IF (i.Order_No=c.Order_No )
UPDATE c
set Record_Status=i.Record_Status
FROM dbo.MVF_Transport_Acc AS c
JOIN inserted as i
ON i.Order_No = c.Order_No
JOIN deleted as d
ON i.Order_No=d.Order_No
OR (i.Record_Status <> d.Record_Status)
WHERE  (i.Record_Status = 'CONFIRMED' OR
       i.Record_Status = 'AMENDED') AND (i.Price > 0)
ELSE

INSERT into dbo.MVF_Transport_Acc
([Order_No], [Record_Status], [Notes], [Transport_Supplier], [Surcharge], [Vendor_No], [Pallets], [Amend_Pallets],[Price], [Input_Date], [Amend_Price], [Transport_Invoice_No],[Transport_Job_No], [Amend_Date], [Report_Price], 
                         [Return_Price], [Report_Date])

SELECT  i.[Order_No], i.[Record_Status], i.[Notes], i.[Transport_Supplier], i.[Surcharge], i.[Vendor_No], i.[Pallets], i.[Amend_Pallets],i.[Price], i.[Input_Date], i.[Amend_Price], i.[Transport_Invoice_No],i.[Transport_Job_No], i.[Amend_Date], i.[Report_Price], 
                         i.[Return_Price], i.[Report_Date]

        FROM   [dbo].[MVF_Transport_Register] c
        INNER JOIN inserted i on
       c.Order_No = i.Order_No   
WHERE  (i.Record_Status = 'CONFIRMED' OR
       i.Record_Status = 'AMENDED') AND (i.Price > 0)

END IF


END

Best Answer

This logic seems wrong to me, but I think you have it way more complicated than it needs to be:

ALTER TRIGGER [dbo].[UpdateTransport_TEST1]
   ON  [dbo].[MVF_Transport_Register]
   AFTER UPDATE
AS
BEGIN
  UPDATE c 
    SET c.Record_Status = i.Record_status
    FROM dbo.MVF_Transport_Acc AS c
    INNER JOIN inserted AS i
    ON c.Order_No = i.Order_No
    AND c.Record_Status <> i.Record_Status
    WHERE i.Record_Status IN ('CONFIRMED', 'AMENDED')
      AND i.Price > 0;

  IF @@ROWCOUNT = 0
  BEGIN
    INSERT dbo.MVF_Transport_Acc
    ([Order_No], [Record_Status], [Notes], [Transport_Supplier], 
     [Surcharge], [Vendor_No], [Pallets], [Amend_Pallets],[Price], 
     [Input_Date], [Amend_Price], [Transport_Invoice_No],[Transport_Job_No], 
     [Amend_Date], [Report_Price], [Return_Price], [Report_Date])
    SELECT i.[Order_No], i.[Record_Status], i.[Notes], i.[Transport_Supplier], 
           i.[Surcharge], i.[Vendor_No], i.[Pallets], i.[Amend_Pallets],
           i.[Price], i.[Input_Date], i.[Amend_Price], i.[Transport_Invoice_No],
           i.[Transport_Job_No], i.[Amend_Date], i.[Report_Price], 
           i.[Return_Price], i.[Report_Date]
    FROM inserted AS i
    WHERE i.Record_Status IN ('CONFIRMED', 'AMENDED')
      AND i.Price > 0;
  END
END
GO

I still feel like that logic is incomplete for some reason.

Related Solutions

Sql-server – What would cause extra records to be inserted during a INSERT trigger

First, you don't need the IF EXISTS part in your trigger code. You have the exact same code in the second part so if there are 0 rows produced, you'll be inserting 0 rows anyway:

CREATE TRIGGER [dbo].[trFooInsert] on [dbo].[Foo]
AFTER INSERT
AS 
BEGIN
    INSERT INTO Foo(ID, Etc, Flag)              
    SELECT a.ID, a.Etc, 1 FROM Inserted a
        LEFT JOIN Foo b ON b.Flag = 1 AND a.ID = b.ID
        WHERE b.Flag IS NULL AND a.Flag = 0
END ; -- trigger

which can also be written as:

CREATE TRIGGER [dbo].[trFooInsert] on [dbo].[Foo]
AFTER INSERT
AS 
BEGIN
    INSERT INTO Foo(ID, Etc, Flag)              
    SELECT a.ID, a.Etc, 1 
        FROM Inserted a
        WHERE a.Flag = 0
          AND NOT EXISTS
              ( SELECT *
                FROM Foo b 
                WHERE b.Flag = 1 AND a.ID = b.ID
              )
END ; -- trigger

So, you want any insert on the table with Flag=0 to also trigger another insert, with same data but Flag=1 (unless there is already a row in Foo with that ID and Flag=1.

Second, and what the actual problem is about, Inserted is a special table with all the (just) "inserted" rows, not the full table you inserted into. It obviously has less (than or equal) rows as Foo after the insert. So a join to it will not give same results as a join to Foo.

The trigger that produces the multiple unwanted rows had Inserted b instead of Foo b so it does this:

For any insert on the table with Flag=0 it also triggered another insert, with same data but Flag=1 (unless this same insert had such a row inserted).

You can read more about the special Inserted and Deleted tables at the MSDN:
How to use the inserted and deleted Tables

Sql-server – Passing info on who deleted record onto a Delete trigger

Is there a way to pass information onto the Delete trigger such that it could know who deleted the record?

Yes: by using a very cool (and under utilized feature) called CONTEXT_INFO. It is essentially session memory that exists in all scopes and is not bound by transactions. It can be used to pass info (any info--well, any that fits into the limited space) to triggers as well as back and forth between sub-proc / EXEC calls. And I have used it before for this exact same situation.

Context info is a VARBINARY(128)
Set via: SET CONTEXT_INFO
Get via: CONTEXT_INFO()

Test with the following to see how it works. Notice that I am converting to CHAR(128) before the CONVERT(VARBINARY(128), ... This is to force blank-padding to make it easier to convert back to VARCHAR when getting it out of CONTEXT_INFO() since VARBINARY(128) is right-padded with 0x00s.

SELECT CONTEXT_INFO();
-- Initially = NULL

DECLARE @EncodedUser VARBINARY(128);
SET @EncodedUser = CONVERT(VARBINARY(128),
                            CONVERT(CHAR(128), 'I deleted ALL your records! HA HA!')
                          );
SET CONTEXT_INFO @EncodedUser;

SELECT CONTEXT_INFO() AS [RawContextInfo],
       RTRIM(CONVERT(VARCHAR(128), CONTEXT_INFO())) AS [DecodedUser];

Results:

0x492064656C6574656420414C4C20796F7572207265636F7264732120484120484121202020202020...
I deleted ALL your records! HA HA!

PUTTING IT ALL TOGETHER:

The app should call a "Delete" stored procedure that passes in the UserName (or whatever) that is deleting the record. I assume this is already the model being used since it sounds like you are already tracking Insert and Update operations.

The "Delete" stored procedure does:

DECLARE @EncodedUser VARBINARY(128);
SET @EncodedUser = CONVERT(VARBINARY(128),
                            CONVERT(CHAR(128), @UserName)
                          );
SET CONTEXT_INFO @EncodedUser;

-- DELETE STUFF HERE

The audit trigger does:

-- Set the INT value in LEFT (currently 50) to the max size of [UserWhoMadeChanges]
INSERT INTO AuditTable (IdOfRecordedAffected, UserWhoMadeChanges) 
   SELECT del.ID, COALESCE(
                     LEFT(RTRIM(CONVERT(VARCHAR(128), CONTEXT_INFO())), 50),
                     '<unknown>')
   FROM DELETED del;

Please note that, as @SeanGallardy pointed out in a comment, due to other procedures and/or ad hoc queries deleting records from this table, it is possible that either:
- CONTEXT_INFO has not been set and is still NULL:
  
  For this reason I have updated the above INSERT INTO AuditTable to use a COALESCE to default the value. Or, if you don't want a default and require a name, then you could do something similar to:
```
DECLARE @UserName VARCHAR(50); -- set to the size of AuditTable.[UserWhoMadeChanges]
SET @UserName = LEFT(RTRIM(CONVERT(VARCHAR(128), CONTEXT_INFO())), 50);

IF (@UserName IS NULL)
BEGIN
   ROLLBACK TRAN; -- cancel the DELETE operation
   RAISERROR('Please set UserName via "SET CONTEXT_INFO.." and try again.', 16 ,1);
END;

-- use @UserName in the INSERT...SELECT
```
- CONTEXT_INFO has been set to a value that is not a valid UserName, and hence might exceed the size of the AuditTable.[UserWhoMadeChanges] field:
  
  For this reason I added a LEFT function to ensure that whatever is grabbed out of CONTEXT_INFO will not break the INSERT. As noted in the code, you just need to set the 50 to the actual size of the UserWhoMadeChanges field.

UPDATE FOR SQL SERVER 2016 AND NEWER

SQL Server 2016 added an improved version of this per-session memory: Session Context. The new Session Context is essentially a hash table of Key-Value pairs with the "Key" being of type sysname (i.e. NVARCHAR(128) ) and the "Value" being SQL_VARIANT. Meaning:

There is now a separation of values so less likely to conflict with other uses
You can store various types, no longer needing to worry about the odd behavior when getting the value back out via CONTEXT_INFO() (for details, please see my post: Why Doesn’t CONTEXT_INFO() Return the Exact Value Set by SET CONTEXT_INFO?)
You get a lot more space: 8000 bytes max per "Value", up to 256kb total across all keys (compared to the 128 bytes max of CONTEXT_INFO)

For details, please see the following documentation pages:

Best Answer

Related Solutions

Sql-server – What would cause extra records to be inserted during a INSERT trigger

Sql-server – Passing info on who deleted record onto a Delete trigger

Related Question