SQL Server - is it not instant to remove from table variables?
I inherited a stored procedure persistence that is executed every night using a SQL Agent job. It has been working well for several months, but all the sudden last nights have chased some work and missed some work.
The work is done in the middle of the night, and during this time there are no users. I restored a database backup just before the problematic run to a test server, restarted the procedure and everything worked fine. This is small data too, maybe 100-200 lines per night.
Here is a representation of one of the loops in the procedure where the problem occurred:
DECLARE @uniqueId int
DECLARE @examId int
DECLARE @TempSingleContactTable TABLE
(
uniqueId int IDENTITY(1,1) PRIMARY KEY,
examId int not null,
contactEmail nvarchar(max) null,
)
[data inserted into @TempSingleContactTable]
WHILE EXISTS (SELECT * FROM @TempSingleContactTable)
BEGIN
Select top 1 @uniqueId = uniqueId,
@examId = examID, from @TempSingleContactTable
[*****PROBLEM HERE- this line with same value for @examId ran multiple times, but eventually continued]
DELETE FROM @TempSingleContactTable WHERE examID = @examId
END
The only thing I can see can cause the problem above if the DELETE call doesn't work. Is it possible that a DELETE call against a table variable is not instantaneous?
EDIT: Any information on what could cause "Remove from @TempSingleContactTable" to fail is greatly appreciated.
EDIT 2: Additional research showed that this automated one-off procedure failed the same twice in two months. Interestingly, every time it failed, the previous evening didn't change any data, and it always had to. Unfortunately there is no check-in information to determine what may have caused the problems of the past nights. It looks like they should be related, although it could be a red herring. I added an entry hoping to get the actual root cause.
source to share
It looks like you inherited a "bad cursor". People have heard that cursors are "evil" and then they come to this = (I'm not going to start a discussion about how set-based operations are preferable to cursor-based operations (read: line by line). In some situations, you have there is simply no choice; perhaps that too.
Converting your loop to a decent cursor will probably "stabilize" that part of the loop already; but it also shows immediately that there is a "problem" with your loop.
At first glance, the equivalent cursor would be:
DECLARE @uniqueId int
DECLARE @examId int
DECLARE @TempSingleContactTable TABLE
(
uniqueId int IDENTITY(1,1) PRIMARY KEY,
examId int not null,
contactEmail nvarchar(max) null
)
-- [data inserted into @TempSingleContactTable]
DECLARE exams_loop CURSOR LOCAL FAST_FORWARD
FOR SELECT uniqueId, examID
FROM @TempSingleContactTable
OPEN exams_loop
FETCH NEXT FROM exams_loop INTO @uniqueId, @examId
WHILE @@FETCH_STATUS = 0
BEGIN
-- internals...
FETCH NEXT FROM exams_loop INTO @uniqueId, @examId
END
CLOSE exams_loop
DEALLOCATE exams_loop
But upon closer inspection, there is a catch: the end of your loop deletes all records for the given one examID
. Therefore, if there are several records with the same examID
, it means that some values uniqueID
will be skipped. (side note: he's not even sure which ones are never tempted to rely on them, being in natural order, because there is a PC on the pitch!)
Thus, the following code is a better replacement:
DECLARE exams_loop CURSOR LOCAL FAST_FORWARD
FOR SELECT MIN(uniqueId), examID
FROM @TempSingleContactTable
GROUP BY examID
OPEN exams_loop
FETCH NEXT FROM exams_loop INTO @uniqueId, @examId
WHILE @@FETCH_STATUS = 0
BEGIN
-- internals...
FETCH NEXT FROM exams_loop INTO @uniqueId, @examId
END
CLOSE exams_loop
DEALLOCATE exams_loop
This time it will be the lowest uniqueID
that wins instead of random, but in all fairness I think that repeatability (which we are really talking about here) should be preferred over randomness.
Anyway, in short:
- rather, use a real cursor instead of a bad person replacement because it is a bad replacement to start with
- If you really want to keep the loop as it is now, change the table definition to this:
=>
DECLARE @TempSingleContactTable TABLE
(
uniqueId int IDENTITY(1,1) PRIMARY KEY,
examId int not null UNIQUE (examId, uniqueId),
contactEmail nvarchar(max) null
)
this way you will at least have an index on the field when you delete. (although I strongly discourage heavy operations on @ table-variables, they tend to go south when you put "average" data in there, let alone start doing operations on it. .. # temp-tables is much more reliable in this regard!)
source to share