Mdthbs

Transactional database used for booking things...

Our vendor was asked to replace #temptables with @tablevariables (because of heavy compile locks) but instead they replaced with an actual table that adds SPID as a column to ensure the stored procedure only acts on the applicable rows.

Do you see any risk in this method of operation? Before all transactions were isolated within their own transaction... I worried we may end up locking this table a bunch but their opinion is that SQL uses row-level locking and this won't create more locks.

CREATE TABLE dbo.qryTransactions (

    ID int IDENTITY (0,1) NOT NULL CONSTRAINT pk_qryTransactions PRIMARY KEY CLUSTERED,

    spid int NOT NULL,

    OrderID int,

    ItemID int,

    TimeTransactionStart datetime,

    TimeTransactionEnd datetime,

...other fields

    )



CREATE INDEX idx_qryTransactions_spidID ON qryTransactions (spid, ID) INCLUDE (ItemID, OrderID, TimeTransactionStart, TimeTransactionEnd)

It seems to me using the @@SPID like that is asking for trouble.

Session ID's are reused frequently; as soon as a user connection logs out, that session ID is available to be used again, and is likely to be used by the next session attempting to connect.

To make it work at least semi-reliably, you'd need a login trigger that purges prior rows from the table with the same @@SPID. If you do that, you're likely to see a lot of locking on the table using the @@SPID column.

SQL Server does indeed use row locking, but it also uses page locking, and table locking. Of course, you might be able to mitigate that via good indexing, but this still looks like an anti-pattern to me.

If the stored procedure is the only method used to access the affected tables, you could investigate using an application lock, via sp_getapplock to essentially serialize access to the relevant parts. Docs for sp_getapplock are here. Erik Darling has an interesting post about it here.

Yes, I do see risks. It is naive to count on SQL using Row Locking. For example, I'm pretty sure inserts and deletes will use page locks at least. SQL Engine chooses the lock type based on several factors and none of those factors include "their opinion".
Blanket solutions like changing temp tables to table variables are generally bad ideas also.
Table Variables are very useful in some situations but they do have limitations and performance issues. I prefer temp tables in most circumstances. Particularly when the table will hold more than a few dozen rows.
I would require the vendor to explain why the system experienced "heavy compile locks" and how that degraded performance. Remember, anytime you look you will find "heavy locks" of some sort. That does not necessarily mean the locks are a problem.
Max's comments about @@SPID are also important.
Additionally, the transaction model and error processing could be big issues. If your system experiences deadlocks or input data quality issues then the standard error processing can result in the session being terminated without the qryTransactions table being properly reset.
IMO the wrong solution approach to the original problem.