When you don't follow your own "bad habits" advice…
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:
A cursor with the name 'c' already exists.
Msg 16905, Level 16, State 1, Procedure bar, Line 19
The cursor is already open.
Msg 16916, Level 16, State 1, Procedure foo, Line 21
A cursor with the name 'c' does not exist.
Msg 16916, Level 16, State 1, Procedure foo, Line 26
A cursor with the name 'c' does not exist.
There are two solutions here; I fully intend to implement both of them, both to solve this problem, and going forward in general:
- 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.
- 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.
You are absolutely right I really don't know why Microsoft did not change the default settings to LOCAL. Great blog by the way!
Marcin, yes that is true, but I challenge you to find many databases out in the real world where the default (GLOBAL) has been changed.
Isn't the CURSOR_DEFAULT database option solution for this? I believe a need for global cursors is quite rare so I assume it can be safely use on most of the real world databases? Especially as one can still declare cursors with global scope by adding word GLOBAL to the definition.
Try a cursor variable instead. As the name implies, they have variable-style scoping:
DECLARE @cursor CURSOR
SET @cursor = CURSOR STATIC FOR
SELECT database_id FROM sys.databases
— will raise an error, since it is already out of scope.