4

I have the following script which works fine:

DECLARE db_cursor1 CURSOR LOCAL FOR 
SELECT  ID, Name table_1

OPEN db_cursor1 
FETCH NEXT FROM db_cursor1 INTO  @ID, @Name                      

WHILE @@FETCH_STATUS = 0  
BEGIN  
   BEGIN TRANSACTION
   BEGIN TRY

     <insert into table values>  

   COMMIT TRANSACTION 
   END TRY

   BEGIN CATCH
         PRINT ERROR_MESSAGE();
   ROLLBACK TRANSACTION 

   END CATCH

   FETCH NEXT FROM db_cursor1 INTO  @ID, @Name 

END 

CLOSE db_cursor1
DEALLOCATE db_cursor1 

The above script works fine in that it rolls back what is in the current iteration of db_cursor1 and then proceeds to the next iteration in case there is an error.

The problem arises when I have a nested cursor. It rolls back what is in the current iteration but DOES NOT proceed to the next iteration of cursor1.

  DECLARE db_cursor1 CURSOR LOCAL FOR 
  SELECT  ID, Name table_1

  OPEN db_cursor1 
  FETCH NEXT FROM db_cursor1 INTO  @ID, @Name                      

  WHILE @@FETCH_STATUS = 0  
  BEGIN  
     BEGIN TRANSACTION
     BEGIN TRY

     <insert into table values>  

      --- inner cursor

      DECLARE db_cursor2 CURSOR LOCAL FOR 
      SELECT  ID, Name table_2

      OPEN db_cursor2
      FETCH NEXT FROM db_cursor2 INTO  @ID, @Name                      

      WHILE @@FETCH_STATUS = 0  
      BEGIN             

          <insert into table values>  

          FETCH NEXT FROM db_cursor2 INTO  @ID, @Name 

      END 

      CLOSE db_cursor2
      DEALLOCATE db_cursor2

  COMMIT TRANSACTION 
  END TRY

  BEGIN CATCH
     PRINT ERROR_MESSAGE();
  ROLLBACK TRANSACTION 

  END CATCH

  FETCH NEXT FROM db_cursor1 INTO  @ID, @Name 

 END 

CLOSE db_cursor1
DEALLOCATE db_cursor1 
General Grievance
  • 4,555
  • 31
  • 31
  • 45
Nate Pet
  • 44,246
  • 124
  • 269
  • 414
  • 1
    Are you getting an error? A hung transaction? Is it actually failing to iterate? Or is it hitting a condition that's stopping it? – Xedni Sep 13 '18 at 22:37
  • @Xedni Yes, for the code where it is a cursor inside a cursor, it fails to iterate if there is an error in insert. Otherwise it works fine. – Nate Pet Sep 14 '18 at 18:32
  • What's the error you're getting? – Xedni Sep 14 '18 at 18:33
  • @Xedni The code that I have for the nested cursor works. If I simulate and error by say inserting a string into an int field while insert, it shows the appropriate error BUT fails to iterate to the next outer cursor loop. – Nate Pet Sep 17 '18 at 14:29
  • I'm not able to get the behavior you're describing from the test harness I built. Can you include some sample data and tables that will reproduce it? There are a couple other problems I see as well including reusing the `@ID` and `@Name` variables for both cursors. Also, in the event the inner cursor fails, it never closes/deallocates the cursor, and the next loop will run into a naming conflict as it tries to (re)create `db_cursor2`. – Xedni Sep 17 '18 at 16:24
  • 1
    @Xedni Yes, what I have is somewhat complex. Let me look at simplying this week and then sending you the script so you have a better idea. Thanks in advance. – Nate Pet Sep 17 '18 at 17:37
  • Cursors are to be avoided, nested cursors even more. You should consider another approach altogether, if possible. – Zohar Peled Sep 20 '18 at 05:52
  • This may help: https://stackoverflow.com/questions/26160641/rollback-transaction-inside-cursors-and-inside-transactions – Alex Sep 21 '18 at 08:04

3 Answers3

0

I was able to create it on my end with basic values, please see SCRIPT #1 - ERROR if you'd like to test it. The issue that you're running into is that when you have an error in db_cursor2, you exit the loop without closing or deallocating the cursor. Then when the code goes to next iteration, it fails with this error A cursor with the name 'db_cursor2' already exists. Please see SCRIPT #2 - SUCCESS for correct results. To give it more color, you needed to add CLOSE db_cursor2; DEALLOCATE db_cursoe2; in your BEGIN CATCH.

SETUP, Designed for SQL Server 2016+

DROP TABLE IF EXISTS #table_1, #table_2

CREATE TABLE #table_1
(
    [ID] INT,
    [Name] VARCHAR(5)
);
CREATE TABLE #table_2
(
    [ID] INT,
    [NAME] VARCHAR(5)
);

INSERT INTO #table_1 SELECT 1, 'j';
INSERT INTO #table_1 SELECT 2, 'j';

INSERT INTO #table_2 SELECT 1, 'j';
INSERT INTO #table_2 SELECT 2, 'j';

SCRIPT #1 - ERROR

DECLARE @ID INT;
DECLARE @name VARCHAR(5);

DECLARE db_cursor1 CURSOR LOCAL FOR SELECT [ID], [Name] FROM #table_1;

OPEN db_cursor1;
FETCH NEXT FROM db_cursor1
INTO @ID, @name;

WHILE @@FETCH_STATUS = 0
BEGIN
    BEGIN TRANSACTION;
    BEGIN TRY

        PRINT('trying 1')
        --- inner cursor

        DECLARE db_cursor2 CURSOR LOCAL FOR SELECT [ID], [Name] FROM #table_2;

        OPEN db_cursor2;
        FETCH NEXT FROM db_cursor2
        INTO @ID, @name;

        WHILE @@FETCH_STATUS = 0
        BEGIN
            PRINT('trying 2')
            SELECT 1/0

            FETCH NEXT FROM db_cursor2
            INTO @ID, @name;
        END;

        CLOSE db_cursor2;
        DEALLOCATE db_cursor2;

        COMMIT TRANSACTION;

    END TRY
    BEGIN CATCH
        PRINT ERROR_MESSAGE();

        ROLLBACK TRANSACTION;
    END CATCH;

    FETCH NEXT FROM db_cursor1
    INTO @ID, @name;

END;

CLOSE db_cursor1;
DEALLOCATE db_cursor1;

SCRIPT #2 - SUCCESS

DECLARE @ID INT;
DECLARE @name VARCHAR(5);

DECLARE db_cursor1 CURSOR LOCAL FOR SELECT [ID], [Name] FROM #table_1;

OPEN db_cursor1;
FETCH NEXT FROM db_cursor1
INTO @ID, @name;

WHILE @@FETCH_STATUS = 0
BEGIN
    BEGIN TRANSACTION;
    BEGIN TRY

        PRINT('trying 1')
        --- inner cursor

        DECLARE db_cursor2 CURSOR LOCAL FOR SELECT [ID], [Name] FROM #table_2;

        OPEN db_cursor2;
        FETCH NEXT FROM db_cursor2
        INTO @ID, @name;

        WHILE @@FETCH_STATUS = 0
        BEGIN
            PRINT('trying 2')
            SELECT 1/0

            FETCH NEXT FROM db_cursor2
            INTO @ID, @name;
        END;

        CLOSE db_cursor2;
        DEALLOCATE db_cursor2;

        COMMIT TRANSACTION;

    END TRY
    BEGIN CATCH
        PRINT ERROR_MESSAGE();

        -- was missing in above script
        CLOSE db_cursor2
        DEALLOCATE db_cursor2

        ROLLBACK TRANSACTION;
    END CATCH;

    FETCH NEXT FROM db_cursor1
    INTO @ID, @name;

END;

CLOSE db_cursor1;
DEALLOCATE db_cursor1;
Tigerjz32
  • 4,324
  • 4
  • 26
  • 34
  • SCRIPT #2 seem ok.IMO there should be 2 TRY CATCH.what if outer cursor throw error. – KumarHarsh Sep 24 '18 at 08:10
  • Sure, we can always have multiple layers of Try Catch but if the outer cursor throws an error I assume the user actually wants to know that something failed. The reason we want the inner Try Catch is so the transaction continues even on an error, related to the sub cursor. – Tigerjz32 Sep 27 '18 at 14:14
-1

Because you are using a TRY-CATCH, if there is an error within the TRY, code execution will begin in the CATCH.

Inside your catch you would have to handle the error, and depending on the error, close and deallocate db_cursor2. Or if the error is benign, perhaps return execution back to the TRY with a GOTO. GOTO statements cannot be used to enter a TRY or CATCH block. GOTO statements can be used to jump to a label inside the same TRY or CATCH block or to leave a TRY or CATCH block.

Paul Wehland
  • 114
  • 6
-1

As @paul-wehland stated, it's because your Try-Catch isn't disposing the nested cursor. As such, your next iteration is going to initialize a cursor by name that already exists. I've provided an example of code that will run your basic scenario with an intended failure condition at iteration 11 of each cursor.

In the example, I've commented out the part of the code that addresses the issue. Where you choose to place that block is totally up to you, but it would make sense to check either before the nested cursor declaration, or inside the Catch block.

declare
    @id tinyint,
    @parent_id tinyint,
    @name varchar(255),
    @parent_name varchar(255);

declare
    @table
table
    (
    id tinyint not null primary key,
    [name] varchar(255) not null
    );

declare
    @target
table
    (
    parent_id tinyint not null,
    child_id tinyint not null,
    parent_name varchar(10) not null,
    child_name varchar(10) not null,
    primary key(parent_id, child_id)
    );

with cteNumber
as  (
    select top 11
        [id] = row_number() over (order by [object_id])
    from
        sys.objects
    )
insert into
    @table
select
    id,
    [name] = replicate('a', id)
from
    cteNumber;

declare
    db_cursor1 
cursor
    local keyset read_only forward_only 
for
    select
        0,
        id,
        'Initial', 
        [name]
    from
        @table;

open
    db_cursor1;
fetch
    next
from
    db_cursor1 
into
    @id,
    @parent_id, 
    @name,
    @parent_name;

while @@FETCH_STATUS = 0  
    begin
        begin transaction;

        begin try
            insert into @target
                (parent_id, child_id, parent_name, [child_name])
            values
                (@parent_id, @id, @parent_name, @name);

            --- inner cursor
            /*
            if CURSOR_STATUS('local', 'db_cursor2') = 1
                begin
                    close
                        db_cursor2;
                    deallocate
                        db_cursor2;
                end;
            -- */

            declare
                db_cursor2 
            cursor
                local keyset read_only forward_only 
            for
                select
                    id, 
                    [name]
                from
                    @table;

            open
                db_cursor2;

            fetch
                next
            from
                db_cursor2 
            into
                @id, 
                @name;

            while @@FETCH_STATUS = 0  
                begin
                    insert into @target
                        (parent_id, child_id, parent_name, [child_name])
                    values
                        (@parent_id, @id, @parent_name, @name);

                    fetch
                        next
                    from
                        db_cursor2 
                    into
                        @id,
                        @name;
                end;

            close
                db_cursor2;

            deallocate
                db_cursor2;

            commit transaction
        end try

        begin catch         
            print ERROR_MESSAGE();

            rollback transaction;
        end catch;

        fetch
            next
        from
            db_cursor1 
        into
            @id,
            @parent_id, 
            @name,
            @parent_name;
    end;

close
    db_cursor1;
deallocate
    db_cursor1;

select
    [Last @id] = @id,
    [Last @name] = @name,
    [Last @parent_id] = @parent_id,
    [Last @parent_name] = @parent_name;

select
    *
from
    @table;

select
    *
from
    @target;

EDITED You could also use the creation of a Cursor Variable, and assign the nested cursor declaration to it, which would negate the problem of dealing with the duplicate names. See below:

declare
    @id tinyint,
    @parent_id tinyint,
    @name varchar(255),
    @parent_name varchar(255);

declare
    @table
table
    (
    id tinyint not null primary key,
    [name] varchar(255) not null
    );

declare
    @target
table
    (
    parent_id tinyint not null,
    child_id tinyint not null,
    parent_name varchar(10) not null,
    child_name varchar(10) not null,
    primary key(parent_id, child_id)
    );

with cteNumber
as  (
    select top 11
        [id] = row_number() over (order by [object_id])
    from
        sys.objects
    )
insert into
    @table
select
    id,
    [name] = replicate('a', id)
from
    cteNumber;

declare
    @db_cursor2 cursor;

declare
    db_cursor1 
cursor
    local keyset read_only forward_only 
for
    select
        0,
        id,
        'Initial', 
        [name]
    from
        @table;

open
    db_cursor1;
fetch
    next
from
    db_cursor1 
into
    @id,
    @parent_id, 
    @name,
    @parent_name;

while @@FETCH_STATUS = 0  
    begin
        begin transaction;

        begin try
            insert into @target
                (parent_id, child_id, parent_name, [child_name])
            values
                (@parent_id, @id, @parent_name, @name);

            --- inner cursor

            set @db_cursor2 = cursor
                local keyset read_only forward_only 
            for
                select
                    id, 
                    [name]
                from
                    @table;

            open
                @db_cursor2;

            fetch
                next
            from
                @db_cursor2 
            into
                @id, 
                @name;

            while @@FETCH_STATUS = 0  
                begin
                    insert into @target
                        (parent_id, child_id, parent_name, [child_name])
                    values
                        (@parent_id, @id, @parent_name, @name);

                    fetch
                        next
                    from
                        @db_cursor2 
                    into
                        @id,
                        @name;
                end;

            close
                @db_cursor2;

            deallocate
                @db_cursor2;

            commit transaction
        end try

        begin catch         
            print ERROR_MESSAGE();

            rollback transaction;
        end catch;

        fetch
            next
        from
            db_cursor1 
        into
            @id,
            @parent_id, 
            @name,
            @parent_name;
    end;

close
    db_cursor1;
deallocate
    db_cursor1;

select
    [Last @id] = @id,
    [Last @name] = @name,
    [Last @parent_id] = @parent_id,
    [Last @parent_name] = @parent_name;

select
    *
from
    @table;

select
    *
from
    @target;
Solonotix
  • 449
  • 3
  • 15