50

I'm writing a script that will delete records from a number of tables, but before it deletes it must return a count for a user to confirm before committing.

This is a summary of the script.

BEGIN TRANSACTION SCHEDULEDELETE
    BEGIN TRY
        DELETE   -- delete commands full SQL cut out
        DELETE   -- delete commands full SQL cut out
        DELETE   -- delete commands full SQL cut out
        PRINT 'X rows deleted. Please commit or rollback.' --calculation cut out.
    END TRY
    BEGIN CATCH 
        SELECT
            ERROR_NUMBER() AS ErrorNumber,
            ERROR_SEVERITY() AS ErrorSeverity,
            ERROR_STATE() AS ErrorState,
            ERROR_PROCEDURE() AS ErrorProcedure,
            ERROR_LINE() AS ErrorLine,
            ERROR_MESSAGE() AS ErrorMessage

            ROLLBACK TRANSACTION SCHEDULEDELETE
            PRINT 'Error detected, all changes reversed.'
    END CATCH

--COMMIT TRANSACTION SCHEDULEDELETE --Run this if count correct.

--ROLLBACK TRANSACTION SCHEDULEDELETE --Run this if there is any doubt whatsoever.

This is my first time writing transaction, is it correct/best practice to have the TRY/CATCH block inside the transaction or should the transaction be inside the TRY block?

The important factor in this script is that the user must manually commit the transaction.

Brian Tompsett - 汤莱恩
  • 5,753
  • 72
  • 57
  • 129
Devasta
  • 1,489
  • 2
  • 17
  • 28
  • 1
    yup outside the try/catch block. – cyan Apr 14 '14 at 10:04
  • 1
    Never wait for an end user to commit the transaction, unless it's a single-user mode database. – dean Apr 14 '14 at 10:15
  • @dean Is there a problem with that? Basically, the user is going to be told that they should expect X number of records deleted, and to commit right away if the number matches. These tables will not be written to while the transaction is running (although other tables will be). – Devasta Apr 14 '14 at 10:19
  • I second what @dean said. Unless your users are somewhat experienced in this sort of stuff, you'll be in a world of trouble. Even then user errors will occur sooner or later. Better to just fill whatever information gaps you have, preventing you from creating a reliable script in the first place, if possible. If it's absolutely necessary to confirm the results before committing, then I'd recommend putting the script on rollback by default. But even that's not really good practice. – Kahn Apr 14 '14 at 10:20
  • If they know beforehand what the number of records affected should be, then better to use a parameter to commit when matched (and no errors found) and rollback with reason and output, when not matched. It's just better in the long run to leave as little room for user error and temporary negligence as possible, the better you can automate these things, the more reliable they tend to be. – Kahn Apr 14 '14 at 10:21
  • @Kahn The user is a DBA with far more experience than I. This script is going to be run every 6 months or so tops. The script is 100% reliable (famous last words!) but I was hoping for it just to have one extra check, just in case. – Devasta Apr 14 '14 at 10:25

3 Answers3

92

Only open a transaction once you are inside the TRY block and just before the actual statement, and commit it straightaway. Do not wait for your control to go to the end of the batch to commit your transactions.

If something goes wrong while you are in the TRY block and you have opened a transaction, the control will jump to the CATCH block. Simply rollback your transaction there and do other error handling as required.

I have added a little check for any open transaction using @@TRANCOUNT function before actually rolling back the transaction. It doesn't really make much sense in this scenario. It is more useful when you are doing some validations checks in your TRY block before you open a transaction like checking param values and other stuff and raising error in the TRY block if any of the validation checks fail. In that case, the control will jump to the CATCH block without even opening a transaction. There you can check for any open transaction and rollback if there are any open ones. In your case, you really don't need to check for any open transaction as you will not enter the CATCH block unless something goes wrong inside your transaction.

Do not ask after you have executed the DELETE operation whether it needs to be committed or rolled back; do all these validation before opening the transaction. Once a transaction is opened, commit it straightaway and in case of any errors, do error handling (you are doing a good job by getting detailed info by using almost all of the error functions).

BEGIN TRY

  BEGIN TRANSACTION SCHEDULEDELETE
    DELETE   -- delete commands full SQL cut out
    DELETE   -- delete commands full SQL cut out
    DELETE   -- delete commands full SQL cut out
 COMMIT TRANSACTION SCHEDULEDELETE
    PRINT 'X rows deleted. Operation Successful Tara.' --calculation cut out.
END TRY

BEGIN CATCH 
  IF (@@TRANCOUNT > 0)
   BEGIN
      ROLLBACK TRANSACTION SCHEDULEDELETE
      PRINT 'Error detected, all changes reversed'
   END 
    SELECT
        ERROR_NUMBER() AS ErrorNumber,
        ERROR_SEVERITY() AS ErrorSeverity,
        ERROR_STATE() AS ErrorState,
        ERROR_PROCEDURE() AS ErrorProcedure,
        ERROR_LINE() AS ErrorLine,
        ERROR_MESSAGE() AS ErrorMessage
END CATCH
Rich Benner
  • 7,873
  • 9
  • 33
  • 39
M.Ali
  • 67,945
  • 13
  • 101
  • 127
  • 1
    I know transactions have been supported by SQL Server for decades, but `TRY/CATCH` was only recently added - so how was `BEGIN TRANSACTION + COMMIT + ROLLBACK` supposed to be implemented before `TRY/CATCH` was added to SQL Server? – Dai Aug 24 '20 at 11:54
  • There is no reason at all to open a transaction insde the `TRY` block. In fact, opening it immediately before the `TRY` is the correct structure. – Anton Shepelev Aug 26 '20 at 10:28
  • 1
    @Dai `TRY/CATCH` has been in SQL Server since 2005, and before SQL Server 2005 (i.e. SQL Sever 2000 and older versions) there were a looooot of things we couldnt do, or there were very convoluded ways of doing the simplest things. So I hope you are not on SQL Server 2000 :) . Just so you know before 2005 we used `@@Error` function after each statement to check whether the statement errored out. – M.Ali Aug 26 '20 at 12:03
  • 2
    @Ant_222 would you like to expand on your claim please? I have provided reasons in detail for the claims I have made. You have just made a statement but haven't provided any reason why your suggested method is the right way to do. – M.Ali Aug 26 '20 at 12:08
  • The reason for keeping `BEGIN TRAN` *outside* the `TRY` blolck is simple: if `BEGIN TRAN` fails, you do not need to `ROLLBACK`. Now, I should like to hear your reason for putting `BEGIN TRAN` *inside* the `TRY` block (I do not see one in your answer). What may go wrong with `BEGIN TRAN` outside? – Anton Shepelev Aug 26 '20 at 15:15
  • 1
    @Ant_222 you are talking abosulote bollocks unfortunately, if you open a transaction outside of try block and do not rollback inside catch block you would leave an open transaction. You will always have to rollback or commit a transaction once you have opened it, your claim that if a transaction fails you would not need to rollback is utter BS sorry to say mate. As far as my reason to open a transaction inside try block, read the 3rd paragraph in my answer carefully. (Additional validation checks before you open a transaction) hint hint – M.Ali Aug 26 '20 at 15:34
  • @Ant_222 its good to have opinions but opinions based on just opinion are mostly nonsense, I suggest you do some reading on how transactions work in SQL Server and maybe then we can have a bit more intellectual debate have a read of this please `[SQL Server Transaction](https://learn.microsoft.com/en-us/sql/t-sql/language-elements/transactions-transact-sql?view=sql-server-ver15)` – M.Ali Aug 26 '20 at 15:38
  • 1
    Watch your language, *Ali*. If `BEGIN TRAN` fails (I did not write *if a transaction fails*!) then there is nothing to `ROLLBACK` and everying is fine. If, on the other hand, `BEGIN TRAN` succeeds, we enter the `TRY` block and thus guarrantee either to `ROLLBACK` or to `COMMIT` the successfully started transaction. So you are wrong. Your test of `@@TRANCOUNT` is just a workaround for the wrongly placed `BEGIN TRAN`. Place it before `TRY` and you shan't need it. If you insist that my approach is wrong, post a small *T-SQL* script breaks it. OK? – Anton Shepelev Aug 26 '20 at 16:29
  • @Ant_222 again you are making outrageous assumptions, how and when a Begin TRAN would fail? It never fails it’s the t-sql statement inside a transaction that can fail. Can you provide an example and make begin TRAN fail? – M.Ali Aug 26 '20 at 16:34
  • 1
    If `BEGIN TRAN` cannot fail, there is even less reason to put it insde the `TRY` block. `TRY` blocks are for stuff that *can* fail. – Anton Shepelev Aug 26 '20 at 16:35
  • @Ant_222 so now you know that BEGIN TRAN can never fail, which you put forward as the main reason why you should put outside of try block, now what reason do you have to put it outside of the try block, you know my reason for putting inside the try block but you havent given a single valid reason that why we should put outside of the try block? – M.Ali Aug 26 '20 at 16:41
  • No, I do not know your justification for putting `BEGIN TRAN` inside the `TRY` block, because you have not provided any. You said my approach was wrong and I asked you to demonstrate how it can be broken. [Here](https://pastebin.com/raw/09GSfRV0) is what I consider the correct pattern for handling a transaction in *T-SQL*. Can you break it? If not, then please take back your harsh words. The correctness of my approach is indefferent to whether `BEGIN TRAN` may fail or not. The documentation does not tell it, by the way. – Anton Shepelev Aug 26 '20 at 16:52
  • @Ant_222 I asked you to read the 3rd paragraph from answer, the reason is there anway I will repeat myself here again. The reason you want to put begin tran inside a try block is, usually you will have some validation checks like checking values passed in params and other sorts of validations checks, if any of the checks fail, you raise an error and the control jumps to catch block without even openning any trasaction, if all the validation passes only then you open a transaction and commit it straight after the statement execution finishes. Keeping the transaction time as short as possible. – M.Ali Aug 26 '20 at 17:06
  • @Ant_222 looking at the code you have provided , you have put a rollback in the catch block contrary to your claims only a few comments ago :) everyday is a school day :) – M.Ali Aug 26 '20 at 17:10
  • I agree that performing non-destructive checks outside a transaction to keep its lifetime shorter is very important, but I did not realise you proposed to singnal failed checks by `RAISE`ing errors and `CATCH`ing them. Now I see your point. But that would be (ab)using the same `TRY..CATCH` block for two purposes: terminating a transaction and reporting failed preconditions, which I never considered. I prefer each `TRY` to have its own purpose, to keep the code clearer. And I never proposed to put `ROLLBACK` outside the `CATCH` block. – Anton Shepelev Aug 27 '20 at 09:31
  • 1
    @M.Ali and @Ant_22, This was a very confrontational discussion and I think both ways of doing it have merit. My gut instinct was to put the `TRAN` outside of the `TRY \ CATCH` and after reading your whole discussion I feel that it technically doesn't matter(?) so I'm going to stick with `TRAN -> TRY` as it makes more sence in my head. It just doesn't smell right to be handling the `ROLLBACK` outside of the block that created the `TRAN`. – Morvael Aug 10 '21 at 07:42
  • https://learn.microsoft.com/en-us/sql/t-sql/language-elements/try-catch-transact-sql?view=sql-server-ver15#b-using-trycatch-in-a-transaction - Microsoft recommends to have TRANSACTION -> TRY ? – variable Jan 24 '22 at 12:11
  • @variable thank you for that, I was looking for something like this. Microsoft appears to recommend TRANS -> TRY, but under item C, they open they BEGIN TRY -> BEGIN TRANS so it doesn't look like we're any closer. https://learn.microsoft.com/en-us/sql/t-sql/language-elements/try-catch-transact-sql?view=sql-server-ver15#c-using-trycatch-with-xact_state – Steven Bitaxi Feb 03 '22 at 18:53
13

In addition to the good advice by M.Ali and dean above, a little bit help for those looking to use the new(er) TRY CATCH THROW paradigm in SQL SERVER:

(I couldn't easily find the complete syntax, so adding it here)

GIST : HERE

Sample stored procedure code here (from my gist):

SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
 
CREATE PROC [dbo].[pr_ins_test]
@CompanyID INT
AS
 
SET NOCOUNT ON
 
BEGIN
 
    DECLARE @PreviousConfigID INT
    
    BEGIN TRY
        BEGIN TRANSACTION MYTRAN; -- Give the transaction a name
        SELECT 1/0  -- Generates divide by zero error causing control to jump into catch
 
        PRINT '>> COMMITING'
        COMMIT TRANSACTION MYTRAN;
    END TRY
    BEGIN CATCH
        IF @@TRANCOUNT > 0
        BEGIN 
            PRINT '>> ROLLING BACK'
            ROLLBACK TRANSACTION MYTRAN; -- The semi-colon is required (at least in SQL 2012)
            
            
        END; -- I had to put a semicolon to avoid error near THROW
        THROW
    END CATCH
END
Ketan
  • 5,861
  • 3
  • 32
  • 39
Sudhanshu Mishra
  • 6,523
  • 2
  • 59
  • 76
  • Just an observation - The THROW should be outside of the BEGIN/END so it throws regardless of a transaction being open. – Mark G Apr 30 '18 at 22:19
  • @MarkG not sure I understand, if the THROW is outside of BEGIN/END Catch, executing this stored proc would always throw an exception, unless you follow the commit tran with a return statement. As the excerpt stands now, just moving the throw outside would be useless – Sudhanshu Mishra May 02 '18 at 07:03
  • 2
    I suspect @MarkG is saying that the THROW should be after the END but before the END CATCH. This would ensure that the exception is thrown irrespective of @@TRANCOUNT. – BrianB May 03 '18 at 14:06
  • I tried to add the THROW statement within a database project in Visual studio and get an error. – Rich May 23 '19 at 04:38
  • The way I deal with the semi-colon regarding `THROW` is I just write `;THROW;` - whatever works. I just like doing it this way so I don't forget to add it. – dyslexicanaboko Dec 16 '21 at 18:30
  • https://learn.microsoft.com/en-us/sql/t-sql/language-elements/try-catch-transact-sql?view=sql-server-ver15#b-using-trycatch-in-a-transaction - Microsoft recommends to have TRANSACTION -> TRY ? – variable Jan 24 '22 at 12:10
3

Never wait for an end user to commit the transaction, unless it's a single-user mode database.

In short, it's about blocking. Your transaction will take some exclusive locks on resources being updated, and will hold on to those lock untill the transaction is ended (committed or rolled back). Nobody will be able to touch those rows. There are some different problems if snapshot isolation is used with version store cleanup.

Better to first issue a select query to determine a number of qualifying rows, present this to the end user, and after he confirms do the actual delete.

dean
  • 9,960
  • 2
  • 25
  • 26
  • 1
    This is the right solution. But make sure to remember the rows eligible for deletion and delete pricesely them, instead of whatever happens to be there when the user confirms the operation. – Anton Shepelev Aug 26 '20 at 10:36