Do I need lock-hints in my select-insert procedure?

Question

I have a stored procedure that inserts a row into a table if the row key doesn't already exist. It looks like this:

create proc EmployeeInsertIfNotExists
     (@id int, @name varchar(50)) 
as 
begin
    SET XACT_ABORT ON

    begin transaction

    if not exists(select * from tbl where id = @id)
        insert into tbl(id, name) 
        values(id, name)

    commit transaction
end

This stored procedure is really just two statements, a select and a possible insert. I both statements inside of a transaction so that nothing can happen in between them to cause an exception. The id column is a primary key, so I want to ensure that I don't insert the same id twice.

My question is: is this enough precaution to prevent problems? Do I need to put any hints in the select statement? If so, do I need HOLDLOCK, TABLOCKX? This is new material for me.

EDIT: Suggested answer

create proc EmployeeInsertIfNotExists
    (@id int, @name varchar(50)) 
as 
begin
    SET XACT_ABORT ON
    SET TRANSACTION ISOLATION LEVEL SERIALIZABLE

    begin transaction

    if not exists(select * from tbl where id = @id)
        insert into tbl(id, name) 
        values(id, name)

    commit transaction
end

score 1 · Accepted Answer · answered Jul 21 '16 at 17:38

1

You want to mark the transaction isolation level to serializable. Otherwise someone can insert a row with the same ID half-way through your transaction. This is known as a "phantom row".

You don't need to lock the whole table. By using the correct isolation level, SQL Server can be smarter about how it applies its locks.

answered Jul 21 '16 at 17:38

Jonathan Allen

68,373
70
259
447

So do I need to change my procedure to the second one I pasted in? Can I avoid transactions altogether by using a merge` statement? – user2023861 Jul 21 '16 at 17:58
Yes, your second version should work. No, merge is not atomic. It has the same problem as your original version. – Jonathan Allen Jul 21 '16 at 18:06
1

This is way overkill for what you are trying to do. Serializable will hurt (kill?) the performance of your machine. If id is has a unique constraint on it you will be guaranteed that no one else can insert more then one key value into that column. You can have a return value from the proc that indicates the insert failed and you can try again with another key value. – benjamin moskovits Jul 21 '16 at 19:46
Whether or not `Serializable` will actually hurt performance depends on a lot of factors including your indexing scheme, the amount of write contention, the type of lock SQL Server chooses to use, etc. Likewise the error handling and retry logic may be cheaper or more expensive depending on both those factors, how likely it is to need to retry, and whether or not deadlocks can occur. But in any event, both are far better than using a table lock. – Jonathan Allen Jul 21 '16 at 21:28

Do I need lock-hints in my select-insert procedure?

1 Answers1