Using "Try-Catch" and "Rollback" on read only stored procedure

  • Hello All,

    I have a confusion on using "Try-Catch" and "Rollback" on stored procedures.

    In general as understand if we have a stored procedure that does operations like inserts or updates, it makes perfect sense to use a rollback operation within a transaction.

    So, if something goes wrong and the transaction does not complete, all changes will be reverted and an error description will be thrown for example.

    Nevertheless, does using a rollback within a try catch statement, make sense in a read only stored procedure, that practically executes some dynamic sql just to select data from some tables?

    I have around 100 Stored procedures, all of them read only. Today a colleague suggested adding try-catch blocks with rollback to all of them. But since they are just selecting data, I don't see a clear benefit of doing so, compared to the hassle of changing such a big number of SP's..

    Any thoughts welcome!

    Thank you.

    Serlal

  • Quick thought, looks to me like an overkill unless IMPLICIT_TRANSACTIONS are set ON in either the code or the Server's Connection Properties.

    😎

  • If you're only doing a SELECT operation, there's no reason to do a TRY/CATCH.

    ----------------------------------------------------The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood... Theodore RooseveltThe Scary DBAAuthor of: SQL Server 2017 Query Performance Tuning, 5th Edition and SQL Server Execution Plans, 3rd EditionProduct Evangelist for Red Gate Software

  • Grant Fritchey (5/1/2015)


    If you're only doing a SELECT operation, there's no reason to do a TRY/CATCH.

    TRY / CATCH is as valid / useful in SELECT as in other DML code, it is the ROLLBACK that is futile;-)

    😎

  • Eirikur Eiriksson (5/1/2015)


    Grant Fritchey (5/1/2015)


    If you're only doing a SELECT operation, there's no reason to do a TRY/CATCH.

    TRY / CATCH is as valid / useful in SELECT as in other DML code, it is the ROLLBACK that is futile;-)

    😎

    True. There just isn't much point. There are exceedingly few errors you'll encounter that going into the CATCH is going to do more than error trapping on the client.

    ----------------------------------------------------The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood... Theodore RooseveltThe Scary DBAAuthor of: SQL Server 2017 Query Performance Tuning, 5th Edition and SQL Server Execution Plans, 3rd EditionProduct Evangelist for Red Gate Software

  • Grant Fritchey (5/1/2015)


    Eirikur Eiriksson (5/1/2015)


    Grant Fritchey (5/1/2015)


    If you're only doing a SELECT operation, there's no reason to do a TRY/CATCH.

    TRY / CATCH is as valid / useful in SELECT as in other DML code, it is the ROLLBACK that is futile;-)

    😎

    True. There just isn't much point. There are exceedingly few errors you'll encounter that going into the CATCH is going to do more than error trapping on the client.

    Something like this?

    😎

    USE tempdb;

    GO

    SET NOCOUNT ON;

    DECLARE @SAMPLE TABLE (TOTAL_PRICE NUMERIC(10,2) NOT NULL, ITEM_COUNT INT NOT NULL, TRAN_DETAIL VARCHAR(50));

    INSERT INTO @SAMPLE(TOTAL_PRICE,ITEM_COUNT,TRAN_DETAIL)

    VALUES (24.10,5,'DETAIL 1'),(0,1,'DETAIL 2'),(1,0,'DETAIL 3');

    BEGIN TRY

    SELECT

    S.TOTAL_PRICE

    ,S.ITEM_COUNT

    ,S.TOTAL_PRICE / ( S.ITEM_COUNT + 0.0 ) AS AVG_PRICE

    FROM @SAMPLE S;

    END TRY

    BEGIN CATCH

    SELECT

    -1 AS TOTAL_PRICE

    ,-1 AS ITEM_COUNT

    ,'DIVISION BY ZERO' AS TRAN_DETAIL;

    END CATCH

  • Eirikur Eiriksson (5/1/2015)


    Grant Fritchey (5/1/2015)


    Eirikur Eiriksson (5/1/2015)


    Grant Fritchey (5/1/2015)


    If you're only doing a SELECT operation, there's no reason to do a TRY/CATCH.

    TRY / CATCH is as valid / useful in SELECT as in other DML code, it is the ROLLBACK that is futile;-)

    😎

    True. There just isn't much point. There are exceedingly few errors you'll encounter that going into the CATCH is going to do more than error trapping on the client.

    Something like this?

    😎

    USE tempdb;

    GO

    SET NOCOUNT ON;

    DECLARE @SAMPLE TABLE (TOTAL_PRICE NUMERIC(10,2) NOT NULL, ITEM_COUNT INT NOT NULL, TRAN_DETAIL VARCHAR(50));

    INSERT INTO @SAMPLE(TOTAL_PRICE,ITEM_COUNT,TRAN_DETAIL)

    VALUES (24.10,5,'DETAIL 1'),(0,1,'DETAIL 2'),(1,0,'DETAIL 3');

    BEGIN TRY

    SELECT

    S.TOTAL_PRICE

    ,S.ITEM_COUNT

    ,S.TOTAL_PRICE / ( S.ITEM_COUNT + 0.0 ) AS AVG_PRICE

    FROM @SAMPLE S;

    END TRY

    BEGIN CATCH

    SELECT

    -1 AS TOTAL_PRICE

    ,-1 AS ITEM_COUNT

    ,'DIVISION BY ZERO' AS TRAN_DETAIL;

    END CATCH

    OK. There's a good example. I'll shut up.

    ----------------------------------------------------The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood... Theodore RooseveltThe Scary DBAAuthor of: SQL Server 2017 Query Performance Tuning, 5th Edition and SQL Server Execution Plans, 3rd EditionProduct Evangelist for Red Gate Software

  • only word of caution would be to check where the BEGIN TRAN is. I've seen a few doozies where the "rollback" is in a child/subprocedure and is intended to rollback some transaction started in another query or procedure.

    as in - this is perfectly valid code:

    create procedure testrback as

    BEGIN

    select top 100 * from mytable

    if @@rowcount=0

    rollback

    END

    In other word - if the BEGIN TRAN is within that read only procedure it's most likely safe to yank them both, but if the rollback is the only there, you might care to do some more homework.

    ----------------------------------------------------------------------------------
    Your lack of planning does not constitute an emergency on my part...unless you're my manager...or a director and above...or a really loud-spoken end-user..All right - what was my emergency again?

  • Grant Fritchey (5/1/2015)


    .... I'll shut up.

    Didn't know that was possible:-P

    😎

  • Matt Miller (#4) (5/1/2015)


    only word of caution would be to check where the BEGIN TRAN is. I've seen a few doozies where the "rollback" is in a child/subprocedure and is intended to rollback some transaction started in another query or procedure.

    as in - this is perfectly valid code:

    create procedure testrback as

    BEGIN

    select top 100 * from mytable

    if @@rowcount=0

    rollback

    END

    In other word - if the BEGIN TRAN is within that read only procedure it's most likely safe to yank them both, but if the rollback is the only there, you might care to do some more homework.

    Good point Matt, seen some horrid consequences or rather "Lost in Transaction" kind of errors which can be very hard to trace.

    😎

  • Eirikur Eiriksson (5/1/2015)


    Grant Fritchey (5/1/2015)


    .... I'll shut up.

    Didn't know that was possible:-P

    😎

    😛

    ----------------------------------------------------The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood... Theodore RooseveltThe Scary DBAAuthor of: SQL Server 2017 Query Performance Tuning, 5th Edition and SQL Server Execution Plans, 3rd EditionProduct Evangelist for Red Gate Software

  • Grant Fritchey (5/1/2015)


    Eirikur Eiriksson (5/1/2015)


    Grant Fritchey (5/1/2015)


    Eirikur Eiriksson (5/1/2015)


    Grant Fritchey (5/1/2015)


    If you're only doing a SELECT operation, there's no reason to do a TRY/CATCH.

    TRY / CATCH is as valid / useful in SELECT as in other DML code, it is the ROLLBACK that is futile;-)

    😎

    True. There just isn't much point. There are exceedingly few errors you'll encounter that going into the CATCH is going to do more than error trapping on the client.

    Something like this?

    😎

    USE tempdb;

    GO

    SET NOCOUNT ON;

    DECLARE @SAMPLE TABLE (TOTAL_PRICE NUMERIC(10,2) NOT NULL, ITEM_COUNT INT NOT NULL, TRAN_DETAIL VARCHAR(50));

    INSERT INTO @SAMPLE(TOTAL_PRICE,ITEM_COUNT,TRAN_DETAIL)

    VALUES (24.10,5,'DETAIL 1'),(0,1,'DETAIL 2'),(1,0,'DETAIL 3');

    BEGIN TRY

    SELECT

    S.TOTAL_PRICE

    ,S.ITEM_COUNT

    ,S.TOTAL_PRICE / ( S.ITEM_COUNT + 0.0 ) AS AVG_PRICE

    FROM @SAMPLE S;

    END TRY

    BEGIN CATCH

    SELECT

    -1 AS TOTAL_PRICE

    ,-1 AS ITEM_COUNT

    ,'DIVISION BY ZERO' AS TRAN_DETAIL;

    END CATCH

    OK. There's a good example. I'll shut up.

    It's clever but I have to disagree with that being a good example of a "Read Only Try/Catch" because it returns two result sets instead of just one. It also doesn't return all of the "bad" items and if the "first" item is a bad item, it doesn't return any of the good ones.

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.
    "Change is inevitable... change for the better is not".

    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)
    Intro to Tally Tables and Functions

  • Jeff Moden (5/1/2015)


    It's clever but I have to disagree with that being a good example of a "Read Only Try/Catch" because it returns two result sets instead of just one. It also doesn't return all of the "bad" items and if the "first" item is a bad item, it doesn't return any of the good ones.

    You are spot on here Jeff, my example isn't the best.;-)

    😎

    Having said that, it's not too far off either, changing it to bring back a single record in a consistent form regardless of errors is straight forward.

    Using TRY/CATCH like this, significantly simplifies the UI or the calling code and also the testing of the whole thing.

    USE tempdb;

    GO

    SET NOCOUNT ON;

    DECLARE @TRAN_ID INT = 10002;

    DECLARE @RESULT TABLE

    (

    TRAN_ID INT NOT NULL

    ,TOTAL_PRICE NUMERIC(10,2) NOT NULL

    ,ITEM_COUNT INT NOT NULL

    ,AVG_PRICE NUMERIC(10,2) NOT NULL

    ,TRAN_DETAIL VARCHAR(50)

    );

    DECLARE @SAMPLE TABLE

    (

    TRAN_ID INT IDENTITY(10000,1) NOT NULL

    ,TOTAL_PRICE NUMERIC(10,2) NOT NULL

    ,ITEM_COUNT INT NOT NULL

    ,TRAN_DETAIL VARCHAR(50) NOT NULL

    );

    INSERT INTO @SAMPLE(TOTAL_PRICE,ITEM_COUNT,TRAN_DETAIL)

    VALUES (24.10,5,'DETAIL 1'),(0,1,'DETAIL 2'),(1,0,'DETAIL 3');

    BEGIN TRY

    INSERT INTO @RESULT(TRAN_ID,TOTAL_PRICE,ITEM_COUNT,AVG_PRICE,TRAN_DETAIL)

    SELECT

    S.TRAN_ID

    ,S.TOTAL_PRICE

    ,S.ITEM_COUNT

    ,S.TOTAL_PRICE / ( S.ITEM_COUNT + 0.0 ) AS AVG_PRICE

    ,S.TRAN_DETAIL

    FROM @SAMPLE S

    WHERE S.TRAN_ID = @TRAN_ID;

    END TRY

    BEGIN CATCH

    INSERT INTO @RESULT(TRAN_ID,TOTAL_PRICE,ITEM_COUNT,AVG_PRICE,TRAN_DETAIL)

    SELECT

    S.TRAN_ID

    ,S.TOTAL_PRICE

    ,S.ITEM_COUNT

    ,-1 AS AVG_PRICE

    ,ERROR_MESSAGE() AS TRAN_DETAIL

    FROM @SAMPLE S

    WHERE S.TRAN_ID = @TRAN_ID;

    END CATCH

    SELECT

    R.TRAN_ID

    ,R.TOTAL_PRICE

    ,R.ITEM_COUNT

    ,R.AVG_PRICE

    ,R.TRAN_DETAIL

    FROM @RESULT R;

    Sample output

    TRAN_ID TOTAL_PRICE ITEM_COUNT AVG_PRICE TRAN_DETAIL

    -------- ------------ ----------- ---------- ----------------------------------

    10002 1.00 0 -1.00 Divide by zero error encountered.

  • My problem with Try/Catch in such situations is that such situations should not occur at all. Either the data is bad or the query is bad. It shouldn't be returning things that could be divided by 0 to begin with, in this case.

    I also have a problem with the use of Try/Catch for non-read-only procs. Most people simply roll things back if a transaction is underway and then they simply regurgitate what SQL Server would have produced to begin with except they obfuscate the true line number that caused the actual error.

    In most cases, the use of explicit transactions along with SET XACT_ABORT ON is more than sufficient for error handling in stored procedures because the error handling of SQL Server is usually as good as or better than the error handling Developers try to build into their CATCH blocks.

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.
    "Change is inevitable... change for the better is not".

    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)
    Intro to Tally Tables and Functions

  • Jeff Moden (5/2/2015)


    My problem with Try/Catch in such situations is that such situations should not occur at all. Either the data is bad or the query is bad. It shouldn't be returning things that could be divided by 0 to begin with, in this case.

    I also have a problem with the use of Try/Catch for non-read-only procs. Most people simply roll things back if a transaction is underway and then they simply regurgitate what SQL Server would have produced to begin with except they obfuscate the true line number that caused the actual error.

    In most cases, the use of explicit transactions along with SET XACT_ABORT ON is more than sufficient for error handling in stored procedures because the error handling of SQL Server is usually as good as or better than the error handling Developers try to build into their CATCH blocks.

    As I mentioned before, returning a consistent output is very useful when i.e. coding an API for a UI. It also suppresses any error messages that might be exploited further. When the back end has to use dynamic sql, normally I would catch any errors generated by the dynamic sql code, log them and return an empty set to the UI.

    😎

    Although many discrepancies in the data can be handled in the query code, not all can. External dependencies such as Location coordinates, Exchange Rates, Prices, Transaction details from a sales system etc. can fail and will fail and depending on the usage, even the most carefully crafted code could fail in those cases.

    My problem with the TRY/CATCH is that it doesn't catch errors to the severity it should, i.e. parse, arithmetic and CLR errors cannot be suppressed which forces one to revert to cursors and other RBAR methods such as the one I wrote about here[/url].

Viewing 15 posts - 1 through 14 (of 14 total)

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