It was found that this code mistake caused an infinite loop in production:
DECLARE @BatchID INT
DECLARE MyCursor CURSOR FOR
SELECT BatchID = ...
OPEN MyCursor
FETCH NEXT FROM MyCursor INTO @BatchID
WHILE @@FETCH_STATUS = 0
BEGIN
...
IF [condition]
BEGIN
...
FETCH NEXT FROM MyCursor INTO @BatchID
END
END
CLOSE MyCursor
DEALLOCATE MyCursor
Assuming the need for a cursor is warranted, is there any way to safeguard against this mistake (besides more testing/code review)?
In other languages, we have FOREACH
loops that manage progressing the iteration for us, or FOR
loops that have an afterthought, so is there any equivalent in SQL Server that prevents sloppy mistakes like misplacing (or forgetting!) it?
I have never seen custom loops in SQL need to do anything fancy with the cursor, and WHILE
loops have the same risk (along with cases like DELETE #WorkData WHERE ID = @BatchID
when @BatchID is NULL
), so how might this risk be mitigated programmatically/cleanly for the typical use case?
An approach like this is very unappealing:
DECLARE @BatchID INT
DECLARE MyCursor CURSOR FOR
SELECT BatchID = -1--dummy entry to always be skipped
UNION ALL SELECT BatchID = ...
OPEN MyCursor
FETCH NEXT FROM MyCursor INTO @BatchID
WHILE @@FETCH_STATUS = 0
BEGIN
FETCH NEXT FROM MyCursor INTO @BatchID
IF @@FETCH_STATUS = 0
BEGIN
...
END
END
CLOSE MyCursor
DEALLOCATE MyCursor
It seems to me that checking @@FETCH_STATUS
twice in a row without any cursor position/control statements in-between could be an effective way for SQL Server to guess if there was such a mistake, as my experience with cursors has never seen two checks in a row intentionally (without nested cursors at least, but those still have control statements like OPEN
and CLOSE
between checks of @@FETCH_STATUS
of the outer cursor).
P.S. The [condition]
this time was equivalent to "not the DST transition day" (which was this Sunday). More time zone bugs!
I am most interested about language features to achieve the same automatically-provided guarantee of how other languages handle the afterthought clause of a for-loop (e.g. the i++
in for (int i = 0; i < myArray.Length; i++)
). The fewer things the individual is responsible for, the fewer places there are to make a mistake, and it would be a monumental task to audit the literally thousands of stored procedures in our system, especially since many of them are inappropriately iterating data instead of using set-based logic already.