Questions about some error handing code?
-
Hi all,
I have this code (from BOL basically) and I think it was recommended to use in an on-going SP development scenario I am working on.
It appears to work, but my questions/concerns are... I have placed the below code at the top of my SP, right after my variable declarations basically (I do have some variable "validation" code above it as well).
1) Is this the "right" place for this code, or should it be somewhere else?
--~~~~~~~~~~~~START ERROR HANDLING BEFORE INSERT~~~~~~~~~~~~--
SELECT @errorcode = @@ERROR
DECLARE @InnerTransactionCount1OnEntry INT
IF @errorcode = 0
BEGIN
SELECT @InnerTransactionCount1OnEntry = @@TRANCOUNT
BEGIN TRANSACTION
2) --? is the whole transaction within this begin & end or what?
END
I'll repeat #2 above in case it was missed in the code:
2) --? is the whole transaction within this begin & end, or does it just exist as a BEGIN & END around the TRANCOUNT and begin transaction (2) lines?
*** Insert of new patient goes here, & select ID just inserted, etc ***
As well.... at the end.. just before returning to the calling SP I have this code.
--FINAL ERROR CHECKING
IF @@TRANCOUNT > @InnerTransactionCount1OnEntry
BEGIN TRANSACTION
IF @errorcode = 0
COMMIT TRANSACTION
ELSE
PRINT 'An error has occurred and/or a transaction needs to be rolled back'
ROLLBACK TRANSACTION
Does this seem right, or am I missing anything... I am getting some weird results at times... like when I execute the SP in QA while testing, it will auto-increment the ID, display back the ID that I selected, but the record WILL NOT have inserted for some reason. So when I go do a SELECT * FROM tablename, the ID that just came back doesn't have a record in the table??? I think it has to do with how the transactions are being handled, but I'm not sure?
Thanks.
Michelle/Dolphin.
"Work like you don't need the money;
dance like no one is watching;
sing like no one is listening;
love like you've never been hurt;
and live every day as if it were your last."
~ an old Irish proverb -
Dolphin Look at MY post on the previous thread.
http://www.sqlservercentral.com/forums/shwmessage.aspx?forumid=8&messageid=199015
1.
For each BEGIN TRAN the variable @@trancount is incremented by 1
For each commit is decremented by one
If rollback is encountered then is set to 0
2. If you are getting extraneous numbers chances are that you already have a trigger on the target table which is inserting in a different one and you are using @@indentity instead of scope_identity()
hth
* Noel -
Dolphin something I just remember having read that may help you with your logic. A transaction belongs to a workspace object which holds all transactions in memeory Inner transactions are not commited untill all transactions have been completed successfuly or a rollback statement is found. If a rollback is found then none of the inner transactions are committed.
BOL
When used in nested transactions, commits of the inner transactions do not free resources or make their modifications permanent. The data modifications are made permanent and resources freed only when the outer transaction is committed. Each COMMIT TRANSACTION issued when @@TRANCOUNT is greater than 1 simply decrements @@TRANCOUNT by 1. When @@TRANCOUNT is finally decremented to 0, the entire outer transaction is committed. --end BOL
Would it be possible to write all of your transactions in one sp. If so a wrapper transaction will solve your problem.
Mike
-
1) BEGIN TRANS should be just before the 1st data modification statement
2) The BEGIN ... END is to allow multiple line conditionals so your:
IF @errorcode = 0
BEGIN
SELECT @InnerTransactionCount1OnEntry = @@TRANCOUNT
BEGIN TRANSACTION
END
Correctly groups the SELECT and BEGIN TRANS statements for the THEN portion of the IF
The BEGIN TRANS is active until a COMMIT TRANS or ROLLBACK TRANS, it is not dependant on statement grouping BEGIN ... END
Your Final error checking has problems, I would replace it with:
--FINAL ERROR CHECKING
SET @errorcode = @@ERROR
IF @@TRANCOUNT > @InnerTransactionCount1OnEntry
BEGIN
IF @errorcode = 0
COMMIT TRANSACTION
ELSE
BEGIN
PRINT 'An error has occurred and/or a transaction needs to be rolled back'
ROLLBACK TRANSACTION
-- This will roll back all open transactions, no matter the nesting level
END
END
The way you wrote it the outer BEGIN TRANS would never be committed, it would bave been rolled back as the ROLLBACK TRANS was not groubed with the IF.
Andy
-
One huge problem is at the end of the error checks:
ELSE
PRINT 'An error has occurred and/or a transaction needs to be rolled back'
ROLLBACK TRANSACTION
The ELSE only applies to the PRINT, the ROLLBACK is always executed. You need a BEGIN/END around PRINT and ROLLBACK TRAN. Aren't you getting errors from this, or is this a nested transaction? (This correction was also in David's code).
What if @errorcode > 0 at the beginning? It looks like it will bypass the BEGIN TRAN and execute the insert code anyway, then skip the COMMIT at the end.
Maybe the END after BEGIN TRAN should be moved down to after the last END in FINAL ERROR CHECKING? Or just remove the initial If @errorcode = 0 BEGIN & END (keep the SELECT & BEGIN TRAN).
I think the error checking at the end was attempting to test the original @errorcode value. David's addition of SELECT @errorcode = @@Error would handle errors occurring within the transaction but not pre-existing errors, because the BEGIN TRAN was skipped and the final COMMIT/ROLLBACK would be skipped. What is the value of @InnerTransactionCount1OnEntry before this code executes? If it isn't set anywhere then it is NULL, and IF @@TRANCOUNT > @InnerTransactionCount1OnEntry will always be false.
I think I would add a more @@Error checks:
IF @errorcode > 0
SET @InnerTransactionCount1OnEntry = 99
ELSE BEGIN
SET @InnerTransactionCount1OnEntry = @@TRANCOUNT
BEGIN TRAN
-- Insert patient
INSERT ...
SELECT @errorcode = @@Error
IF @errorcode > 0 BEGIN
-- Insert successful, commit it
SELECT @ID = @@IDENTITY
COMMIT TRAN
END
END
-- Final Error Checks
IF @InnerTransactionCount1OnEntry > @@TRANCOUNT BEGIN
PRINT ...
ROLLBACK TRAN
END
If the patient insert is a multi-step process, I would nest them all inside additional "SELECT @errorcode = @@Error IF @errorcode > 0 BEGIN ... END" blocks. The COMMIT only happens if you make it through all the error checks, otherwise the final test will cause a ROLLBACK.
-
BE Careful with this code:
...
-- Insert patient
INSERT ...
SELECT @errorcode = @@Error
IF @errorcode > 0 BEGIN
-- Insert successful, commit it
SELECT @ID = @@IDENTITY
...
It is flawed. You must select Identity and error code in the SAME line of code
Like
-- Insert patient
INSERT ...
SELECT @errorcode = @@Error, @ID = @@IDENTITY
IF @errorcode > 0 BEGIN
-- Insert successful, commit it
COMMIT TRAN
...
The reason is that in between the statements another insert could have happened in a table with an Identity colunm and you may get the wrong value
Cheers!
* Noel -
I know that's true of @@ERROR but I haven't seen it with @@IDENTITY. You're probably correct, but the only way I know of to screw up @@IDENTITY is to have a trigger that inserts into another table with an identity field. If something else (like a trigger) is generating identity values on the same connection between the INSERT and the SELECT, it doesn't matter how soon you check it. Maybe it should be changed to IDENT_CURRENT('table').
From BOL:
IDENT_CURRENT is similar to the Microsoft® SQL Server™ 2000 identity functions SCOPE_IDENTITY and @@IDENTITY. All three functions return last-generated identity values. However, the scope and session on which 'last' is defined in each of these functions differ.
- IDENT_CURRENT returns the last identity value generated for a specific table in any session and any scope.
- @@IDENTITY returns the last identity value generated for any table in the current session, across all scopes.
- SCOPE_IDENTITY returns the last identity value generated for any table in the current session and the current scope.
I've never seen a good explanation of what else may be executing in the another "scope" that is outside of the stored proc. There may be a case (other than triggers) where it would not work the way I wrote it, but I haven't run into it.
-
On heavy transaction conditions what one worker is doing is can be preempted for what another is executing and under those conditions the scope switches therefore if another proc is inserting in a table that has an identity column @@Identity will be updated to that one and when the scope switches back to yours you are receiving the wrong value.
That is the meaning of:
- @@IDENTITY returns the last identity value generated for any table in the current session, across all scopes.
No triggers are needed for that to happen. Safest way will be to use SCOPE_IDENTITY though
* Noel -
Noel,
Thanks for your response. I've been in meetings so haven't looked at the other post yet, but I will.
I am aware of how transactions and the variable @@trancount work, I am just getting weird results with the code itself.
Just for yours & everyones' info: there are NO triggers on the table(s) and I am using scope_identity().
I have both in there, but the @@indentity is the one currently commented out... as I was testing between the 2 at one point... some of the documentation on the differences can be a bit vague, but this article http://techrepublic.com.com/5100-9592_11-5751509.html?tag=nl.e101 although short seems to do a bit better job than most of the others that ramble all over the place and don't think to explain things in clearer terms
It looks like several of the other posts may have hit on some ideas, so I will move on down the list... as I think that when it works when the SP is executed by itself and then doesn't when the wrapper is executed, but the ID increments anyway something weird is happening with the rollback, etc as suggested in other posts.
Thanks.
"Work like you don't need the money;
dance like no one is watching;
sing like no one is listening;
love like you've never been hurt;
and live every day as if it were your last."
~ an old Irish proverb -
Mike,
Great "gem" from the BOL... sometimes there are things in BOLin clear language!!! That's what I like about the newer .NET docs.. they actually created help that helps.. hopefully they will do the same with SQL server 2005
I did not realize that with a nested transaction that the inner ones would not commit fully until the outer one does
. Interesting... so something in my inner transaction(s) IS messing with the @@TRANCOUNT, so everything else will fail no matter what. I think some of the other posts are looking to shed some light on why things might be failing, but I do have one question for you based on your post:I'm not sure what you mean by "Would it be possible to write all of your transactions in one sp"?
Here is modifed view of how things are currently laid out and I've cut some "non-essential" code out for brevity. Let me know if that helps clarify what I am doing and hopefully you can clarify with "code" or psuedo-code what you mean by your last question.
Note.. indenting didn't come through time for some reason.
I.e.
/*~~~~~~~~~~~~~~~~~~~~INSERT INTO InsertFormFullDetails ("wrapper SP)~~~~~~~~~~~~~~~*/
CREATE PROCEDURE dbo.InsertFormFullDetails
--DECLARE LOCAL VARIABLES (NEED ALL TO BE HERE SO CAN BE PASSED TO various SPs CALLED!!!)
--List...
--~~~~~~~~~~~~END DECLARE LOCAL VARIABLES BEFORE INSERT~~~~~~~~~~~~--
/*~~~~~~~~~~~~~~~~~~~~INSERT INTO Patient Table~~~~~~~~~~~~~~~~~*/
/*~~~~~~~~~~~~~~~~~~~~Separate SP~~~~~~~~~~~~~~~~~*/
DECLARE @NewPatientID INT
EXEC @NewPatientID=dbo.InsertNewPatient @NewPatientID OUTPUT
--along with list of other variables req'd
SELECT @NewPatientID AS ReturnValue
--PatientID is req'd for next insert
/*~~~~~~~~~~~~~~~~INSERT INTO Patient Visit Records Table~~~~~~~~~~~~~~~~*/
/*~~~~~~~~~~~~~~~~~~~~Separate SP~~~~~~~~~~~~~~~~~*/
DECLARE @NewPatientVisitRecordID INT
EXEC @NewPatientVisitRecordID=dbo.InsertNewPatientVisitRecord @NewPatientVisitRecordID OUTPUT
--along with list of other variables req'd (PatientID)
SELECT @NewPatientVisitRecordID AS ReturnValue
--PatientVisitRecordID is req'd for next insert
/*~~~~~~~~~~~~~~~~INSERT INTO InsertActualFormTable~~~~~~~~~~~~~~~~*/
/*~~~~~~~~~~~~~~~~~~~~Separate SP~~~~~~~~~~~~~~~~~*/
--Currently commented out as focusing on getting first 2 working properly first, as they will be RE-USED for all other form inserts and each out form details SP & actual form SP will be the only ones that are unique.
--EXEC InsertActualForm
--along with list of other variables req'd (PatientVisitRecordID)
--OUTPUT will likely be cpInvestigationID INT or other
~~~~~~~~~~~~~END~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Make sense?
Thanks.
Michelle/Dolphin
"Work like you don't need the money;
dance like no one is watching;
sing like no one is listening;
love like you've never been hurt;
and live every day as if it were your last."
~ an old Irish proverb -
When an identity field is incremented by INSERT, it stays incremented even if the INSERT is rolled back. From BOL under @@IDENTITY: "The @@IDENTITY value does not revert to a previous setting if the INSERT or SELECT INTO statement or bulk copy fails, or if the transaction is rolled back."
Back to scopes & sessions for a minute:
I agree that SCOPE_IDENTITY is more precise and probably the correct choice. But in the absence of any triggers, functions, or subprocedures, how would SCOPE_IDENTITY differ from @@IDENTITY?
Certainly threads from other SPIDs on the same server will be getting their turn, but if your stored proc is not calling any other procs or functions, there are no triggers involved, and there is nothing but a few statements dealing with local variables between the (single-row) INSERT and the SELECT @@IDENTITY, how did some other INSERT (running on the same session) sneak in there? I know SQL Server tries to take advantage of parallelism but I wouldn't expect some kind of out-of-order execution of something outside the stored proc.
-
My question is why are you using three sp's. I would nest the transactions and check for errors after each transaction. I would also set XACT_Abort to ON. From BOL When SET XACT_ABORT is ON, if a Transact-SQL statement raises a run-time error, the entire transaction is terminated and rolled back. [end BOL]
BeginTrans A
BeginTrans B
BeginTrans C
--Code for trans c goes here
--Check for error if no error then commit
--If error then rollback
--End of trans c is here
--Code for trans b goes here
--Check for error if no error then commit
--If error then rollback
--End of trans B is here
--Code for trans A goes here
--Check for error if no error then commit
--If error then rollback
--End of trans A is here
Nothing is actually committed untill Transcation A completes.
Mike
-
Mike,
Thanks for your response!
I thought I had mentioned why I am using 3 SPs (actually 4 if you include the wrapper SP) in an earlier post, or even in the original post. InnerSP1(your B?) (InsertPatient) & InnerSP2(your C ?)(InsertVisitRecord) were separated out for re-use purposes (or porpoises
). They will be used in each and every "patient form data" insert.I.e. Patient is inserted, VisitRecord is inserted, and then relevant form data (for that situation) is inserted, but the form (& data) can be different every time. So 3 separate tables, and InsertPatient inserts into the Patient table and has to return the PatientID, which is the FK in VisitRecord; InsertVisitRecord inserts into the VisitRecord table and has to return the VisitRecordID as this is then the FK in the various form table(s), which ever one was “filled out”. I hope that makes sense?!
However, if I don't get this working how I need it too, I may have to resort back to putting it all in one and then your code structure would come in handy (and another thread would need to be started
. But my fear with doing it that way is that it would be so huge with so much code and hard to manage. Plus instead of creating modular code and re-using the InsertPatient & InsertVisitRecord code in their own SPs (along with the form insert in it's own separate SP) I would have the same code repeated in EVERY form insert SP and while there are only ~40 forms right now.. there will be more added in future versions, so this code needs to carry forward with that in mind as well.So, I would rather be able to figure out a way to handle it the way I have outlined, as long as it is possible?!
As well, based on what I understand from your original post, the transactions would function the same way regardless of whether they are nested in one SP, or in a wrapper SP that calls an EXEC SPname command and goes off to another SP for a while and then comes back? I have some thinking to do on this subject, so I will likely come back to you with questions on this "sub-topic".
I will just keep working on things, I have to respond still to several of the other posts, but I can only spend so much time on here in a day
. Fortunately, my boss actually knows that I am researching and posting in forums to try and find a solution to this problem and somewhat supports and understands the reason & need 
!BTW, I am using XACT_Abort to ON (and SET NoCount ON).. thanks
Thanks again to everyone who has posted so far... I am working on a couple of the ideas suggested and will try and respond as soon as I can. I will also see if I can clean up a larger & more detailed segment of my code for the wrapper SP and the actual inner SPs as there might be something really simple (or dumb) that I am missing.
Cheers.
Michelle/Dolphin
"Work like you don't need the money;
dance like no one is watching;
sing like no one is listening;
love like you've never been hurt;
and live every day as if it were your last."
~ an old Irish proverb -
Scott, Think PREEMTIVE MULTITASKING!
when your procedure is running others can be too!!! scope_identity is one vaiable per session, @@identity is one GLOBAL thing, if another procedure inserts a record on another table that has an identity column, between the time your insert happened and your separated read statement is actually executed the @@identity function that you will get is the last value which could be not what you are expecting
cheers!
* Noel -
Mike if you follow the all code in one shot don't need to start 3 separated transactions just wrap them all in ONE! -- it will make your life a lot simpler
and of course after each DML check for errors and act accordingly
cheers!
* Noel
Viewing 15 posts - 1 through 15 (of 16 total)
You must be logged in to reply to this topic. Login to reply