would like to rollback the entire thing.
This approach works but is it a good design'
Please let me know.
declare @.err int
declare @.id1 int
declare @.id2 int
set @.err = -1
begin tran
insert into tbl1(c1,c2) values (1,2)
if @.@.error = 0
set @.err = 0
else
set @.err = -1
SELECT @.id1 = SCOPE_IDENTITY()
if @.err = 0
begin
insert into tbl2(c1,c2) values (1,2)
if @.@.error = 0
set @.err = 0
else
set @.err = -1
SELECT @.id2 = SCOPE_IDENTITY()
end
if @.err = 0
begin
insert into tbl3(c1,c2) values (1,2)
if @.@.error = 0
set @.err = 0
else
set @.err = -1
end
if @.err = 0
begin
commit tran
end
else
begin
rollback tran
RAISERROR('Please check bla bla bla',16,1)
endWhy continue attempting to do work if you know you are going to roll back?
BEGIN TRAN;
INSERT INTO foo(a) SELECT 1;
IF @.@.ERROR <> 0
BEGIN
RAISERROR('foo', 11, 1);
ROLLBACK;
RETURN;
END;
INSERT INTO bar(a) SELECT 2;
IF @.@.ERROR <> 0
BEGIN
RAISERROR('bar', 11, 1);
ROLLBACK;
RETURN;
END;
COMMIT TRAN;
Some people use labels and GOTO, but I'm not overly fond of that structure,
personally... reminds me too much of QBasic.
For a much more thorough treatment of the topic, Erland has a couple of
great articles:
http://www.sommarskog.se/error-handling-I.html
http://www.sommarskog.se/error-handling-II.html
"sqlster" <nospam@.nospam.com> wrote in message
news:A62E348D-7F82-4FD6-8F91-B54C8371351E@.microsoft.com...
>I am trying to insert into multiple tables and if any of the inserts fail,
>I
> would like to rollback the entire thing.
> This approach works but is it a good design'
> Please let me know.
> declare @.err int
> declare @.id1 int
> declare @.id2 int
> set @.err = -1
>
> begin tran
> insert into tbl1(c1,c2) values (1,2)
> if @.@.error = 0
> set @.err = 0
> else
> set @.err = -1
> SELECT @.id1 = SCOPE_IDENTITY()
> if @.err = 0
> begin
> insert into tbl2(c1,c2) values (1,2)
> if @.@.error = 0
> set @.err = 0
> else
> set @.err = -1
> SELECT @.id2 = SCOPE_IDENTITY()
> end
>
> if @.err = 0
> begin
> insert into tbl3(c1,c2) values (1,2)
> if @.@.error = 0
> set @.err = 0
> else
> set @.err = -1
>
> end
> if @.err = 0
> begin
> commit tran
> end
> else
> begin
> rollback tran
> RAISERROR('Please check bla bla bla',16,1)
> end|||Aaron,
Thank you VERY MUCH..
"Aaron Bertrand [SQL Server MVP]" wrote:
> Why continue attempting to do work if you know you are going to roll back?
> BEGIN TRAN;
> INSERT INTO foo(a) SELECT 1;
> IF @.@.ERROR <> 0
> BEGIN
> RAISERROR('foo', 11, 1);
> ROLLBACK;
> RETURN;
> END;
> INSERT INTO bar(a) SELECT 2;
> IF @.@.ERROR <> 0
> BEGIN
> RAISERROR('bar', 11, 1);
> ROLLBACK;
> RETURN;
> END;
> COMMIT TRAN;
> Some people use labels and GOTO, but I'm not overly fond of that structure
,
> personally... reminds me too much of QBasic.
> For a much more thorough treatment of the topic, Erland has a couple of
> great articles:
> http://www.sommarskog.se/error-handling-I.html
> http://www.sommarskog.se/error-handling-II.html
>
>
>
> "sqlster" <nospam@.nospam.com> wrote in message
> news:A62E348D-7F82-4FD6-8F91-B54C8371351E@.microsoft.com...
>
>|||I agree that GOTO isn't the most elegant solution, but SQL 2000 doesn't have
TRY/CATCH logic, so that's what we're stuck with. I dislike the solution
you've suggested because the error handling code is distributed throughout
the procedure. For two or three inserts, that may not seem to be a problem,
but for a complicated process like posting a batch of inventory
transactions, distributing the error handling introduces a lot of redundant
code. Another reason to consolidate the error handling is that it's much
simpler to unit test the procedure--that is, to step through the error
handling logic--if it exists in a single place. Finally, consistency
simplifies development and maintenance and reduces bugs, so it's better to
use the same mechanism for all procedures--even those with only two or three
inserts.
By the way, there are a couple problems with your code:
(1) change "ROLLBACK" to "IF @.@.TRANCOUNT > 0 ROLLBACK" Of course, this
doesn't take into account transaction savepoints.
(2) change "RETURN" to "RETURN -1"
It should be clear even with this simple example that the changes I
mentioned must be applied throughout the procedure, illustrating the
maintenance problems that can and will occur.
The bottom line is that it cost less to code, test, and maintain procedures
with consolidated error handling code. I use GOTO because it yields better
code for less time and money. I haven't done any production code in SQL
2005 yet, but the TRY/CATCH logic sounds like a promising alternative.
"Aaron Bertrand [SQL Server MVP]" <ten.xoc@.dnartreb.noraa> wrote in message
news:usuTkajEGHA.2380@.TK2MSFTNGP12.phx.gbl...
> Why continue attempting to do work if you know you are going to roll back?
> BEGIN TRAN;
> INSERT INTO foo(a) SELECT 1;
> IF @.@.ERROR <> 0
> BEGIN
> RAISERROR('foo', 11, 1);
> ROLLBACK;
> RETURN;
> END;
> INSERT INTO bar(a) SELECT 2;
> IF @.@.ERROR <> 0
> BEGIN
> RAISERROR('bar', 11, 1);
> ROLLBACK;
> RETURN;
> END;
> COMMIT TRAN;
> Some people use labels and GOTO, but I'm not overly fond of that
> structure, personally... reminds me too much of QBasic.
> For a much more thorough treatment of the topic, Erland has a couple of
> great articles:
> http://www.sommarskog.se/error-handling-I.html
> http://www.sommarskog.se/error-handling-II.html
>
>
>
> "sqlster" <nospam@.nospam.com> wrote in message
> news:A62E348D-7F82-4FD6-8F91-B54C8371351E@.microsoft.com...
>|||>I agree that GOTO isn't the most elegant solution, but SQL 2000 doesn't
>have TRY/CATCH logic, so that's what we're stuck with. I dislike the
>solution you've suggested because the error handling code is distributed
>throughout the procedure. For two or three inserts, that may not seem to
>be a problem, but for a complicated process like posting a batch of
>inventory transactions, distributing the error handling introduces a lot of
>redundant code.
In my defense, my suggestion was based on the code provided, not for every
situation you could come up with, and was only meant as pseudo-code. My
main thrust was to go read the articles, because Erland has shed a lot more
light on this topic than you or I could in a single thread.
A|||I didn't think your suggestion needed defending. Your code provided a
convenient example to illustrate the benefits of using GOTO. If it's any
consolation, most of the examples in Erland's articles don't include "IF
@.@.TRANCOUNT > 0" either, and the error handling code is also distributed.
"Aaron Bertrand [SQL Server MVP]" <ten.xoc@.dnartreb.noraa> wrote in message
news:%23$srncmEGHA.1312@.TK2MSFTNGP09.phx.gbl...
> In my defense, my suggestion was based on the code provided, not for every
> situation you could come up with, and was only meant as pseudo-code. My
> main thrust was to go read the articles, because Erland has shed a lot
> more light on this topic than you or I could in a single thread.
> A
>|||>I didn't think your suggestion needed defending.
Well, you said you disliked it, and attacked it. So... to avoid that
perception in the future, my advice to you is to reply to the original
poster and just provide the helpful part of the advice, instead of adding
the beating to the respondant(s) and peppering that with your own take.
> Your code provided a convenient example to illustrate the benefits of
> using GOTO.
I don't think my code used GOTO at all...
> Erland's articles don't include "IF @.@.TRANCOUNT > 0" either
Well, to be honest, the only time I've ever seen the following type of
example require a @.@.TRANCOUNT check:
BEGIN TRAN
INSERT ...
IF @.@.ERROR <> 0
..
Is if the act of beginning a transaction itself had failed. In which case,
won't the batch abort anyway (at least under default conditions)? How do
you force a BEGIN TRAN to fail anyway? Aside from trying to commit a
transaction involving a linked server, where DTC is not running or
misconfigured, I can't think of one off the top of my head (and I don't
think that would fail on the BEGIN TRAN anyway, but it's too late for me to
bother checking right now anyway). The only time I have ever seen the above
kind of example error out due to a bad @.@.TRANCOUNT is if there are logic
errors in the code (e.g. redundant BEGIN TRAN statements).
I've seen this @.@.TRANCOUNT check used so heavily, and in such simplistic
examples, in the code of a very pedantic co-worker which,
ly, I no longerhave to maintain -- because we replaced his slow and bloated codebase with a
completely revamped version (the @.@.TRANCOUNT checks weren't the slow and
bloated part I'm talking about). This anal retentive guy spent so much time
triple-bulletproofing every single line of code that, come delivery time,
much of the functionality was either broken, missing, or so horribly scraped
together that it was useless. He left us with little alternative except a
complete re-write. But he sure checked @.@.TRANCOUNT right after every BEGIN
TRAN, ROLLBACK, and COMMIT, even though the statements surrounding them were
rubbish! :-(
In any event, TRY/CATCH is a very welcome addition to SQL Server 2005,
though it still leads to what you complain about, distributed error
handling. Personally, I think it makes more sense to place error handling
right with the part of the code that errors out, instead of trying to shovel
it off to some common location at the bottom of the code. For one, when I
call a raiserror, the resulting line number lets me go right to the line of
code, instead of havint to retrace my steps. But that's just a preference
and there is no point in attacking it.
A|||Look, if you really want to turn this into a slug fest, then I'm game.
> I don't think my code used GOTO at all...
I didn't say that it did. The distributed nature of the error handling in
your code provided the perfect example of what NOT to do. Any error
handling change requires modifying every block of error handling code
throughout the procedure. Each of those modifications must then be tested.
MY point is that without TRY/CATCH, GOTO is the next best thing because it
serves to consolidate error handling into a single block of code.
I didn't say that you needed to check @.@.TRANCOUNT AFTER any statement. You
SHOULD, however, check it BEFORE a ROLLBACK to avoid raising the error:
Server: Msg 3903, Level 16, State 1, Line 1
The ROLLBACK TRANSACTION request has no corresponding BEGIN TRANSACTION.
I most certainly didn't make the asinine suggestion that @.@.TRANCOUNT should
be checked after a BEGIN TRAN. Where did that come from?
"Aaron Bertrand [SQL Server MVP]" <ten.xoc@.dnartreb.noraa> wrote in message
news:OvXE%23ZpEGHA.2912@.tk2msftngp13.phx.gbl...
> Well, you said you disliked it, and attacked it. So... to avoid that
> perception in the future, my advice to you is to reply to the original
> poster and just provide the helpful part of the advice, instead of adding
> the beating to the respondant(s) and peppering that with your own take.
>
> I don't think my code used GOTO at all...
>
> Well, to be honest, the only time I've ever seen the following type of
> example require a @.@.TRANCOUNT check:
> BEGIN TRAN
> INSERT ...
> IF @.@.ERROR <> 0
> ...
> Is if the act of beginning a transaction itself had failed. In which
> case, won't the batch abort anyway (at least under default conditions)?
> How do you force a BEGIN TRAN to fail anyway? Aside from trying to commit
> a transaction involving a linked server, where DTC is not running or
> misconfigured, I can't think of one off the top of my head (and I don't
> think that would fail on the BEGIN TRAN anyway, but it's too late for me
> to bother checking right now anyway). The only time I have ever seen the
> above kind of example error out due to a bad @.@.TRANCOUNT is if there are
> logic errors in the code (e.g. redundant BEGIN TRAN statements).
> I've seen this @.@.TRANCOUNT check used so heavily, and in such simplistic
> examples, in the code of a very pedantic co-worker which,
ly, I no> longer have to maintain -- because we replaced his slow and bloated
> codebase with a completely revamped version (the @.@.TRANCOUNT checks
> weren't the slow and bloated part I'm talking about). This anal retentive
> guy spent so much time triple-bulletproofing every single line of code
> that, come delivery time, much of the functionality was either broken,
> missing, or so horribly scraped together that it was useless. He left us
> with little alternative except a complete re-write. But he sure checked
> @.@.TRANCOUNT right after every BEGIN TRAN, ROLLBACK, and COMMIT, even
> though the statements surrounding them were rubbish! :-(
> In any event, TRY/CATCH is a very welcome addition to SQL Server 2005,
> though it still leads to what you complain about, distributed error
> handling. Personally, I think it makes more sense to place error handling
> right with the part of the code that errors out, instead of trying to
> shovel it off to some common location at the bottom of the code. For one,
> when I call a raiserror, the resulting line number lets me go right to the
> line of code, instead of havint to retrace my steps. But that's just a
> preference and there is no point in attacking it.
> A
>|||> I didn't say that it did. The distributed nature of the error handling in
> your code provided the perfect example of what NOT to do.
I think you're talking about a preference here, not an everyone-must-do-it
best practice.
> I didn't say that you needed to check @.@.TRANCOUNT AFTER any statement.
> You SHOULD, however, check it BEFORE a ROLLBACK to avoid raising the
> error:
> Server: Msg 3903, Level 16, State 1, Line 1
> The ROLLBACK TRANSACTION request has no corresponding BEGIN TRANSACTION.
In the example I provided, please tell me how you can reach that path.
Because remember, you said you didn't like *my* example because I didn't use
@.@.TRANCOUNT. The logic in the piece of code I posted was so simple that the
above error message would be impossible to reach.|||You may be free to do as you please. I have to minimize development and
maintenance cost. Since development projects generally have budgets
attached, there are few who are free to do as they please. I think that
we'll both agree that it is a universal BAD practice to release code that
has not been thoroughly tested. This means that every path in the code must
be touched during testing. Your solution--distributing error handling
code--requires more test cases, more programming time, and more testing
time. In fact, the amount of development time for distributed error
handling code increases linearly along with the number of instances of
duplicate code, whereas the amount of development time when GOTO is used
remains flat. In addition, a change to the error handling code (for
example, if the need arises to rollback to a savepoint) requires that every
instance of duplicate code must be modified and tested--and again cost
increases linearly. The bottom line is that it costs less to use GOTO.
Because it costs less, I would consider that an everyone-must-do-it best
practice.
I said that I dislike your SOLUTION: distributing error handling code. I
acknowledge the point that "IF @.@.TRANCOUNT > 0" may not be needed in your
simplistic example. Since I consolidate error handling code, I always check
@.@.TRANCOUNT before issuing a ROLLBACK. It may add a few unnecessary CPU
cycles, but if I later add a call to another stored procedure, I don't have
to worry about forgetting to change the block of error handling code. All I
need to do is to make sure that control is redirected there whenever an
error occurs. That's one of the benefits of code reuse.
"Aaron Bertrand [SQL Server MVP]" <ten.xoc@.dnartreb.noraa> wrote in message
news:ueOfQx$EGHA.472@.TK2MSFTNGP12.phx.gbl...
> I think you're talking about a preference here, not an everyone-must-do-it
> best practice.
>
> In the example I provided, please tell me how you can reach that path.
> Because remember, you said you didn't like *my* example because I didn't
> use @.@.TRANCOUNT. The logic in the piece of code I posted was so simple
> that the above error message would be impossible to reach.
>
Chris Howarth wrote: