Trigger with a RAISERROR and ELSE case issue

Question

I am trying to make a bit of code that takes in 2 separate columns, a month and a year. From there I want it to see if those numbers entered have already passed or not. If they have passed, cause an error to pass and stop the transaction. Otherwise, I want it to continue on and insert new information into the table. I know I am close on getting this to work, but I cant seem to get the RAISERROR to fire. I am sure it has to do with the fact I am pretty new at this and I am missing some small detail.

Currently I am taking the two months in as variables and the making a third variable to use to turn the other two into a proper datetime format. Then I use the datediff function to try and see if it has passed that way. To no avail though. I keep getting the insert function going, even if the card date is old.

USE AdventureWorks2012
GO

CREATE TRIGGER BadCreditCardDate
ON Sales.CreditCard
INSTEAD OF INSERT
AS
Begin
DECLARE @ExpMonth tinyint,
        @ExpYear smallint,
        @ExpMonthYear datetime

SELECT  @ExpMonth=ExpMonth, 
        @ExpYear=ExpYear,
        @ExpMonthYear = @ExpYear + '-' + @ExpMonth + '-00' 
FROM INSERTED
    IF
    DATEDIFF(MONTH,@ExpMonthYear,GETDATE()) < 0
    BEGIN
        RAISERROR ('The Credit Card you have entered has expired.' ,10,1)
        ROLLBACK TRANSACTION
    END 

ELSE    
Begin
    INSERT INTO CreditCard (CardType, CardNumber, ExpMonth, ExpYear, ModifiedDate)
    Select CardType, CardNumber, ExpMonth, ExpYear, ModifiedDate FROM inserted
END
End

Your trigger has **MAJOR** flaw in that you seem to assume it'll be called **once per row** - that is **not** the case. The trigger will fire **once per statement**, so if your `UPDATE` statements affects 25 rows, you'll get the trigger fired **once**, but then `Inserted` and `Deleted` will each contain 25 rows. Which of those 25 rows will your code select here?? `SELECT @ExpMonth = ExpMonth FROM Inserted` - it's non-deterministic. You need to rewrite your trigger to take this into account! — marc_s, Jan 25 '15 at 18:57
You have a hard-coded '00' for the expiration date day. I would expect the DATEDIFF to fail if the expression is not null. — Dan Guzman, Jan 25 '15 at 19:01

Giorgos Betsos · Accepted Answer · 2015-01-25T19:26:08.423

8

I think there is a simpler way to check for expiration:

CREATE TRIGGER BadCreditCardDate
ON Sales.CreditCard
INSTEAD OF INSERT
AS
BEGIN
   IF EXISTS (
      SELECT 1
      FROM inserted
      WHERE (YEAR(GETDATE()) > ExpYear) OR (YEAR(GETDATE()) = ExpYear AND MONTH(GETDATE()) > ExpMonth)
   )
   BEGIN
      RAISERROR ('The Credit Card you have entered has expired.' ,10,1)
      ROLLBACK TRANSACTION
   END 
   ELSE    
      BEGIN
         INSERT INTO CreditCard (CardType, CardNumber, ExpMonth, ExpYear, ModifiedDate)
         SELECT CardType, CardNumber, ExpMonth, ExpYear, ModifiedDate 
         FROM inserted
   END    
END

In this way you effectively check every record to be inserted in CreditCard.

edited Jan 25 '15 at 19:26

answered Jan 25 '15 at 19:20

Giorgos Betsos

71,379
9
63
98

This worked like a charm. I am going to have to look over this bit of code to see how its working and then remember this little beauty. – MikeyZ Jan 25 '15 at 19:28
Won't this rollback the entire transaction (and possible thousands of other possibly valid credit cards from other users if they just so happened to have been on the same transaction? – PSXGamerPro1 Jul 26 '22 at 04:51
@PSXGamerPro1 Yes, it will rollback the entire transaction. It was the OP's intention to place the `ROLLBACK` inside the trigger. I guess he's not inserting more than one rows per transaction. – Giorgos Betsos Jul 26 '22 at 06:58

Trigger with a RAISERROR and ELSE case issue

1 Answers1