SQL Server WHILE loop runs twice with update, once without

So this is weird. Here’s my code:

PRINT 'Define cursor'
DECLARE cursor1 CURSOR FOR
SELECT  b.EmploymentTypeID
FROM    EmploymentTypes b INNER JOIN #ListEmployments l
        on  b.EmploymentID = l.EmploymentID

PRINT 'Open cursor'
OPEN cursor1
FETCH NEXT FROM cursor1 INTO @pEmploymentTypeID

WHILE @@FETCH_STATUS = 0
BEGIN
 PRINT '1) Inside WHILE loop.  @pEmploymentTypeID: ' + convert(varchar(20), @pEmploymentTypeID)
 PRINT '2) Inside WHILE loop.  @@FETCH_STATUS: ' + convert(varchar(20), @@FETCH_STATUS)
 -- do some work
 UPDATE EmploymentTypes
       SET  EmploymentTypeRD = EmploymentTypeID
    WHERE EmploymentTypeID = @pEmploymentTypeID
      AND EmploymentTypeRD = 0

 PRINT '3) Inside WHILE loop.  Pre-FETCH @@FETCH_STATUS: ' + convert(varchar(20), @@FETCH_STATUS)
 FETCH NEXT FROM cursor1 INTO @pEmploymentTypeID
 PRINT '4) Inside WHILE loop.  Post-FETCH @@FETCH_STATUS: ' + convert(varchar(20), @@FETCH_STATUS)
  
END
CLOSE cursor1
DEALLOCATE cursor1

When I run this, the loop executes twice and I see the following output.

Define cursor
Open cursor
1) Inside WHILE loop.  @pEmploymentTypeID: 695837
2) Inside WHILE loop.  @@FETCH_STATUS: 0
3) Inside WHILE loop.  Pre-FETCH @@FETCH_STATUS: 0
4) Inside WHILE loop.  Post-FETCH @@FETCH_STATUS: 0
1) Inside WHILE loop.  @pEmploymentTypeID: 695837
2) Inside WHILE loop.  @@FETCH_STATUS: 0
3) Inside WHILE loop.  Pre-FETCH @@FETCH_STATUS: 0
4) Inside WHILE loop.  Post-FETCH @@FETCH_STATUS: -1
Ran in ROLLBACK

If I comment out the UPDATE statement following the “– do some work” comment, the output is:

Define cursor
Open cursor
1) Inside WHILE loop.  @pEmploymentTypeID: 695837
2) Inside WHILE loop.  @@FETCH_STATUS: 0
3) Inside WHILE loop.  Pre-FETCH @@FETCH_STATUS: 0
4) Inside WHILE loop.  Post-FETCH @@FETCH_STATUS: -1
Ran in ROLLBACK

Why does the loop run twice with the update enabled?

You’ve run into a problem better known as the Halloween problem, in other words you are reading rows that you have just modified.

This is one of the many reasons you should not use a cursor.

Instead do a simple joined update. This is a single statement that performs everything in your script. The server has specific logic to take into account rows being modified while being read.

UPDATE et
SET EmploymentTypeRD = EmploymentTypeID
FROM EmploymentTypes et
INNER JOIN #ListEmployments l ON et.EmploymentID = l.EmploymentID
WHERE et.EmploymentTypeRD = 0;

A SET based operation would be best for this. However, if you must use a cursor, or simply want to know why it’s not working as you’d expect…

You’re updating rows to the underlying tables used by the cursor and then causing it to return an extra row after the first update. The update during the second loop does nothing because EmploymentTypeRD is no longer 0, then the cursor exists. Try adding the STATIC argument to the cursor declaration. This will cache the results of the cursor to a temp table, in the state they existed before the loop begins and any updates are run.

Review microsoft’s documentation on cursor arguments here: https://docs.microsoft.com/en-us/sql/t-sql/language-elements/declare-cursor-transact-sql?view=sql-server-ver15

Below is the altered code.

PRINT 'Define cursor'
DECLARE cursor1 CURSOR **STATIC** FOR
SELECT  b.EmploymentTypeID
FROM    EmploymentTypes b INNER JOIN #ListEmployments l
        on  b.EmploymentID = l.EmploymentID

PRINT 'Open cursor'
OPEN cursor1
FETCH NEXT FROM cursor1 INTO @pEmploymentTypeID

WHILE @@FETCH_STATUS = 0
BEGIN
 PRINT '1) Inside WHILE loop.  @pEmploymentTypeID: ' + convert(varchar(20), @pEmploymentTypeID)
 PRINT '2) Inside WHILE loop.  @@FETCH_STATUS: ' + convert(varchar(20), @@FETCH_STATUS)
 -- do some work
 UPDATE EmploymentTypes
       SET  EmploymentTypeRD = EmploymentTypeID
    WHERE EmploymentTypeID = @pEmploymentTypeID
      AND EmploymentTypeRD = 0

 PRINT '3) Inside WHILE loop.  Pre-FETCH @@FETCH_STATUS: ' + convert(varchar(20), @@FETCH_STATUS)
 FETCH NEXT FROM cursor1 INTO @pEmploymentTypeID
 PRINT '4) Inside WHILE loop.  Post-FETCH @@FETCH_STATUS: ' + convert(varchar(20), @@FETCH_STATUS)
  
END
CLOSE cursor1
DEALLOCATE cursor1