How to lock table in SQL Server to prevent other transactions from INSERT and avoid deadlocking?

Question

I'm writing a database trigger on Table A. In said trigger, I insert a new record into Table B by doing:

1. Look up the PK ID of Table B
2. Increment the PK ID and insert new row into Table B

(Note: Table B does not use Identity. That is, the PK ID is not auto-incrementing. This is not something I can change. Reason being is the database schema is vendor-controlled. They don't use identity in any of their tables (auto-incrementing). Vendor keeps track of the next ID when it inserts records. That is how they designed their app. I want to be consistent with their design practices, as veering off course and adding identity to the table could break during a patch or upgrade cycle.)

I want to ensure two things during my transaction:

1. Avoid deadlocks on Table B
2. Prevent other insert queries/transactions from inserting into Table B (PK ID should be consistent)

Keep in mind, in my use case, the PK ID is not an integer but a varchar; it is an alphanumeric value with a 3-letter prefix followed by several digits.

Here is my code:

CREATE TRIGGER [dbo].[AfterINSERT_TableA_Trigger]
   ON  [dbo].[TableA]
   AFTER INSERT
AS 
BEGIN
    -- SET NOCOUNT ON added to prevent extra result sets from
    -- interfering with SELECT statements.
    SET NOCOUNT ON;

    -- [Table A code goes here]

    -- Table B code

    -- Instruct SQL Server to rollback entire transaction and abort the batch when a run-time error occurs
    SET XACT_ABORT ON;

    BEGIN TRANSACTION TableB
    
    -- Ex: SUB1234567890
    DECLARE @subID VARCHAR(15)

    DECLARE @newSubID VARCHAR(15)

    -- Lookup last PK ID inserted into Table B
    SELECT
        @subID = submissionID
    FROM
        TableB WITH (TABLOCKX, HOLDLOCK)
    ORDER BY
        submissionID DESC

    -- Increment PK ID, separating prefix, increment by 1, and concatenate
   SELECT @newSubID = LEFT(@subID, 3) + CAST((RIGHT(@subID, LEN(@subID) - 3) + 1) AS VARCHAR(15))

   -- Insert new record into TableB
   INSERT TableB WITH (TABLOCKX, HOLDLOCK)
           (SubmissionID, Description, Status)
   VALUES
           (@newSubId, 'Test', 0)

   COMMIT TRANSACTION TableB

    -- Turn off rollback flag
    SET XACT_ABORT OFF;
END

I read that HOLDLOCK is equivalent to SERIALIZABLE (https://learn.microsoft.com/en-us/sql/t-sql/queries/hints-transact-sql-table?view=sql-server-ver15), so I'm not sure it makes a difference which one I use.

Does the code look OK such that it'll satisfy the criteria of avoiding deadlocks and blocking other insert transactions on Table B while my transaction is running? Do I need the WITH locks on both the SELECT and the INSERT statements? Should I be using Read Committed or similar at the transaction level instead of or in addition to?

To be clear, I don't want other insert transactions on the table to necessarily fail. I just want them to wait their turn until my transaction is complete, while keeping the PK ID column consistent.

Answer 1

You should use a sequence for this, and get the next value with NEXT VALUE FOR or sys.sp_sequence_get_range to reserve a range of values for a multi-row insert.

Sequences only provide a numeric value, but you can easily prefix the character code yourself.

An alternative would be an identity column with a computed column incorporating the prefix, but you have said you do not wish to go that route.

The advantage of sequences and identity columns is they are maintained by the system without participating in the current user transaction. It can be extremely difficult to avoid blocking and deadlocking using alternative code.

If you really want to avoid all the built-in solutions for this type of problem, another robust alternative is a manual sequence as I describe in an old article of mine about Sequence Tables.

It sounds like the vendor is using that type of approach for their other tables, but I would be very surprised if they have implemented it correctly. I have personally never seen it done properly in vendor code. Perhaps yours is a first.

In any case, if you really want to follow their pattern, you should look into that more closely. That said, you should probably contact them about your requirement instead of making your own changes, depending on your licencing agreement with them.

Answer 2

You have a number of major flaws in your trigger:

• It does not take into account there being multiple (or zero) rows in the inserted table.
• XACT_ABORT is always ON in a trigger anyway, and I can't see a reason to try turn it off.
• BEGIN TRAN \ COMMIT is also a waste of time because triggers run in the same transaction.
• You need HOLDLOCK, UPDLOCK to keep the lock on the table until the end of the transaction, TABLOCK is not necessary.
• I don't know the exact logic behind the numbering system, you may need to adjust it, but ROW_NUMBER would be necessary here, because you have multiple rows.

CREATE TRIGGER [dbo].[AfterINSERT_TableA_Trigger]
   ON  [dbo].[TableA]
   AFTER INSERT
AS 

    -- SET NOCOUNT ON added to prevent extra result sets from
    -- interfering with SELECT statements.
    SET NOCOUNT ON;

    IF (NOT EXISTS (SELECT 1 FROM inserted))
        RETURN;  -- early bail-out

    INSERT TableB
        (SubmissionID, Description, Status)
    SELECT
        CONCAT(
            LEFT(b.submissionID, 3),
            CAST(RIGHT(b.submissionID, LEN(b.submissionID) - 3) AS int) + ROW_NUMBER() OVER (ORDER BY (SELECT NULL)
        ),
        'Test',
        0
    FROM inserted i
    CROSS APPLY (
        SELECT TOP (1) submissionID
        FROM TableB WITH (HOLDLOCK, UPDLOCK)
        ORDER BY submissionID DESC
    ) b;

GO