Today I came across a self-created problem that could have been avoided if I had only followed my own advice. It wasn't directly and explicitly mentioned in this context, but the concept is the same. Let's start with the source of the problem: one stored procedure that called another. The first ("inner") procedure was something like this: take a database_id as a parameter, and show all the table names with indexes that have more updates than seeks/scans/lookups:

CREATE PROCEDURE dbo.bar
   @database_id  INT,
   @object_id    INT = NULL
AS
BEGIN
   SET NOCOUNT ON;
 
   PRINT '';
   PRINT DB_NAME(@database_id);
   PRINT '';
 
   DECLARE c CURSOR
       FORWARD_ONLY STATIC READ_ONLY 
       FOR
       (
           SELECT DISTINCT [object_id]
               FROM sys.dm_db_index_usage_stats
               WHERE [database_id] = @database_id
               AND [object_id] = COALESCE(@object_id, [object_id])
               AND user_updates > user_seeks + user_scans + user_lookups
       );
 
   OPEN c;
 
   FETCH NEXT FROM c INTO @object_id;
 
   WHILE @@FETCH_STATUS = 0
   BEGIN
       PRINT OBJECT_SCHEMA_NAME(@object_id, @database_id) 
           + '.' + OBJECT_NAME(@object_id, @database_id);
 
       FETCH NEXT FROM c INTO @object_id;
   END
 
   DEALLOCATE c;
END
GO

Great, but what if I wanted to do this for multiple (or all databases), without modifying dbo.bar? Simple, right? We'll just create a new ("outer") stored procedure, dbo.foo, which acts as a wrapper for dbo.bar. In reality this could probably take a comma-separated list of database names or IDs, but for brevity let's just keep it simple and assume we want a report for all databases. This just means another cursor, like so:

ALTER PROCEDURE dbo.foo
AS
BEGIN
   SET NOCOUNT ON; 
   DECLARE @database_id INT; 
   DECLARE c CURSOR
       FORWARD_ONLY STATIC READ_ONLY 
       FOR
       (
           SELECT [database_id] 
               FROM sys.databases
       );
 
   OPEN c;
 
   FETCH NEXT FROM c INTO @database_id;
 
   WHILE @@FETCH_STATUS = 0
   BEGIN
       EXEC dbo.bar @database_id = @database_id;
 
       FETCH NEXT FROM c INTO @database_id;
   END 
 
   DEALLOCATE c;
END
GO

Do you spot the problem yet?

In the inner procedure, I've declared a cursor with a pretty short and meaningless name, c. Which I tend to do a lot (I did say "bad habit" right?), since I've never come across this issue before – I don't tend to architect solutions that require nested cursors, in fact I avoid cursors as much as I can. The problem is that the outer procedure is coded using the same construct and, both predictably and unfortunately, the same cursor name, c. So when the inner procedure starts, it fails to create the cursor because it already exists and is open. And once the inner procedure is finished, it deallocates the cursor, but then the outer procedure also tries to reference it, leading to these errors:

There are two solutions here; I fully intend to implement both of them, both to solve this problem, and going forward in general:

1. Add LOCAL to the declaration of the cursors (or just use a local variable, like @c), so that they are not global (which is the default). In fact, I usually do this but forgot here.
2. Use a more meaningful and unique name for each cursor, such as "objects" on the inner, and "databases" on the outer.

Through this exercise I also came across another interesting problem with the use of nested cursors, but I will talk about that in a more general upcoming blog post about some of the worst error messages that come out of SQL Server.