CODEWITHSUNDEEP
sql-serverdata-integritylocks
RichardHowells
RichardHowells

Reputation: 9066

How do I get the right locks for this SQL?

My database is SQL Server 2005/8. In a booking system we have a limit of 24 bookings on an event. This code in a stored procedure checks: - that the current user (@UserId) is not already booked on the event (@EventsID) - that the current event has a current booking list of under 24 - inserts a new booking.

BEGIN TRANSACTION 
IF (((select count (*) from dbo.aspnet_UsersEvents with (updlock) 
      where UserId = @UserId and EventsId = @EventsId) = 0) 
AND  ((SELECT Count(*)  FROM dbo.aspnet_UsersEvents with (updlock) 
      WHERE EventsId = @EventsId) < 24))
BEGIN
  insert into dbo.aspnet_UsersEvents (UserId, EventsId) 
      Values (@UserId, @EventsId)
END
COMMIT

The problem is that it is not safe. Two users might perform the test simultaneously and conclude they can both book. Both insert a row and we end up with 25 bookings.

Simply enclosing it in a transaction does not work. I tried adding WITH (UPDLOCK) to the selects in the hope that one would take update locks and keep the other out. That does not work.

Upvotes: 3

Views: 160

Answers (2)

ta.speot.is
ta.speot.is

Reputation: 27214

Just do it in one statement, at READ COMMITTED or higher.

INSERT dbo.aspnet_UsersEvents
       (UserId,EventsId)
OUTPUT inserted.UserEventsId -- Or whatever, just getting back one row identifies the insert was successful
SELECT @UserId
       , @EventsId
 WHERE ( SELECT COUNT (*)
           FROM dbo.aspnet_UsersEvents
          WHERE UserId = @UserId
                AND EventsId = @EventsId ) = 0
       AND ( SELECT COUNT(*)
               FROM dbo.aspnet_UsersEvents
              WHERE EventsId = @EventsId ) < 24

Side note: your SELECT COUNT(*) for duplicate checking seems excessive, personally I'd use NOT EXISTS(SELECT NULL FROM ... WHERE UserID = ..., EventsID = ....

Upvotes: 1

Mark Storey-Smith
Mark Storey-Smith

Reputation: 997

Three options:

  1. SET TRANSACTION ISOLATION LEVEL REPEATABLE READ
  2. Change the lock hint to WITH (UPDLOCK, HOLDLOCK)
  3. Add a unique constraint to dbo.aspnet_UsersEvents and a TRY/CATCH around the insert.

You can use the following script to confirm that the lock is taken and immediately released if you omit HOLDLOCK. You will also see that the lock is not released (no 'releasing lock reference on KEY' output) when HOLDLOCK is used.

(Gist script)

Upvotes: 3

Related Questions