Take the 2-minute tour ×
Database Administrators Stack Exchange is a question and answer site for database professionals who wish to improve their database skills and learn from others in the community. It's 100% free, no registration required.

Question about use of Cursors in combination with RETURN in a SQL Server 2008 Stored Procedure

Consider this example:

CREATE PROCEDURE [dbo].[test]
    @ReturnEarly BIT = 0
AS
BEGIN
    SET NOCOUNT ON

    SELECT 1 AS Result INTO #Test

    DECLARE @Result INT, @HasResult INT = 1

    DECLARE TestCursor CURSOR FOR 
        SELECT Result
        FROM #Test
        WHERE Result = 0

    OPEN TestCursor
    FETCH NEXT FROM TestCursor INTO @Result

    IF (@@FETCH_STATUS <> 0)
    BEGIN
        IF (@ReturnEarly = 1)
            RETURN 0
        ELSE 
            SET @HasResult = 0
    END

    CLOSE TestCursor
    DEALLOCATE TestCursor

    DROP TABLE #test --technically superflous

    IF (@HasResult = 0)
        RETURN 0
    ELSE
        RETURN 1 -- in reality many more checks on the actual results
END
GO

-- Allow DEALLOCATE CURSOR to be called
DECLARE   @RC int, @ReturnEarly bit
SET @ReturnEarly = 1
EXECUTE @RC = [test] @ReturnEarly
PRINT @RC
GO

-- Exit before DEALLOCATE CURSOR is called
DECLARE   @RC int, @ReturnEarly bit
SET @ReturnEarly = 0
EXECUTE @RC = [test] @ReturnEarly
PRINT @RC
GO

DROP PROCEDURE [dbo].[test]

Both EXEC calls of the Stored Procedure return the same result. My question is this: is there a potential leak associated with returning from inside the Cursor? There is not for the temporary table (#test), for example.

Bonus: Can you point me to docs suggesting either way? My google skills weren't up to it.

Extra Bonus: If there is a problem, is there any way I could find evidence of the leak? perhaps a dmv, event log, etc which would show the problem?

Thanks in advance!

share|improve this question
    
Are you asking for advice with new work, or are you asking for help detecting a suspected problem? If you are talking about new work, I would not do the early RETURN as you could simply use BREAK instead. Mind you, that is assuming you really mean to have a WHILE loop in that code... –  Max Vernon Aug 30 '12 at 18:51
1  
If it's new work, then I would recommend not using a Cursor at all. There's almost never a need to. –  RBarryYoung Aug 30 '12 at 19:23
    
The actual situation is that I'm making an update to an application and reviewing sprocs that may need updating. This sproc (created on SQL 2000 in ~2003) exists because the non-cursor version wasn't reliably consistent in its behavior. The use case is to "lock" a record for asynchronous update (by updating a relevant status column), if it has not already been so locked. For example, I click in an application to indicate I want to update an record - you shouldn't be able to do the same, even thought it may take me several minutes to complete the update. –  dividius Aug 30 '12 at 19:46
    
@MaxVernon, the use case is to return an appropriate status code - so the sproc needs to indicate a difference between "no such record" and "found the record but it is invalid because...". The specific use case had no while loop (see comment above on actual use case) –  dividius Aug 30 '12 at 19:52
1  
I know this doesn't exactly address your question, but why not simply comment out the RETURN statement, and set the appropriate return code there by way of a variable? I up-voted your question because I'm interested to know if there are any system dmv's etc that display this type of diagnostic information. –  Max Vernon Aug 30 '12 at 20:04

3 Answers 3

up vote 2 down vote accepted

Copying part of my answer from here...

How long the resources hang around depends on whether you declared the cursor locally or globally (and what the default is in your environment - default is global but you can change it).

If the cursor is global, then it can stay "alive" in SQL Server until the last piece of code is touched in the scope in which it was created - and can often persist even beyond that. For example if you call a stored procedure that creates a global cursor, then call 20 other stored procedures, the cursor will live on while those other 20 stored procedures are running, until the caller goes out of scope. I believe it will stay alive at the session level, not the connection level, but haven't tested this thoroughly. If the cursor is declared as local, then it should only stay in scope for the current object (but again, this is theoretical, and I haven't done extensive, low-level memory tests to confirm). I don't know of any DMVs that show this usage individually, or even if on a global level you could detect memory changes in DMVs that probably don't get updated often enough to reflect the truth at any microsecond.

The general concept, though, should be: when you're done with something, say so. I believe there are several ways to write your code such that the RETURN doesn't have to happen within the cursor, and probably ways to re-write the code to eliminate the cursor.

In order to make my cursors as efficient as possible, I always use the following declarations:

DECLARE c CURSOR
  LOCAL STATIC FORWARD_ONLY READ_ONLY
  FOR SELECT ...

I've also heard that there can be memory issues if you only CLOSE or only DEALLOCATE so - even though I haven't proven or disproven it - I always do both when I'm done:

CLOSE c;
DEALLOCATE c;
share|improve this answer

Try sys.dm_exec_cursors:

-- Create local fast_forward ( forward-only, read-only ) cursor 
DECLARE test_cursor1 CURSOR FAST_FORWARD LOCAL FOR 
SELECT object_id
FROM sys.objects

select 'after declare' s, * from sys.dm_exec_cursors(null)

-- Cursor variables
DECLARE @object_id  INT

OPEN test_cursor1

select 'after open' s, * from sys.dm_exec_cursors(null)

FETCH NEXT FROM test_cursor1 INTO @object_id
WHILE @@fetch_status = 0
BEGIN

    -- cursor code here 
    -- ...

    FETCH NEXT FROM test_cursor1 INTO @object_id
END

CLOSE test_cursor1

select 'after close' s, * from sys.dm_exec_cursors(null)

DEALLOCATE test_cursor1

-- This won't show anything ...
select 'after deallocate' s, * from sys.dm_exec_cursors(null)
share|improve this answer
    
That's awesome wBob, thanks much! –  dividius Sep 19 '12 at 5:17

Well well. One obvious proof of a problem would seem to be that the example cannot be executed twice against the same database in short succession.

Attempting to do so yields:

Msg 16915, Level 16, State 1, Procedure test, Line 14
A cursor with the name 'TestCursor' already exists.
Msg 16905, Level 16, State 1, Procedure test, Line 18
The cursor is already open.

Still, interested in what insight the community can provide!

edit: removing the STATIC declaration on the cursor eliminates this problem. Not sure why the cursor was declared static > 5 years ago. Edited the example to remove this declaration.

share|improve this answer
    
STATIC makes the cursor copy a working set into tempdb. If the system is not tempdb-bound this can often be a more efficient approach. In cases like this you should also look into the keywords LOCAL, READ_ONLY and FORWARD_ONLY. I've observed as much as a 400% performance improvement on the same cursor code with the only difference being the addition of these four keywords. –  Aaron Bertrand Sep 18 '12 at 3:51

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.