1

I have a stored procedure preparing a dataset using an employee listing as base query and performing a CROSS APPLY to a MSTVF to build a small resultset per employee. In the MSTVF, a cursor is used to build the interim result set (performing complex math using running totals and case logic). MSTVFs don't allow TRY...CATCH blocks so I can't trap an error directly and use CURSOR_STATUS() to close and deallocate the cursor, or can I? What can I do in the declaration of the cursor or in the CATCH block of the stored procedure to check for an open cursor in the MSTVF?

Additional Context after Earlier Comments (Functional Logic)

The MSTVF code in question builds a set of instructions to submit to a timekeeping app's API engine to open a timecard, adjust one or more paycode balances with offsetting transactions and close a timecard. Basic workflow below...

  1. MSFTVF takes an employee ID and pay period date ranges as params and returns a 10-column table variable

  2. A variable is set with a sum of specific paycode balances. This sum is to be washed via debits and credits via the API. This is the pending balance.

  3. A cursor of qualifying transactions to wash is declared in priority order.

  4. While there is a pending balance... 3a) A debit matching credit transaction is inserted into a table variable (for the API call records) 3b) The amount of the debit/credit is the smaller of the current transaction or pending balance (handled via IF statement) 3c) If the transaction is more than one pay period in the past, another API call record is inserted that would flag the transaction for a historical correction.

  5. When no balance remains, the while loop exits out 4a) If there are no xactions in the cursor left and still there is a pending balance, a final set of API calls are inserted to wash this balance.

  6. The API table variable is then queried to insert API call records for final transactions at the timecard level and housekeeping calls (unlock, retotal, lock). If no records are present by the end of 4), nothing is generated.

  7. The API table variable is aggegrated on paycode and transaction type and inserted into the MSTVF table variable and returned back to the calling procedure.

The use of a MSTVF was to encapsulate the logic generating all the API calls. To scale to an ILTVF, I figure after calculating the pending balances in 1) in the calling stored procedure, the ILTVF would run a query with a windowing function for SUM() for the running totals. On first blush, maybe the windowing query could be used in a CTE to reproduce steps 3) and 4) with a multi-section UNION statement. I figure the ILTVF result would be stored in a temp table in the calling procedure and steps 5) and 6) run to build in the housekeeping and store the aggregated result for the next ETL step (API execution).

4
  • 2
    In SQL Server 2016 what complex math and case expression are you using that can't be solved without a cursor? I would suggest addressing that and removing MSTVF from the solution entirely is a better approach (though you can start by using a cursor variable instead of a cursor, so you don't have to worry about closing and deallocating, details here). What you're doing is saying, "Gee, I have a lot of fires at my house, I should move next door to the fire station." – Aaron Bertrand Aug 24 '20 at 16:41
  • Can you clarify if you have actually experienced errors in this scenario, or if you're just trying to be complete in your error handling? – Aaron Bertrand Aug 24 '20 at 20:53
  • Completing SQL error handling to compensate for bad API error handling. Short version: Java API calls the sproc and waits synchronously to complete; sproc CATCH block caught a 1205 deadlock error, issued a ROLLBACK and sent error info via DB mail at end of CATCH, but API still running; app control not returned and couldn't be cancelled via the API GUI so needed to issue a KILL (noted cursor OPEN was last active statement). My guess is that a THROW was needed at the end of the CATCH block or a SET XACT_ABORT ON at the start of the sproc to ensure the API would close out session. More below. – MattyZDBA Aug 24 '20 at 23:51
  • Added functional logic for the current MSTVF for context in the main question section – MattyZDBA Aug 25 '20 at 0:07
2
  1. Use a cursor variable (details here), then you don't have to worry about closing and deallocating (which, as David points out, isn't necessary inside a TVF, but it - and/or LOCAL - is required in other contexts). Just change:

     DECLARE c CURSOR ... FOR SELECT ...
     ...
     OPEN c;
     FETCH NEXT FROM c INTO ...
    
     ...
    
     CLOSE c;
     DEALLOCATE c;
    

    To:

     DECLARE @c CURSOR;
     SET @c = CURSOR READ_ONLY FORWARD_ONLY FOR SELECT ...
     ...
     OPEN @c;
     FETCH NEXT FROM @c INTO ...
    
     ...
    
  2. Work on the logic that you think requires a cursor and conditional/flow logic, so that you can eliminate the MSTVF and re-write it as an inline TVF. While it wasn't true before SQL Server 2012, I can assure you that most cursor logic in SQL Server 2016 can be rewritten using window functions. There are still isolated cases where a cursor is a better choice (not always for performance reasons), but it's doubtful that a TVF is one of them.

1

Only local cursors are allowed in a function, and so there's no need to close or deallocate them explicitly in a catch block.

The following statements are valid in a function:

...

DECLARE statements defining local data variables and local cursors.

...

Cursor operations referencing local cursors that are declared, opened, closed, and deallocated in the function.

https://docs.microsoft.com/en-us/sql/t-sql/statements/create-function-transact-sql?view=sql-server-ver15#interoperability

And consistent with many cursor options, in a function you get a local cursor regardless of whether you ask for one.

Local cursors are not only have batch-level lifetime, they have batch-level visibility. They are not visible in nested batches, eg this

 declare c cursor local for select * from sys.objects 
 open c 
 exec ('close c')

fails with

Msg 16916, Level 16, State 1, Line 28
A cursor with the name 'c' does not exist.

Nor are they visible in parallel batches using MultipleActiveResultSets.

Each time you create a local cursor it gets a unique cursor_id. EG

 declare c cursor local for select * from sys.objects 
 open c 
 
 exec ('declare c cursor local for select * from sys.objects;  open c;
       select cursor_id, name from sys.dm_exec_cursors(@@spid)')

outputs

cursor_id   name
----------- -----
180150091   c
180150093   c

Additionally a batch or procedure like this:

create or alter procedure ct
as
begin
    declare c cursor local for select * from sys.objects 
    open c 
    select cursor_id,name from sys.dm_exec_cursors(@@spid) 
end

Can be invoked multiple times serially,

create or alter procedure ct
as
begin
    declare c cursor local for select * from sys.objects 
    open c 
    select cursor_id,name from sys.dm_exec_cursors(@@spid) 
end
go

exec ct 
go 2

outputs

Beginning execution loop
cursor_id   name
----------- ------------------------------
180150115   c

(1 row affected)

cursor_id   name
----------- ------------------------------
180150117   c

(1 row affected)

or with interleaved concrrent execution from a client program like this:

using Microsoft.Data.SqlClient;
using System;
using System.Collections.Generic;
using System.Linq;

namespace SqlClientTest
{

    class Program
    {

        static void Main(string[] args)
        {

            var constr = @"server=localhost;database=tempdb;integrated security=true;multipleactiveresultsets= true";

            using (var con = new SqlConnection(constr))
            {
                con.Open();

                var rs = new List<SqlDataReader>();
                for (int i = 0; i < 10; i++)
                {
                    var cmd = new SqlCommand("exec ct",con);
                    rs.Add(cmd.ExecuteReader());
                }

                int rr = 0;
                foreach (var r in rs)
                {
                    while (r.Read())
                    {
                        rr++;
                        Console.WriteLine($"{r[0]} {r[1]}");
                    }
                }

                Console.WriteLine(rr);
            }
        }
    }

}

which outputs

180150003 c
180150005 c
180150007 c
180150009 c
180150011 c
180150013 c
180150015 c
180150017 c
180150019 c
180150021 c
4
  • Yeah, I'm not quite sure how the OP is generating an error directly from a cursor inside a function, or if they're just trying to code defensively. Still, in general, a cursor variable is definitely a good way to prevent trampling over yourself if you forget to close/deallocate, and doesn't force you to always remember to add LOCAL, in contexts where it does matter. Eliminating the cursor altogether is an even better goal, of course. – Aaron Bertrand Aug 24 '20 at 20:16
  • 'just specifying LOCAL doesn't help you even in a stored procedure if you run it multiple times from the same session' I don't think so. See update to answer. – David Browne - Microsoft Aug 24 '20 at 20:38
  • 1
    You're right, flawed testing. Need to learn to stop re-using the same session windows. (The cursor name that caused the problem even with LOCAL was from a previous version of the procedure without it, but executed in the same session.) – Aaron Bertrand Aug 24 '20 at 20:39
  • The error was a 1205 for a deadlocked resource which was trapped and reported back by the calling stored procedure. The problem came up with the API engine in the app tier as it didn't address the sproc aborting and I had to contact the DBA team to kill the SPID. For the short term, I'll try out the cursor variable and will test-throw a divide by zero to see if the API traps cleanly. Thoughts on how to avoid a RBAR scenario in my code coming in the next comment. – MattyZDBA Aug 24 '20 at 21:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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