Search code examples
sql-serverstored-proceduresconcurrencytransactionslocking

Read, Increment and Update Transaction in SQL Server


To implement special Id, we need to append current date with an identity counter which resets per day. As an example, the Id must be something like 20240825-0001 for the first record of August 25th and it must increment per record insertion.

Since current table has an identity column (integer with auto increment) along with a CreatedDate column of datetime type, I tried the following query to negate the need for a secondary intermediate table:

SELECT 
    ROW_NUMBER() OVER(PARTITION BY Cast(CreatedDate AS DATE) ORDER BY CreatedDate ASC) AS Row#,
    Id, 
    CreatedDate
FROM 
    MyBaseTable
WHERE
    CreatedDate > CAST('2024-08-25' AS DATE) 

Yet, it fails to provide required concurrency.

My second attempt is to write an stored procedure to hold first available value to use while inserting records. The procedure looks like this:

CREATE PROCEDURE [dbo].[GetSpecialId]
    (@CreatedDate date)
AS
BEGIN
    SET TRANSACTION ISOLATION LEVEL REPEATABLE READ

    BEGIN TRAN
        DECLARE @CurrentValue INT
        SET @CurrentValue = 1

        -- Insert initial row if necessary 
        IF NOT EXISTS (SELECT 1 FROM [dbo].[IndexingTable] 
                       WHERE CreationDate = @CreatedDate)
        BEGIN
            INSERT INTO [dbo].[IndexingTable] (CreationDate,  AvailableValue) 
            VALUES (@CreatedDate, 1)

            IF @@Error <> 0
            BEGIN
                ROLLBACK TRAN
                RETURN -1
            END 
        END
        ELSE
        BEGIN
            SELECT @CurrentValue = AvailableValue  
            FROM [dbo].[IndexingTable]

            UPDATE [dbo].[IndexingTable] 
            SET AvailableValue = AvailableValue + 1
            WHERE CreationDate = @CreatedDate 

            IF @@Error <> 0
            BEGIN
                ROLLBACK TRAN
                RETURN -1
            END
        END

        COMMIT TRAN

        SET TRANSACTION ISOLATION LEVEL REPEATABLE READ

        RETURN @CurrentValue
    END

Then, calling this stored procedure before creating a new record in the MyBaseTable.

I am just checking if I have missed anything regarding concurrency issues or any better solution.

My primary concern is just an incident happened earlier when the same code outputting fine and incrementing per call, yet no record was inserted in the IndexingTable, which was totally strange, as my debugger from visual studio hung and failed to resume or even cancel query execution, and every thing returned to normal after SQL Server service restart!

I am using SQL Server 2019 with latest cumulative updates.


Solution

  • There are a number of locking issues here.

    • Firstly, as noted by @AaronBertrand, the IF (NOT) EXISTS... INSERT ELSE ... UPDATE... sequence is unnecessarily complex and badly performing. You can just use @@ROWCOUNT after the UPDATE to check if it worked, then conditionally insert otherwise.

    • You need a clustered index (preferably a primary key) on CreationDate for this to work correctly without deadlocking.

    • Furthermore, REPEATABLE READ is not enough here. The server doesn't know the SELECT @CurrentValue = is being used to hold the previous value, so it only places a shared S-lock, which is not enough here, you need at least a U-lock. T place a U-lock you need WITH (UPDLOCK). And using SERIALIZABLE would be even better for consistency.

    • An even better option is to just set the variable at the same time as doing the UPDATE, which gives it the value before the update happens, unless you do @variable = column = newvalue in which case you get the value afterwards.
      There also seems to be an issue in that your current code gives the previous value in case of an UDPATE, and the new value in case of an INSERT. You need to decide which you want.

    • Finally, the error handling is unnecessary. Just place SET XACT_ABORT ON; at the top, and the server will handle rollbacks automatically.

    • You probably also want to return the result back from the procedure, for that you can use an OUTPUT parameter.

    CREATE PROCEDURE dbo.GetSpecialId
        @CreatedDate date,
        @CurrentValue int = 0 OUTPUT
    AS
    
    SET XACT_ABORT, NOCOUNT ON;
    
    BEGIN TRAN;
    
    UPDATE dbo.IndexingTable WITH (UPDLOCK, SERIALIZABLE)
    SET @CurrentValue = AvailableValue,  -- gives the old value
        AvailableValue = AvailableValue + 1
    -- for the new value use a single line:
    --  @CurrentValue = AvailableValue = AvailableValue + 1
    WHERE CreationDate = @CreatedDate;
    
    IF @@ROWCOUNT = 0
    BEGIN
        INSERT INTO dbo.IndexingTable
          (CreationDate, AvailableValue) 
        VALUES
          (@CreatedDate, 1);
    END;
    
    COMMIT;
    

    Having said all that, I'm not sure what exactly your issue was with the ROW_NUMBER solution. The only problem with it that I can see is that it's not stable: if you delete a row then later rows will get re-numbered, although that may be desired anyway.