Nesting transactions/SPs and error questions (Part 1)

  • Hey there all,

    I have a ton of questions I've been meaning to post, but haven't had the time to "clean" the code up for posting purposes.  However, I will start with this question, as it will lead into the others (I think).

    What I need to do is insert a patient, a patient visit record and then the related form data for that "incident".

    As the patient and patient visit record inserts will be reused lots, I pulled them out and have them in their own SPs.  And for ease of coding, readability, etc, I pulled the form data insert out into it's own SP as well.

    So, there is an "outer" SP that will call (and manage) these 3 SPs mentioned above.

    I have questions ? and comments -- in the code below, as it was hard to just extract out where my questions were without the related code… and I have many... think of it as a Solstice SQL treasure hunt

    The outer SP has this structure:

    CREATE PROCEDURE InsertFormInsertFullDetails --names changed!

    --DECLARE LOCAL VARIABLES (none for this SP - as only calling other SPs)

    AS

    --Other Local Variables needed

     DECLARE @ReturnCode INT

     SELECT @ReturnCode = 1

     DECLARE @errorcode = INT   --for transaction tracking & error checking

     SELECT @errorcode = @@ERROR

     DECLARE @OuterTransactionCountOnEntry INT

     IF @errorcode = 0

     BEGIN

        SELECT @OuterTransactionCountOnEntry = @@TRANCOUNT

        BEGIN TRANSACTION

        --? is the whole transaction within this begin & end or is this code really just put at the beginning?

       --code is from (http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dnsqlpro2k/html/sql00f15.asp)

     END

    /*~~~~~~~~~~~~~~~~~~~~INSERT INTO Patient Table~~~~~~~~~~~~~~~~~*/

    /*~~~~~~~~~~~~~~~~~~~~Separate SP~~~~~~~~~~~~~~~~~*/

    --required from Patient insert (PatientID) for Patient Visit Records INSERT

    --INPUT (none additional required for Patient insert)

    --Does it make sense to handle this using an SP with an output variable (PatientID) or would a function or something else make more sense?

    --Other error handling?

      EXEC InsertNewPatient

    --Other error handling?

    --OUTPUT from Patient insert (PatientID) -> required for next SP

    /*~~~~~~~~~~~~~~~~INSERT INTO  Patient Visit Records Table~~~~~~~~~~~~~~~~*/

    /*~~~~~~~~~~~~~~~~~~~~Separate SP~~~~~~~~~~~~~~~~~*/

    --required from Patient Visit Records insert (PatientVisitRecordID) for Form Data INSERT

    --INPUT for Patient Visit Records insert (PatientID)

    --Does it make sense to handle this using an SP with an output variable (PatientVisitRecordID) or would a function or something else make more sense?

    --Other error handling?

      EXEC InsertNewPatientVisitRecord

    --Other error handling?

    --OUTPUT from PatientVisitRecord insert (PatientVisitRecordID) -> required for next SP

    /*~~~~~~~~~~~~~~~~INSERT INTO  Form Table~~~~~~~~~~~~~~~~*/

    /*~~~~~~~~~~~~~~~~~~~~Separate SP~~~~~~~~~~~~~~~~~*/

    --INPUT for Form insert (PatientVisitRecordID)

    --Other error handling?

      EXEC FormInsert

    --Other error handling?

    --OUTPUT FormID INT --? may not be required, as only "notification" of successful form insert is likely needed

    /*At this point the patient, patient visit record and form inserts if all were successful will need to be committed as one.  I don’t know if there is a better way to do this?*/

    IF @@TRANCOUNT > @OuterTransactionCountOnEntry

    BEGIN

       IF @errorcode = 0

          COMMIT TRANSACTION

       ELSE

     PRINT 'A transaction needs to be rolled back'

          ROLLBACK TRANSACTION

    END

    RETURN @ReturnCode

    GO

    I hope this makes some sense.  And please don't just post those links (http://www.sommarskog.se/error-handling-I.html) and (http://www.sommarskog.se/error-handling-II.html) as I've read them numerous times and they are not helping.  I think that my questions have moved beyond these barely basic articles.

    As well, I've noticed that there seem to be ALOT of questions regarding error handling in T-SQL, SPs, etc and the resources/help seem pretty lacking.  So, hopefully someone can help, or shed some insight or light on how to approach things better, or if there are more detailed resources out there those are welcome too!

    I also had a bunch of additional info and comments and when I went to post, I guess things had timed out and I lost it all... should have cut & pasted it out I guess (on a side note… why does this happen & can it be fixed!?).  So, if something is missing or needs more explanation, please just ask!

    Thanks.

    Dolphin/Michelle

    "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

  • Although you appear to have Erland Sommarskog's, you must read them in detail. Here are some rule of thumb:

    Transaction must be initiated by the client and not in database components such as stored procedures.

    Always have the first statement in stored procedure be "set nocount on" followed by "set xact_abort on"

    Error checking in stored procedure is a waste of time since many errors will cause the stored procedure to terminate, other errors will cause the batch to terminate and some errors will terminate the database connection.

    The value of the return code should not have a meaning. Always return +1.

    Here is an exampe stored procedure:

    CREATE PROCEDURE InsertFormInsertFullDetails

    ( @PatientNamevarchar(128)

    , @PatientIdinteger

    , @VistDatedatetime

    , @VisitLocationvarchar(128)

    )

    AS

    SET NoCount ON

    SET Xact_Abort ON

    IF @@Trancount = 0

    BEGIN

    RAISERROR ('Procedure must be executed within a transaction',16,1)

    RETURN +1

    END

    IF @@Trancount > 1

    BEGIN

    RAISERROR ('Procedure cannot be executed within a nested transaction',16,1)

    RETURN +1

    END

    DECLARE @SpStatus integer

    ,@ErrStatus integer

    EXECUTE @SpStatus = Patient_Add

    @PatientName= @PatientName

    , @PatientId= @PatientId

    SET@ErrStatus = COALESCE( NULLIF ( @SpStatus, 0), @@error)

    IF @ErrStatus 0 RETURN +1

    EXECUTE @SpStatus = PatientVisit_Add

    @PatientId= @PatientId

    , @VistDate= @VistDate

    , @VisitLocation = @VisitLocation

    SET@ErrStatus = COALESCE( NULLIF ( @SpStatus, 0), @@error)

    IF @ErrStatus 0 RETURN +1

    RETURN 0

    go

    SQL = Scarcely Qualifies as a Language

  • Carl,

    Thanks for your responses... I've read the Sommarskog's stuff so many times I can't recall the count right now... there is a point as well where you need to see some different code examples, etc as the ones there are so basic they barely begin to cover the many aspects of error handling.

    "Transaction must be initiated by the client and not in database components such as stored procedures. "
     
    Not sure what you mean here as the "official start" of the transaction is of course with the client, but I am asking about the BEGIN TRANS as part of the SP code.
     

    "Always have the first statement in stored procedure be "set nocount on" followed by "set xact_abort on""
     
    Can you explain why?  I think I have too many people say "just do it this way", but then the exception to the case rolls along and if you don't understand the reasoning and why something should be done a certain way... you never know when to do it differently (or why you're doing it that way in the first place).  And why is my favorite question
     

    "Error checking in stored procedure is a waste of time since many errors will cause the stored procedure to terminate, other errors will cause the batch to terminate and some errors will terminate the database connection."
     
    I agree that this is often the case, but I need to do some error handing.. or it may fail b/c that wasn't done!
     

    "The value of the return code should not have a meaning. Always return +1."
     
    Every thing I have been taught and every piece of sample code I have from school, jobs, etc has used return code, can you explain why you are saying don't use it?
     
    Thanks.. will address the code separately as I have to look through it a bit still.

    Btw... I realized in re-reading this note that it may seem "challenging" in it's tone, but I'm not just looking for the "answer", I'm trying to understand the concepts better.

     
    Thanks in advance!
    Dolphin/Michelle.

     
     

    "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

  • Michelle you do WHY very well. and if you are like my daughter once started getting to the end is a long process and as I do not have any extra time this week  I will not start. BTW I think my daughter is the world champ at WHY.

    Mike

  • here's my 2 cents.

    "Transaction must be initiated by the client and not in database components such as stored procedures. "

    A transaction should be initiated because of business logic, which occurs outside of a stored proc; the stored proc being called may be one of many, so the proc shouldn't need to track transactions outside of it's own scope.
     

    "Always have the first statement in stored procedure be "set nocount on" followed by "set xact_abort on""
     
    if you do not set nocount on, unusual behavior will occur when retreiving the results of the proc to an ADODOB recordset or ODBC driver. Setting nocount on assures only the last SELECT statement in the procedure is returned to the driver.
     

    "Error checking in stored procedure is a waste of time since many errors will cause the stored procedure to terminate, other errors will cause the batch to terminate and some errors will terminate the database connection."
     
    an update which violates a foreign key restraint would raise a run time error that would completely avoid any errors you could trap at the db level; at the client side, you could try...catch or otherwise handle the error, but not at the db /sp side. these unanticipated error are the errors you want to trap, but you cannot do it. only if you added business logic to the proc such as  raising an error if the #rows affected were not the same as the #rows expected, and using the RAISEERROR function, would capturing @@error be of much use.
     

    "The value of the return code should not have a meaning. Always return +1."
    I've always been told pass or fail...either the proc worked as expected (returned zero) or it didn't (return any other value.) pretty simple rule, and while you can catch the RETURN value, and make it have some meaning to the application, it's still pass or fail.

    Lowell


    --help us help you! If you post a question, make sure you include a CREATE TABLE... statement and INSERT INTO... statement into that table to give the volunteers here representative data. with your description of the problem, we can provide a tested, verifiable solution to your question! asking the question the right way gets you a tested answer the fastest way possible!

  • "Always have the first statement in stored procedure be "set nocount on" followed by "set xact_abort on""
    [edited to give example of the effects of setting nocount]
    set Nocount on prevents the server from sending data to the stored procedure that will not be used thus lessining network traffic 
    set xact_abort on forces the rollback to the beginning of the transaction when an error occures.
    HTH Mike 
    From Bol

    Each INSERT, UPDATE, and DELETE statement returns a result set containing only the number of rows affected by the modification. These counts can be canceled by including a SET NOCOUNT ON statement in the batch or stored procedure.

    Transact-SQL includes the SET NOCOUNT statement. When the NOCOUNT option is set to ON, SQL Server does not return the counts of the rows affected by a statement.

     

    If a run-time statement error (such as a constraint violation) occurs in a batch, the default behavior in SQL Server is to roll back only the statement that generated the error. You can change this behavior using the SET XACT_ABORT statement. After SET XACT_ABORT ON is executed, any run-time statement error causes an automatic rollback of the current transaction. Compile errors, such as syntax errors, are not affected

    /* to see the effects of setting nocount run the following with nocount off

    in the messages window you will see a message (1 row(s) affected) for each insert statement.

    Now set nocount on and run the script the mesagees window displays The command(s) completed successfully.

    when nocount is off the additional data is sent by T-sql. Best practice is to never

    request any information you do not need

     

    */

    IF OBJECT_ID('Tempdb..Test') > 0

    DROP TABLE Test

    CREATE TABLE TEST

    (

     VAL INT

    )

    set nocount off

    INSERT INTO Test Values(1)

    INSERT INTO Test Values(1)

    INSERT INTO Test Values(1)

    INSERT INTO Test Values(1)

    INSERT INTO Test Values(1)

    INSERT INTO Test Values(1)

    INSERT INTO Test Values(1)

    INSERT INTO Test Values(1)

    INSERT INTO Test Values(1)

    INSERT INTO Test Values(1)

    INSERT INTO Test Values(1)

    INSERT INTO Test Values(1)

    go

  • The value of the return code should not have a meaning. Always return +1."

    Knowing if a procedure passed or failed is vauable information and has a definite meaning.

    You can use the retrun code any way you like (which is why documenting code is important) keeping in mind that the return value is limited to data type int.

    HTH Mike

  • Mike,

    Well.. I developed the "talent of WHY" at a very young age... actually got "asked to leave" Sunday school when I was ~ 5 yrs old b/c I asked why too often... apparently I wasn't willing to take it all on faith even then.

    I expect that this might be a longer, on-going thread as I muddle through things myself and have a moment away from actual work to look at what the responses are... so I'm sure you'll have time to post   Thanks!

    Cheers,

    Dolphin/Michelle.

    "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

  • Lowell,

     

    Thanks for your responses...

     

    I should have mentioned that the problem with my situation (well one of the problems) is that the business logic is being "handled" on the front end, which is all being done in "classic" (read prehistoric) ASP.  It's a longer story than I can go into, but issues include that there is no middle layer (I know! but beyond my control), and I am VERY uncertain of the error handling that will be performed on the front-end, as I've already seen some things that scare the heck out me!  So I need to put as much as I can into the SPs so the whole thing doesn't blow up... oh and did I mention that testing... aside from what I am doing, I have been told that the app will be "tested on the fly".  I won't say more, b/c it will incriminate me (badly)... and I'll lose my temper

     

    A transaction should be initiated because of business logic, which occurs outside of a stored proc; the stored proc being called may be one of many, so the proc shouldn't need to track transactions outside of it's own scope.

     

    Ok... so with the above "background", yes the transaction should be initiated by some great BL in a middle layer that has decided that all is ok and things can proceed.. but in our case, the BL is in the ASP, JavaScript & VBScript that hopefully the other person did right (and I have some huge worries about this).  It makes sense that the SP doesn't need to track transactions outside it's own scope, but with the nesting that I am trying to do and the dependencies occurring does that change things.

     

    I.e. simplified code

    CREATE PROCEDURE OuterSP

    --As I understand it a BEGIN TRAN in the outer SP will "wrap" all the other calls inside it... including any in the inner SPs?  and the inner SPs will also be in their own transactions... anyone lost yet?

    EXEC InnerSP1

    --Inner SP1 inserts a patient

    --brings back Patient ID

     

    EXEC InnerSP2

    --needs Patient ID to proceed

    --Inner SP2 inserts a patient visit record

    --brings back Patient Visit Record ID

     

    EXEC InnerSP3

    --needs Patient Visit Record ID to proceed

    --Inner SP3 inserts a form data record

    --brings back form data record ID (may or may not be needed, still trying to nail this requirement down!)

     

    --All done?!

     

    So, since there will be transaction in all the inner SPs, with nesting, I thought it tracked the overall transaction count, or is that simply local to the SPs that it is in and the trans count will be again inside each SP?

    I hope that makes sense?!

     

    if you do not set nocount on, unusual behavior will occur when retreiving the results of the proc to an ADODOB recordset or ODBC driver. Setting nocount on assures only the last SELECT statement in the procedure is returned to the driver.

     

    Interesting... I was never taught this ever... and all I've ever used is ADODOB recordset and/or ODBC driver(s) with VB6.0 middle tier and mostly ASP/web or Access or VB front-end, and no unusual behavior occurred, I must have gotten lucky... scary!  It makes sense as I read the BOL info after re-reading Carl's post again today.  And I can see in this case where there may be various output from the inner SPs, that the required last SELECT is the one that contains the ID needed to proceed with the next INSERT.

     

    an update which violates a foreign key restraint would raise a run time error that would completely avoid any errors you could trap at the db level; at the client side, you could try...catch or otherwise handle the error, but not at the db /sp side. these unanticipated error are the errors you want to trap, but you cannot do it. only if you added business logic to the proc such as  raising an error if the #rows affected were not the same as the #rows expected, and using the RAISEERROR function, would capturing @@error be of much use.

     

    This is the one time where I like Oracle better, as I remember having greater options around things... but anyway...

    I will be able to deal with foreign key constraints by doing IF EXISTS (I hope), but in this situation, the values coming in that are FKs, came from the database initially (into a web-form) and then are being passed back... I.e. doctor ID, etc.  As mentioned if there was a middle layer, I could trust that the error handling client side would be pretty good (as I would have created the middle layer), but with the current situation, the only thing I can do is find a way to add the right balance of BL in the SP so it inserts, but doesn't break b/c the data is invalid or b/c an error occurred that I couldn't handle. 

    Are you talking about using @@ROWCOUNT then to do the above?  And why would the @@ERROR be useless here... is it b/c it's nested... so that error won't actually be returned, or am I missing something here.

     

    I've always been told pass or fail...either the proc worked as expected (returned zero) or it didn't (return any other value.) pretty simple rule, and while you can catch the RETURN value, and make it have some meaning to the application, it's still pass or fail.

     

    I was aware that the RETURN value could only be an INT, and that if it is zero if all went as planned... and if anything else was there.. it didn't.  I think the code where I actually (in the past) used it passed it back to a middle layer and if it wasn't zero then the error message came along... I don't recall right now... again... another piece of code that they tell you to use in school/job/etc (just put it in & copy & paste it , but don't worry about what it actually does!)  Sigh... so you are saying that I don't need it here in my code at all?  Or is it a “leave it, it doesn't hurt (or help)" situation.

     

    Well, this note is long enough and I have to think on the other responses some more... at least this thread will keep us entertained for a while

     

    Thanks,

    Dolphin/Michelle

    "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

Viewing 9 posts - 1 through 8 (of 8 total)

You must be logged in to reply to this topic. Login to reply