0

I have this insert trigger which handles duplicate key insert attempts:

-- Create trigger to not throw in case of duplicate mapping attempt
CREATE TRIGGER dbo.BlockDuplicates_Product_Category_Mapping
ON dbo.Product_Category_Mapping
INSTEAD OF INSERT
AS
BEGIN
  SET NOCOUNT ON;

  IF NOT EXISTS (SELECT 1 
                 FROM inserted AS i 
                 INNER JOIN dbo.Product_Category_Mapping AS p
                            ON i.ProductId = p.ProductId
                            AND i.CategoryId = p.CategoryId)
  BEGIN
      INSERT INTO dbo.Product_Category_Mapping (ProductId, CategoryId, IsFeaturedProduct, DisplayOrder)
          SELECT ProductId, CategoryId, IsFeaturedProduct, DisplayOrder 
          FROM inserted;
  END
  ELSE
  BEGIN
      PRINT 'Duplicate';
  END
END

And it works fine if I run it directly from my DBMS:

enter image description here

But, if I use it in code with EF:

<PackageReference Include="Microsoft.EntityFrameworkCore" Version="3.1.15" />
<PackageReference Include="Microsoft.EntityFrameworkCore.Relational" Version="3.1.15" />
<PackageReference Include="Microsoft.EntityFrameworkCore.SqlServer" Version="3.1.15" />

It doesn't seem to work, a duplicate key exception is thrown, which is what I was trying to avoid:

Microsoft.EntityFrameworkCore.Relational: An error occurred while updating the entries. See the inner exception for details. Core Microsoft SqlClient Data Provider: Violation of UNIQUE KEY constraint 'UQ_PCM_ProductId_CategoryId'. Cannot insert duplicate key in object 'dbo.Product_Category_Mapping'. The duplicate key value is (920, 129).

This is being inserted through a navigation property, if it makes any difference.

Can someone point out to me why this is not working?

I followed these two related questions but I don't see nothing that can help:

SQL Server 2008 insert trigger not firing

SQL Server 2008 - insert trigger not firing

21
  • 1
    Why use a trigger for this at all? Why do you want to hide the error from the UNIQUE CONSTRAINT? This smells like an XY problem.
    – Thom A
    Commented Mar 14, 2024 at 12:22
  • @ThomA it has a unique constraint, but I don't want it to cause an error, just want to print out the 'Duplicate' text, as shown in the image and move on.
    – anastaciu
    Commented Mar 14, 2024 at 12:23
  • 2
    Then, in your application, handle the error code there. Don't smother it in your trigger.
    – Thom A
    Commented Mar 14, 2024 at 12:24
  • 1
    It will just have unexpected values values for IsFeaturedProduct, DisplayOrder after such a no op insert. That does sound harmful to me.
    – Ralf
    Commented Mar 14, 2024 at 12:32
  • 2
    Your trigger code is pretty bad, since if 1 out of 5 rows exist, it still inserts everything. This is a sure sign that things have gone wrong. I won't mention locking issues. Also, you might still have duplicates in the inserted table Commented Mar 14, 2024 at 12:32

1 Answer 1

4

I'm going to bang the drum again: This idea is a bad idea. Giving the impression to an application/user that things "worked" when they didn't is not a good idea. In this case you say it's "not a problem" but I would say with confidence that at some point a user is going to ask "Why is the product I added just now not featured, when I asked it to be." and the answer will be "Your INSERT was ignored, but we smothered that error for you and didn't tell you."

That being said, you're going about this the wrong way if you "must" (don't) do this in a trigger. If you're inserting multiple rows this is going to go all sorts of wrong, as if even 1 of those rows is a duplicate you ignore all of them. Inserting 100 new products, and 1 is a dupe? You insert zero.

Instead, use a INSERT with a NOT EXISTS and "bin" the rest. Not ideal, and your users will be confused, but this appears to be the behaviour you are after, at least:

CREATE TRIGGER dbo.BlockDuplicates_Product_Category_Mapping
ON dbo.Product_Category_Mapping
INSTEAD OF INSERT
AS
BEGIN
    SET NOCOUNT ON;

    INSERT INTO dbo.Product_Category_Mapping (ProductId,
                                              CategoryId,
                                              IsFeaturedProduct,
                                              DisplayOrder)
    SELECT i.ProductId,
           i.CategoryId,
           i.IsFeaturedProduct,
           i.DisplayOrder
    FROM inserted i
    WHERE NOT EXISTS (SELECT 1
                      FROM dbo.Product_Category_Mapping PCM
                      WHERE PCM.ProductId = i.ProductId
                        AND PCM.CategoryId = i.CategoryId);
END;

If you really wanted to be nice to your users (to avoid their confusion when things aren't inserted), you could do an Upsert. Then you UPDATE the rows that already existed, to the values they wanted to INSERT, and INSERT the new ones.

CREATE TRIGGER dbo.BlockDuplicates_Product_Category_Mapping
ON dbo.Product_Category_Mapping
INSTEAD OF INSERT
AS
BEGIN
    SET NOCOUNT ON;

    --No need to define a transaction, as a trigger is already inside one and automatically has XACT_ABORT enabled

    UPDATE PCM
    SET IsFeaturedProduct = i.IsFeaturedProduct,
        DisplayOrder = i.DisplayOrder
    FROM dbo.Product_Category_Mapping PCM WITH (UPDLOCK, SERIALIZABLE)
        JOIN inserted i ON PCM.ProductId = i.ProductId
                       AND PCM.CategoryId = i.CategoryId;

    INSERT INTO dbo.Product_Category_Mapping (ProductId,
                                              CategoryId,
                                              IsFeaturedProduct,
                                              DisplayOrder)
    SELECT i.ProductId,
           i.CategoryId,
           i.IsFeaturedProduct,
           i.DisplayOrder
    FROM inserted i
    WHERE NOT EXISTS (SELECT 1
                      FROM dbo.Product_Category_Mapping PCM
                      WHERE PCM.ProductId = i.ProductId
                        AND PCM.CategoryId = i.CategoryId);
END;
5
  • Thanks, I used the second, which is great, a definite upgrade, I found a bug though, it was inserting 2 equal rows in the same transaction, which was still failing even with your improved code, I guess its a very corner, corner case. Thanks.
    – anastaciu
    Commented Mar 14, 2024 at 13:57
  • Perhaps the data you are passing has 2 or more rows with the same values, @anastaciu . That will cause the error. Something like INSERT INTO dbo.Product_Category_Mapping (ProductID, CategoryId) VALUES (1,2),(1,2); That wouldn't be a trigger issue, that's an input issue. Fixing the application to not allow that would be the requirement there (but you've already stated there's not appetite to fix that application(s)).
    – Thom A
    Commented Mar 14, 2024 at 14:05
  • Yes, that was it.
    – anastaciu
    Commented Mar 14, 2024 at 14:06
  • So the trigger wasn't be ignored. You could handle this in the trigger too, @anastaciu , but you'd have to know which row is the correct row when you're inserting it twice. But, to bang that drum again: fix the applications.
    – Thom A
    Commented Mar 14, 2024 at 14:08
  • Yes, preaching to the choir my friend, thanks again.
    – anastaciu
    Commented Mar 14, 2024 at 17:29

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.