2

I need a little help with a trigger. Where I'm in wrong:

CREATE TRIGGER [dbo].[tr_ins_MyTriggerName]
ON [dbo].MyTableName
FOR INSERT
AS 
    DECLARE 
    @Number nvarchar(20) ,
    @Date datetime ,
    @Supplier nvarchar(12);

    SET @Number = (SELECT Number FROM INSERTED);
    SET @Date = (SELECT Date FROM INSERTED);
    SET @Supplier = (SELECT Supplier FROM INSERTED);
BEGIN
    IF EXISTS (
        SELECT TOP 1 1 FROM MyTableName x
        WHERE x.Number = @Number
            AND x.Date = @Date
            AND x.Supplier = @Supplier
        )
    BEGIN
        RAISERROR ('There is the same Number for the same Supplier!' ,11,1)
        ROLLBACK
    END
END

This trigger runs in all cases even the the EXITS is not true!

0

2 Answers 2

1

I'd suggest you to create a unique index on the 3 columns, i.e. date / number / supplier.

If this is not applicable, I'd say you modify your trigger as follows

CREATE TRIGGER [dbo].[tr_ins_MyTriggerName]
ON [dbo].MyTableName
FOR INSERT
AS 
    DECLARE 
    @Number nvarchar(20) ,
    @Date datetime ,
    @Supplier nvarchar(12);

    SET @Number = (SELECT Number FROM INSERTED);
    SET @Date = (SELECT Date FROM INSERTED);
    SET @Supplier = (SELECT Supplier FROM INSERTED);
BEGIN
    IF  (
        SELECT count(*) FROM MyTableName x
        WHERE x.Number = @Number
            AND x.Date = @Date
            AND x.Supplier = @Supplier
        ) > 1
    BEGIN
        RAISERROR ('There is the same Number for the same Supplier!' ,11,1)
        ROLLBACK
    END
END

However, this trigger is problematic when you insert multiple values in one insert statement.

0
2

This trigger runs in all cases even the the EXITS is not true!

The condition will always be true since this trigger fires after the record is inserted. FOR is synonymous with AFTER for TRIGGERS, thus the EXISTS would never not be true.

FOR | AFTER

Specifies that the DML trigger is fired only when all operations specified in the triggering SQL statement have executed successfully. All referential cascade actions and constraint checks also must succeed before this trigger fires.

AFTER is the default when FOR is the only keyword specified.

When you say it runs do you mean the trigger fires, or that the error is always raised? I'd expect both, as explained above.

It seems like you are trying to do error handling for an INSERT in which I'd do it in the procedure or code block that you are doing the INSERT via a TRY / CATCH block, or just add a constraint on the table to prevent this type of duplicate insertion. Erland Sommarskog has an extensive blog on error handling that's very helpful. Here's a basic framework via one of this posts, but be sure to read it!

Here is a stored procedure that showcases how you should work with errors and transactions.

CREATE PROCEDURE insert_data @a int, @b int AS 
   SET XACT_ABORT, NOCOUNT ON
   BEGIN TRY
      BEGIN TRANSACTION
      INSERT sometable(a, b) VALUES (@a, @b)
      INSERT sometable(a, b) VALUES (@b, @a)
      COMMIT TRANSACTION
   END TRY
   BEGIN CATCH
      IF @@trancount > 0 ROLLBACK TRANSACTION
      DECLARE @msg nvarchar(2048) = error_message()  
      RAISERROR (@msg, 16, 1)
      RETURN 55555
   END CATCH
1
  • Hi, @scsimon, Thank you very match for your answer! It is very useful. Details are: 1. I do not have access to source code. 2. We have many old records and that is the reason that I want to add trigger to stop the new errors. :)
    – StanYan
    Commented Jan 9, 2019 at 5:50

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.