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). <sub>P.S. The `[condition]` this time was equivalent to "not the DST transition day" (which was this Sunday). More time zone bugs!</sub> 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. : https://stackoverflow.com/questions/58141/why-is-it-considered-bad-practice-to-use-cursors-in-sql-server : https://en.wikipedia.org/wiki/For_loop#Traditional_for-loops : https://docs.microsoft.com/en-us/sql/t-sql/language-elements/declare-cursor-transact-sql?view=sql-server-ver15#arguments : https://docs.microsoft.com/en-us/sql/t-sql/language-elements/equals-transact-sql?view=sql-server-ver15#remarks
To avoid problems like this I'm using different pattern 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 you need to check fetch status once too.