SQL Server Cursors - Avoiding the Duplicate FETCH NEXT

The example below comes from SQL Server Books Online (or just BOL as it’s affectionately known) about how to use a cursor. I have trimmed the predicates and the body of the cursor loop to remove some unnecessary noise, but it essentially remains the same:-

DECLARE authors_cursor CURSOR FOR
SELECT au_id, au_fname, au_lname
FROM authors

OPEN authors_cursor

FETCH NEXT FROM authors_cursor
INTO @au_id, @au_fname, @au_lname

WHILE @@FETCH_STATUS = 0
BEGIN
  --
  -- stuff done here with inner cursor elided
  --

  -- Get the next author. 
  FETCH NEXT FROM authors_cursor 
  INTO @au_id, @au_fname, @au_lname
END

CLOSE authors_cursor
DEALLOCATE authors_cursor
GO

What bugs me about this code, and other examples I’ve come across[*], is that there is obvious duplication; the FETCH NEXT statement is specified twice - once before entering the loop and again at the bottom.

Now, as a software developer we are taught that cut-and-paste coding is frowned upon because you increase the cost of maintenance. Yes, it’s just a simple loop and no one is really going to forget to keep them in sync are they (especially if SQL is their bread and butter)... But, if we did want to remove the duplication, could we?

Of course we can and the revised version below is how a colleague has tackled this for many years - we just need to move the FETCH NEXT inside the loop and then change the loop condition. But this is where we swap one dubious construct (duplicate code) for another - the infinite loop:-

OPEN authors_cursor

WHILE (1 = 1) 
BEGIN
  -- Get the next author. 
  FETCH NEXT FROM authors_cursor 
  INTO @au_id, @au_fname, @au_lname 

  IF (@@FETCH_STATUS <> 0)
    BREAK

  --
  -- stuff done here with inner cursor elided
  --
END

CLOSE authors_cursor

If this code were written in a static language like C++ and you ran a Lint tool over it, it would probably wince at the (1 = 1) expression - I certainly did when I first came across it. There are some constructs that send shivers down your spine and the infinite loop is one of them because we expect the loop condition to have a natural point of exit[#]. Here is an attempt to set that record straight:-

DECLARE @status int = 0

WHILE (@status = 0) 
BEGIN
  -- Get the next author. 
  FETCH NEXT FROM authors_cursor 
  INTO @au_id, @au_fname, @au_lname 

  SET @status = @@FETCH_STATUS

  IF (@status = 0)
  BEGIN
    --
    -- stuff done here with inner cursor elided
    --
  END
END

Single fetch statement, check. No early loop exit, check. No duplication, che... Sadly we’ve just swapped duplication of the FETCH NEXT for duplication of the loop test. Hmmm, what about swapping the BREAK for a CONTINUE and then changing the loop tests:-

DECLARE @done bit = 0

WHILE (@done = 0) 
BEGIN
  -- Get the next author. 
  FETCH NEXT FROM authors_cursor 
  INTO @au_id, @au_fname, @au_lname 

  IF (@@FETCH_STATUS <> 0)
  BEGIN
    SET @done = 1
    CONTINUE
  END

  --
  -- stuff done here with inner cursor elided
  --
END

To be honest I think that’s cheating because we’ve still got two tests to control the loop. But the switch from a BREAK to CONTINUE somehow feels a little less dirty. Sadly my SQL skills are fairly average and so perhaps someone more experienced can come along and finish the job off properly.

 

[*] Hey, it’s just example code and examples are meant to be simple so that people can understand them! True, I’m just saying that I’ve not seen it done any other way when Googling for stuff that utilised cursors which is what prompted me to write it up.

[#] Even services that sit there and spin indefinitely handling requests still need to be coded to terminate gracefully at some point and so in the very least you’ll be waiting on a synchronisation object of some sort.