1

It was found that this code mistake caused an infinite loop on PROD:

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!

  • 1
    Add a timer but if you are going to add code, just fix it. – scsimon Nov 5 at 17:17
  • You could try unit testing your code/procedures. – LowlyDBA Nov 5 at 17:27
  • 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. – Elaskanator Nov 5 at 17:47
  • 1
    There is, unfortunately, nothing innate to SQL Server that will catch this. There may be some bizarre combination of requirements (probably involving nested cursors - you may now shudder) that would require this logic. Since this is true, having a built-in limitation on the functionality makes no sense. Also, it isn't in the SQL language specification. And finally, since the control structure is not an innate part of the looping logic (as it is in the for construct), there is no reasonable way to implement the restriction within the concepts being used here. – Laughing Vergil Nov 5 at 18:49
  • 2
    To clarify the last concept - it would be perfectly legal code to create a cursor, FETCH NEXT from it three times in three different bits of code, and dispose of the cursor, with no looping at all. It may even be useful in some obscure circumstance that I can't even imagine right now (that is left as an exercise for the reader.) But it is perfectly legitimate SQL code. – Laughing Vergil Nov 5 at 18:52
4

To avoid problems like this I'm using different patter for fetch:

OPEN Cursor;

WHILE 1=1
BEGIN 

    FETCH NEXT FROM Cursor INTO [...];
    IF @@FETCH_STATUS <> 0 BREAK;

    [...]

END;

CLOSE Cursor;
DEALLOCATE Cursor;

In this pattern you have FETCH exactly once and then you need to check fetch status once too.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.