Trigger that updates counter

Question

I've got the following schema, which can not be modified since it's a problem taken from an exam:

Player
-------
player_id    (PK)
player_name
player_games_total
player_games_won

Competition
-----------
comp_id      (PK)
comp_player1 (FK references Player.player_id, NOT NULL)
comp_player2 (FK references Player.player_id, NULL)
comp_player3 (FK references Player.player_id, NULL)
comp_player4 (FK references Player.player_id, NULL)
comp_player5 (FK references Player.player_id, NULL)
comp_winner  (integer from 1 to 5 or NULL, indicates which player, if any, won)

For example, the following row in the Competition table:

1  |  23  |  42  |  NULL  |  NULL  |  NULL  |  2

indicates that only players 23 and 42 participated in competition 1 and player 42 won.

The exercise stated:

A trigger needs to be created that updates Player.player_games_total and Player.player_games_won with every new competition inserted in Competitions. The trigger should handle correctly individual INSERTs as well as bulk INSERTs. In other words, don't assume the INSERTED table will contain only one row at any given time.

The solution I wrote is in the following paragraph, but first I'd like to make a few comments. Programming is not new to me but SQL is. I know about coding best practices and like to think that I can recognize good, clear code when I see it. I definitely don't like what I see in my solution, but I don't have enough experience with SQL to know whether it's because of my own limitations with the language or because the schema is not ideal and is forcing me to make queries like this.

CREATE TRIGGER [dbo].[tr_aff_ins_competitions]
ON [dbo].[COMPETITIONS]
AFTER INSERT
AS BEGIN

-- This is the part of the trigger that updates 'player_games_total' values

    update dbo.Player
    set player_games_total = player_games_total + ( select count(comp_player1)
                                                    from inserted
                                                    where Player.player_id = inserted.comp_player1)
    where Player.player_id IN (select comp_player1 from inserted)

    update dbo.Player
    set player_games_total = player_games_total + ( select count(comp_player2)
                                                    from inserted
                                                    where Player.player_id = inserted.comp_player2)
    where Player.player_id IN (select comp_player2 from inserted)

    update dbo.Player
    set player_games_total = player_games_total + ( select count(comp_player3)
                                                    from inserted
                                                    where Player.player_id = inserted.comp_player3)
    where Player.player_id IN (select comp_player3 from inserted)

    update dbo.Player
    set player_games_total = player_games_total + ( select count(comp_player4)
                                                    from inserted
                                                    where Player.player_id = inserted.comp_player4)
    where Player.player_id IN (select comp_player4 from inserted)

    update dbo.Player
    set player_games_total = player_games_total + ( select count(comp_player5)
                                                    from inserted
                                                    where Player.player_id = inserted.comp_player5)
    where Player.player_id IN (select comp_player5 from inserted)


-- This is the part of the trigger that updates 'player_games_won' values

    update dbo.Player
    set player_games_won = player_games_won + ( select COUNT(winner) 
                                                from dbo.COMPETITIONS comp_sub
                                                where comp_sub.winner = 1
                                                and comp_sub.comp_player1 = player_id)
    where Player.player_id IN (select comp_player1 from inserted)

    update dbo.Player
    set player_games_won = player_games_won + ( select COUNT(winner) 
                                                from dbo.COMPETITIONS comp_sub
                                                where comp_sub.winner = 2
                                                and comp_sub.comp_player2 = player_id)
    where Player.player_id IN (select comp_player2 from inserted)

    update dbo.Player
    set player_games_won = player_games_won + ( select COUNT(winner) 
                                                from dbo.COMPETITIONS comp_sub
                                                where comp_sub.winner = 3
                                                and comp_sub.comp_player3 = player_id)
    where Player.player_id IN (select comp_player3 from inserted)

    update dbo.Player
    set player_games_won = player_games_won + ( select COUNT(winner) 
                                                from dbo.COMPETITIONS comp_sub
                                                where comp_sub.winner = 4
                                                and comp_sub.comp_player4 = player_id)
    where Player.player_id IN (select comp_player4 from inserted)

    update dbo.Player
    set player_games_won = player_games_won + ( select COUNT(winner) 
                                                from dbo.COMPETITIONS comp_sub
                                                where comp_sub.winner = 5
                                                and comp_sub.comp_player5 = player_id)
    where Player.player_id IN (select comp_player5 from inserted)

END

Answer 1

Your code looks fine (the schema is not very developer friendly), but applying the DRY principle, it can become smaller allowing addition of players with little effort:

alter TRIGGER [dbo].[tr_aff_ins_competition]
ON [dbo].[competition]
AFTER INSERT
AS BEGIN
    DECLARE @SQL NVARCHAR(4000)

    SELECT * INTO #tmp
    FROM inserted

    DECLARE @index INT = 1
    WHILE (@index <= 5)
    BEGIN
        DECLARE @indexStr VARCHAR(10) = CAST(@index AS VARCHAR)

        SET @SQL = '
            update dbo.Player
            set player_games_total = player_games_total + ( select count(comp_player' + @indexStr + ')
                                                            from #tmp
                                                            where Player.player_id = #tmp.comp_player' + @indexStr + ')
            where Player.player_id IN (select comp_player' + @indexStr + ' from #tmp)

            update dbo.Player
            set player_games_won = player_games_won + ( select COUNT(1) 
                                                        from dbo.competition comp_sub
                                                        where comp_sub.comp_winner = ' + @indexStr + '
                                                        and comp_sub.comp_player' + @indexStr + ' = player_id)
            where Player.player_id IN (select comp_player' + @indexStr + ' from #tmp)'
        EXEC (@SQL)

        SET @index = @index + 1
    END
END
GO

Answer 2

Joining updates

It looks like you should extend your queries to sum across all records with an extended where clause. Something like the following:

UPDATE dbo.Player
SET player_games_total = player_games_total + 
      ( SELECT count(*) FROM inserted
         WHERE
               inserted.comp_player5 = Player.player_id
            OR inserted.comp_player4 = Player.player_id
            OR inserted.comp_player3 = Player.player_id
            OR inserted.comp_player2 = Player.player_id
            OR inserted.comp_player1 = Player.player_id )

WHERE Player.player_id IN (
         SELECT comp_player5 FROM inserted
   UNION SELECT comp_player4 FROM inserted
   UNION SELECT comp_player3 FROM inserted
   UNION SELECT comp_player2 FROM inserted
   UNION SELECT comp_player1 FROM inserted
) -- You might need to alias all id's to the same name...

See UNION (Transact-SQL) for more information on union.

A similar approach should be possibly for the other update, if I'm not mistaken and misunderstand your request.

Doing this would remove the need to update the same dbo.Player row more than once (or possibly twice for the games won as separate query) instead of five (or ten) which you do today. The select's will be almost the same, but union'ised. Since select's are cheaper than update's, this should result in a performance gain.

If you opt to, you should be able to extend this pattern using loops like indicated in the other answer by Alexei.