0

I'm pretty new in T-SQL and I'm in trouble with some huge scripts with transactions, cursors and storage procedures. So, my code is something like this (this code is just an example of the structure of my scripts, in fact I have multiples procedures inside OuterProc cursor and multiple operations inside InnerProc cursor):

create proc InnerProc
as
begin
  declare @Id int

  begin tran

  declare mycursor cursor local static read_only forward_only
  for select Id
      from MyOtherTable

  open mycursor
  fetch next from mycursor into @Id

  while @@fetch_status = 0
  begin
    select 1/0

    if @@ERROR <> 0 
    begin
      rollback tran
      return @@ERROR
    end          

    fetch next from mycursor into @Id
  end

  close mycursor   
  deallocate mycursor

  commit tran
end


create proc OuterProc
as
begin

  declare @Id int

  begin tran

  declare mycursor cursor local static read_only forward_only
  for select Id
      from MyTable

  open mycursor
  fetch next from mycursor into @Id

  while @@fetch_status = 0
  begin
    exec @error = InnerProc

    if @@ERROR <> 0 
    begin
      rollback tran
      return
    end
    else
      commit tran

    fetch next from mycursor into @Id
  end

  close mycursor   
  deallocate mycursor
end

With this structure I have this error:

Msg 515, Level 16, State 2, Procedure InnerProc, Line 448
  Cannot insert the value NULL into column 'InitialQuantity', table 'MySecondTable'; column does not allow nulls. INSERT fails.
  The statement has been terminated.
Msg 266, Level 16, State 2, Procedure InnerProc, Line 0
  Transaction count after EXECUTE indicates a mismatching number of BEGIN and COMMIT statements. Previous count = 1, current count = 0.
Msg 3903, Level 16, State 1, Procedure CreateSASEExtraction, Line 79
  The ROLLBACK TRANSACTION request has no corresponding BEGIN TRANSACTION.

What is wrong with my code? If something goes wrong inside innerProc, I want all operations for that outer cursor rollback and stop the inner cursor. If something goes wrong in the outerProc I want all operations for that cursor to rollback but I want that cursor continue to looping...
There is a better way to do this?

UPDATE:

After I correct some errors @Bernd Linde detected, I add a try-catch in InnerProc and I named the InnerProc transaction. Now I have this code:

create proc InnerProc
as
begin
  declare @Id int

  begin tran

  begin try

    declare mycursor cursor local static read_only forward_only
    for select Id
        from MyOtherTable

    open mycursor
    fetch next from mycursor into @Id

    while @@fetch_status = 0
    begin
      select 1/0

      if @@ERROR <> 0 
        return @@ERROR     

      fetch next from mycursor into @Id
    end

    close mycursor   
    deallocate mycursor

    commit tran
    return 0

  end try
  begin catch

    return @@ERROR

  end catch

end


create proc OuterProc
as
begin

  declare @Id int

  declare mycursor cursor local static read_only forward_only
  for select Id
      from MyTable

  open mycursor
  fetch next from mycursor into @Id

  while @@fetch_status = 0
  begin

    begin tran

    exec @error = InnerProc

    if @@ERROR <> 0
    begin
      rollback tran
      return
    end
    else
      commit tran

    fetch next from mycursor into @Id
  end

  close mycursor   
  deallocate mycursor
end

But now I have other error message:

Msg 266, Level 16, State 2, Procedure InnerProc, Line 0
Transaction count after EXECUTE indicates a mismatching number of BEGIN and COMMIT statements. Previous count = 1, current count = 2.

How can I solve this?

Ninita
  • 1,209
  • 2
  • 19
  • 45
  • 1
    why are you using a cursor at all? – HLGEM Oct 02 '14 at 13:46
  • @HLGEM because I need to do some operations for each row of a certain table – Ninita Oct 02 '14 at 14:05
  • 1
    That does not require a cursor. Cursors are a performance killer and should be a last resort. No one except a DBA with at least 10 years of experience should even consider using them at all. – HLGEM Oct 02 '14 at 14:46
  • 1
    @HLGEM so what is the better way to do the same? – Ninita Oct 02 '14 at 15:09
  • http://wiki.lessthandot.com/index.php/Cursors_and_How_to_Avoid_Them – HLGEM Oct 02 '14 at 17:37
  • Avoiding cursors requires mental change in how you think about data. You have to ask yourself what you want to do to a column instead of a row. – Sean Lange Oct 02 '14 at 19:05
  • 1
    Another important concept with transactions is that nested transactions are a myth. They simply do not work the way you might think. A nested begin transaction does nothing but increment @@trancount. And a rollback will rollback all "nested" transaction and @@trancount will be 0. – Sean Lange Oct 02 '14 at 19:08
  • http://www.sqlskills.com/blogs/paul/a-sql-server-dba-myth-a-day-2630-nested-transactions-are-real/ – Sean Lange Oct 02 '14 at 19:09
  • @SeanLange what you said about @@trancount is true and I already get that. In my case I call multiple procedures for each row of a certain table. Each of that procedures return a value, then I insert that values and some values I get from cursor in another table. I'm doing something like a datawarehouse.. Is really hude my code. – Ninita Oct 03 '14 at 09:17
  • @HLGEM please ready the previous comment I did to SeanLange – Ninita Oct 03 '14 at 09:20
  • @Ninita if you are doing this for datawarehousing then you will be adding many hours to process using this method. You would be killing yourself by reusing code that was written for one record to affect thousands, possibly millions of records. Set based processing faster by a lot. I removed a cursor on a trigger once that made an import of 40K records go from oveer 45 minutes to around 40 secends. For a datawarehouse it would be an especially bad thing to use cursors just to reuse procs. – HLGEM Oct 03 '14 at 13:41
  • @HLGEM so with what I must substitute the cursor? I can't use `select` for this pack of operations and using a `while` looping isn't the better way to looping to the right data. I remind you that I need for each row of a certain table, executes multiple procedures, get the returned data and insert that data and some data from the actual row in another table, and control the transactions – Ninita Oct 03 '14 at 14:47
  • You have to replace the procs with set based solutions. YOu can't reuse the procs - period. You need to get all the rows in one operation not use a loop of any kind. In fact you should forget you know how to loop as it is something that should be used extremely rarely. – HLGEM Oct 03 '14 at 14:56
  • @HLGEM what you are saying is I should have a single script with 1 million of lines of code to insert multiples times in multiples tables that reference a single table row that can be inserted before or after that all others rows? I can't see how can I do that – Ninita Oct 03 '14 at 15:20

2 Answers2

1

From the first look, you are committing transactions inside your loops, but you are only starting them once outside the loop.
So each time the loop goes into it's second iteration, it will try to either commit or rollback a transaction that does not exist, hence why you are getting the error "The ROLLBACK TRANSACTION request has no corresponding BEGIN TRANSACTION."

I would suggest reading up on transactions in SQLServer on MSDN here

Bernd Linde
  • 2,098
  • 2
  • 16
  • 22
  • That makes sense. In the code I posted InnerProc is wrong and I have the commit outside the cursor (I already corrected that), but in the OuterProc I really have that bug. But I have the same errors. I must have a try-catch or this structure must work? – Ninita Oct 02 '14 at 12:59
  • You have the same thing happening in the OutProc with the transaction boundaries. If you want the OuterProc to never stop doing it's looping, then yes you will need a try catch. But I would rather say look at why you where getting the error `Cannot insert the value...`. – Bernd Linde Oct 02 '14 at 13:09
  • Please see my updated question. I make some changes and now I have other errors. InnerProc transaction isn't found when a error occurs. I think InnerProc cursor continue looping even though rollback or return be executed – Ninita Oct 02 '14 at 13:26
  • You cannot used name transaction boundaries inside nested transactions, from the [MSDN](http://msdn.microsoft.com/en-US/en-en/library/ms188929.aspx) documentation: `Use transaction names only on the outermost pair of nested BEGIN...COMMIT or BEGIN...ROLLBACK statements.` – Bernd Linde Oct 02 '14 at 13:48
  • @Berned Linde I read that and this [nested transactions document](http://technet.microsoft.com/en-us/library/ms189336(v=sql.105).aspx). That is correct but I rectified that and isn't that the problem. The problem persists – Ninita Oct 02 '14 at 14:17
0

After many attemps, finally I get it.

The InnerProc must only have COMMITs and the OuterProc will be responsible for rollback. For that, when InnerProc causes some error that must be catch in the OuterProc and forced to act like a exception. How I want to continue looping in the OuterProc, that procedure must have a try-catch where the looping is forced and the rollback is done.

For a better transaction number control I used the @@TRANCOUNT.

So I solve the problem with this code:

create proc InnerProc
as
begin
  declare @Id int

  begin try

  begin tran

    declare mycursor cursor local static read_only forward_only
    for select Id
        from MyOtherTable

    open mycursor
    fetch next from mycursor into @Id

    while @@fetch_status = 0
    begin
      select 1/0

      IF @@ERROR <> 0   
      begin

        if @@TRANCOUNT > 0
          rollback tran

        close mycursor   
        deallocate mycursor
        return @@ERROR

      end

      fetch next from mycursor into @Id

    end

    close mycursor   
    deallocate mycursor

    commit tran
    return 0

    end try
    begin catch

        close mycursor   
        deallocate mycursor

        return @@ERROR
    end catch

end


create proc OuterProc
as
begin

  declare @Id int

  declare mycursor cursor local static read_only forward_only
  for select Id
      from MyTable

  open mycursor
  fetch next from mycursor into @Id

  while @@fetch_status = 0
  begin

    begin tran

    begin try

      exec @error = InnerProc

      if @@ERROR <> 0
          RAISERROR('Exception',1,1)


      if@@TRANCOUNT > 0
          commit tran

      fetch next from mycursor into @Id, @Name, @CodeDGAE, @Code, @NUIT, @Project   

    end try
    begin catch
        if @@TRANCOUNT > 0
            rollback tran
        fetch next from mycursor into @Id, @Name, @CodeDGAE, @Code, @NUIT, @Project 
    end catch
  end

  close mycursor   
  deallocate mycursor
end
Ninita
  • 1,209
  • 2
  • 19
  • 45