Condition in trigger failing to rollback

Question

I'm trying to create a trigger on insert in the [user_dance_style] table of the following database.

The [user_dance_style] table has two columns [id_user] and [style_ref]. There are 3 styles available and each id_user can have one to three of the syles. On insert, I want to check if the id_user has already three styles, if so ROLLBACK

I also want to check if the id_user has already the style to be inserted in that case ROLLBACK.

In clear I just want to be able to make a new insert if the id_user hasn't all 3 styles already and doesn't have the style that I'm trying to insert.

My trigger looks like this:

DROP TRIGGER IF EXISTS update_style
GO

CREATE TRIGGER update_style 
ON [user_dance_style] 
AFTER INSERT
AS
BEGIN
    DECLARE @id_user int, @number_of_style int, @style INT

    SELECT @style = style_ref FROM inserted
    SELECT @id_user = id_user FROM inserted

    SELECT @number_of_style = COUNT(*)
    FROM [user_dance_style]
    WHERE id_user = @id_user

    IF @number_of_style > 3
        ROLLBACK

    IF @style IN (SELECT [user_dance_style].[style_ref]
                  FROM [user_dance_style]
                  WHERE [user_dance_style].[id_user] = @id_user)
        ROLLBACK
END

It seems that the problem is here:

IF @style IN (SELECT [user_dance_style].[style_ref]
              FROM [user_dance_style]
              WHERE [user_dance_style].[id_user] = @id_user)
        ROLLBACK

The tables' structure is:

CREATE TABLE [user]
(
    [id_user] INT PRIMARY KEY NOT NULL IDENTITY(1,1),
    [user_name] VARCHAR(45)  NOT NULL UNIQUE,
    [User_Sex] CHAR(1) NOT NULL,
    [date_of_birth] DATE NOT NULL,
    [account_type] INT NOT NULL,
    [id_address] INT NOT NULL,
);

CREATE TABLE [address]
(
    [id_address] INT PRIMARY KEY NOT NULL IDENTITY(1,1),
    [street] VARCHAR(255) NOT NULL,
    [number] INT NOT NULL,
    [locality] VARCHAR(255) NOT NULL,
    [city] VARCHAR(255) NOT NULL,
    [country_code] CHAR(2) NOT NULL
);

CREATE TABLE [membership]
(
    [account_type] INT PRIMARY KEY NOT NULL IDENTITY(1,1),
    [membership_name] VARCHAR(45) UNIQUE NOT NULL,
    [membership_price] DECIMAL(4,2) NOT NULL
);

CREATE TABLE [style]
(
    [style_ref] INT PRIMARY KEY NOT NULL IDENTITY(1,1),
    [style_name] VARCHAR(45) UNIQUE NOT NULL
);

CREATE TABLE [dance]
(
    [id_dance] INT NOT NULL IDENTITY(1,1),
    [dancer_1_id_user] INT,
    [dancer_2_id_user] INT,
    [dance_dtg] DATETIME NOT NULL,
    [style_ref] INT NOT NULL,
    FOREIGN KEY (dancer_1_id_user) REFERENCES [user] (id_user),
    FOREIGN KEY (dancer_2_id_user) REFERENCES [user] (id_user),
    FOREIGN KEY (style_ref) REFERENCES [style] (style_ref)  
);

CREATE TABLE [user_dance_style]
(
    [id_user] INT,
    [style_ref] INT NOT NULL
    FOREIGN KEY (id_user) REFERENCES [user] (id_user),
    FOREIGN KEY (style_ref) REFERENCES [style] (style_ref)
)

ALTER TABLE [user]
     ADD CONSTRAINT fk_user_memebership 
         FOREIGN KEY (account_type) REFERENCES membership (account_type),
         CONSTRAINT fk_user_address 
         FOREIGN KEY (id_address) REFERENCES address (id_address);

-- disable all constraints
EXEC sp_MSforeachtable "ALTER TABLE ? NOCHECK CONSTRAINT all"

INSERT INTO [membership] ([membership_name], [membership_price])
VALUES ('free', '0'), ('regular', '15'), ('premium', '30')
GO

INSERT INTO [style]([style_name])
VALUES ('Salsa'), ('Bachata'), ('Kizomba') 
GO

INSERT INTO [user] ([user_name], [User_Sex], [date_of_birth], [account_type], [id_address]) 
VALUES ('sara', 'f', '1990-04-23', '1', '1'),
       ('elenor', 'f', '1989-02-18', '1', '2'),
       ('eva', 'f', '1987-01-04','1','3'),
       ('mike', 'm', '1985-05-02', '1', '4'),
       ('phil', 'm', '1985-03-01', '1', '5'),
       ('laurent', 'm', '1986-02-14', '2', '6'),
       ('nidia', 'f', '1985-01-16', '2', '7'),
       ('franz', 'm', '1990-03-17', '2', '8'),
       ('stephan', 'm', '1991-05-23', '2', '9'),
       ('sandra', 'f', '1993-03-25', '3', '10'),
       ('virginie', 'f', '1999-05-03', '3', '11'),
       ('claire', 'f', '1992-02-24', '3', '12'),
       ('laurence', 'f', '1991-04-26', '3', '13'),
       ('pierre', 'm', '1987-02-14', '3', '14'),
       ('thierry', 'm', '1989-01-04', '3', '15'),
       ('nancy', 'f', '1950-04-15', '1', '16'),
       ('cédric', 'm', '1980-02-02', '1', '17')
GO

INSERT INTO [address] ([street], [number], [locality], [city], [country_code])
VALUES ('av de l''exposition', '13', 'laeken', 'bruxelles', 'be'),
       ('rue cans', '2', 'ixelles', 'bruxelles', 'be'),
       ('rue goffart', '32', 'ixelles', 'bruxelles', 'be'),
       ('ch de haecht', '17', 'schaerbeek', 'bruxelles', 'be'),
       ('rue metsys', '108', 'schaerbeek', 'bruxelles', 'be'),
       ('rue du pré', '223', 'jette', 'bruxelles', 'be'),
       ('rue sergent sorenser', '65', 'ganshoren', 'bruxelles', 'be'),
       ('rue d''aumale', '38', 'anderlecht', 'bruxelles', 'be'),
       ('av de fré', '363', 'uccle', 'bruxelles', 'be'),
       ('rue de lisbonne', '52', 'saint gilles', 'bruxelles', 'be'),
       ('av neptune', '24', 'forest', 'bruxelles', 'be'),
       ('av mozart', '76', 'forest', 'bruxelles', 'be'),
       ('rue emile delva', '92', 'laeken', 'bruxelles', 'be'),
       ('av de la chasse', '68', 'etterbeek', 'bruxelles', 'be'),
       ('rue leopold 1', '42', 'laeken', 'bruxelles', 'be'),
       ('av charle woeste', '68', 'jette', 'bruxelles', 'be'),
       ('ch de boondael', '12', 'ixelles', 'bruxelles', 'be')
GO

INSERT INTO [user_dance_style] ([id_user], [style_ref])
VALUES (1, 1), (1, 2), (1, 3),
       (2, 1), (2, 2), (2, 3),
       (3, 1), (3, 2),
       (4, 1), (4, 2), (4, 3),
       (5, 2), (5, 3),
       (6, 1), (7, 3), (8, 3),
       (9, 1), (9, 2), (9, 3),
       (10, 1), (10, 2), (10, 3),
       (11, 3), (12, 2), (13, 2),
       (14, 1), (15, 3), (16, 1)
GO

INSERT INTO [dance]([dancer_1_id_user], [dancer_2_id_user], [dance_dtg], [style_ref])
VALUES (1, 2, convert(datetime, '2019-11-24 10:34:09 PM',20), 3),
       (4, 2, convert(datetime, '2019-11-24 10:50:00 PM',20), 3),
       (3, 5, convert(datetime, '2019-11-24 10:35:00 PM',20), 2),
       (6, 1, convert(datetime, '2019-11-24 10:37:00 PM',20), 1),
       (7, 2, convert(datetime, '2019-11-24 10:37:00 PM',20), 3),
       (8, 1, convert(datetime, '2019-12-03 11:20:03 PM',20), 3),
       (9, 3, convert(datetime, '2019-12-23 10:45:00 AM',20), 1),
       (10, 12, convert(datetime, '2019-12-26 11:20:00 AM',20), 2),
       (11, 4, convert(datetime, '2020-01-02 08:45:00 AM',20), 3),
       (12, 5, convert(datetime, '2020-01-02 11:10:04 AM',20), 2),
       (13, 12, convert(datetime, '2020-02-04 09:25:00 PM',20), 2),
       (14, 10, convert(datetime, '2020-02-25 10:45:00 AM',20), 1),
       (2, 14, convert(datetime, '2020-02-25 08:45:00 PM',20), 1),
       (5, 10, convert(datetime, '2020-03-01 11:15:06 AM',20), 2),
       (17, 2, convert(datetime, '2020-03-04 03:15:06 AM',20), 1)
GO

If I run:

SELECT
    [user_dance_style].[id_user], 
    STRING_AGG([user_dance_style].[style_ref], ', ') AS style
FROM  
    [user_dance_style]
GROUP BY  
    [user_dance_style].[id_user]

I get these results:

The id_user 1 has all 3 styles trying to insert with:

insert into [user_dance_style]([id_user], [style_ref])
values (1,1)

This throws an error

Msg 3903, Level 16, State 1, Procedure update_style, Line 16
The ROLLBACK TRANSACTION request has no corresponding BEGIN TRANSACTION.

but the error should be

The transaction ended in the trigger. The batch has been aborted.

The id_user 3 has only style 1 and 2 so I should be able to insert a style 3 but when I try:

insert into [user_dance_style]([id_user], [style_ref])
values (3,3)

I got the error:

The transaction ended in the trigger. The batch has been aborted.

How can I fix the code to achieve the desired result?

Answer 1

As noted in the comments the inserted pseudo table could have 0-N rows in it, so you cannot assume a single row. And as with all T-SQL you should look for a set based operation before using a procedural one.

The actual error you reported was because even though you called ROLLBACK you didn't RETURN and therefore the trigger continued running and attempted a second ROLLBACK which is what caused your error.

In the set based solution below I am considering all users involved in the update, grouping by them, then using the having clause to exclude any users with 3 or less rows (first test).

And in the second test I am doing the same thing, but now grouping by style_ref as well, and checking for more than 1 row - because at this point in the trigger, the INSERT that fired the trigger has already taken place, so any duplicate row is already in the table, hence why checking for more than one row.

Here is your corrected trigger, using set based logic:

CREATE TRIGGER update_style 
ON [user_dance_style] 
AFTER INSERT
AS
BEGIN
    SET NOCOUNT ON;

    -- Check for more then 3 records
    -- Not actually required, because already covered by the next check
    /*if exists (
        SELECT 1
        FROM [user_dance_style]
        WHERE id_user = (select id_user FROM inserted)
        group by id_user
        having count(*) > 3
    ) BEGIN
        ROLLBACK;
        RETURN;
    END;*/

    -- Check whether a user has multiple identical records
    -- Because by this point the INSERT has taken place
    if exists (
        SELECT 1
        FROM [user_dance_style]
        WHERE id_user = (select id_user FROM inserted)
        group by id_user, style_ref
        having count(*) > 1
    ) BEGIN
        ROLLBACK;
        -- As mentioned by Smor in a real example you would throw an error here (and remove the RETURN as its no longer needed).
        -- THROW 51000, 'Style failed to be inserted as it already exists.', 1; 
        RETURN;
    END;
END

Note: Having re-read the question I think you only need the second check because it automatically covers the first. And as others have said a unique constraint would be the best way to accomplish this - assuming that it allowed for your assignment.

Answer 2

Note! You asked only about INSERT which is cover using triggers well. Since I cannot add this as comment, I will add it as "answer" since it is important and maybe it is what you actually need

If you want to cover any change data in the table and not only INSERT (UPDATE as well for example), then there is no reason to use triggers. Triggers has multiple cases which are not enforced (for example bulk insert)

In this case, all you need is simple 2 constraint (assuming I understood your need)

-- On insert, I want to check if the id_user has already three styles, if so ROLLBACK
-- I also want to check if the id_user has already the style to be inserted in that case ROLLBACK.
ALTER TABLE user_dance_style
    ADD CHECK (style_ref = 1 or style_ref = 2 or style_ref = 3);
GO
ALTER TABLE user_dance_style
    ADD CONSTRAINT UC_Person UNIQUE (id_user, style_ref);
GO

Since you can have only the values 1,2,3 and you have UNIQUE for each user, this confirm that you can only have 1, or , 2 , or 3 for each user and each one of these can appear once only.