8

I am a c# developer learning more TSQL. I wrote a script like this:

begin transaction
--Insert into several tables
end transaction

But I was told that was not a good idea and to use something like this:

BEGIN TRANSACTION;

BEGIN TRY
    -- Generate a constraint violation error.
    DELETE FROM Production.Product
    WHERE ProductID = 980;
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;

    IF @@TRANCOUNT > 0
        ROLLBACK TRANSACTION;
END CATCH;

IF @@TRANCOUNT > 0
    COMMIT TRANSACTION;
GO

I don't see why the second example is more correct. Would the first one not work the same way? It seems the first one would either update all tables, or not at all? I don't see why checking the @@TRANCOUNT is necessary before the commit.

Greg Gum
  • 33,478
  • 39
  • 162
  • 233
  • 1
    I would make the same argument that you are. Additionally the try/catch pattern recommended to you is an anti-pattern I call try/squelch. It captures and error and then proceeds silently. That is NOT handling errors, it is suppressing them. That being said a try/catch block is not required for a transaction. Especially if you are in a trigger, using a try/catch will likely cause far more problems than it will ever solve. – Sean Lange May 06 '16 at 14:58
  • 2
    If anything in the second example, the commit should be in the try block, not after the catch... i think – Kritner May 06 '16 at 15:00

3 Answers3

4

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.

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

I have added a little check before actually rolling back the transaction checking for any open transaction using @@ROWCOUNT function, It doesnt really make much sence 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 try block if any of the validation checks fail, In that case control will jump to 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 as it is, you really dont need to check for any open transaction as you will not entre the catch block unless something goes wrong inside your transaction.

BEGIN TRY

  BEGIN TRANSACTION 
     -- Multiple Inserts
    INSERT INTO....
    INSERT INTO.... 
    INSERT INTO.... 
 COMMIT TRANSACTION 
    PRINT 'Rows inserted successfully...'

END TRY

BEGIN CATCH 
  IF (@@TRANCOUNT > 0)
   BEGIN
      ROLLBACK TRANSACTION 
      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
M.Ali
  • 67,945
  • 13
  • 101
  • 127
  • I think you meant `@@TranCount` in your explanation, not `@@RowCount`. I'd also hesitate to include in a general message _"all changes reversed"_ since there may be "lost" identity values, side effects caused by triggers, ... . – HABO May 06 '16 at 15:50
1

I want to give my perspective here as a C# developer:

In the simple scenario given above (just inserting into a few tables from a script), there is no reason to add a try/catch, as it adds no benefit to the transaction. Both examples will produce exactly the same result: Either all tables will be inserted, or they will not be. The state of the database remains consistent. (Since COMMIT TRANSACTION is never called, the rollback is implicitly called by Sql Server at the end of the script.)

However, there are times when you can do things in the try/catch that are not possible with the integrated error handling. For example, logging the error to an error table.

In my C# experience, the only time to use a Try/Catch is when there are things which are outside of the developer's control, such as attempting to open a file. In such a case, the only way to manage an exception generated by the .Net framework is through Try/Catch.

If I were doing a stored procedure, and wanted to manually check on the state of data and manually call ROLLBACK TRANSACTION, I could see that. But it still would not require a try/catch.

Greg Gum
  • 33,478
  • 39
  • 162
  • 233
  • What made you think that `ROLLBACK` is called automatically if `COMMIT` is not called? I think you wrong about this one, unless your *C#* program uses short-lived connections and closes them as soon as the current unit of work is finished. In that case, *MSSQL* performs a `ROLLBACK` at the moment you close the connection in *C#*, but it is only one possible approach. In most of my *C#* programs, connections to *MSSQL* have the lifetime of the program, so that I must take utmost care with transactions. I have even written a wrapper for nested transaction to make my approach more comfortable. – Anton Shepelev Aug 27 '20 at 18:00
0

I'm in two minds about TRY...CATCH in T-SQL.

While it's a potentially-useful addition to the language, the fact that it's available is not always a reason to use it.

Taking your

DELETE FROM Table WHERE...

example (which I realise is only an example). The only way that will ever fail with an error is if the code has got seriously out of whack from the schema. (for example, if someone creates a foreign key with Table on the PK end of it).

Given proper testing, that kind of mismatch between the code and the schema should never make it into production. Assuming that it does, IMHO the "rude", unmediated error message that will result is a better indication of what's gone wrong than a "polite" wrapping of it into a SELECT statement to return to the client. (Which can amount to the try/squelch antipattern SeanLange mentions).

For more complicated scenarios, I can see a use for TRY...CATCH. Though IMHO it's no substitute for careful validation of input parameters.

SebTHU
  • 1,385
  • 2
  • 11
  • 22