From: Senna on 24 Mar 2010 01:58 Hi Am trying to code correct and are wondering which of these code blocks are the most right. Thanks --Version 1: BEGIN TRY BEGIN TRANSACTION DELETE FROM dbo.Address WHERE UserId = @userid; IF(@@ROWCOUNT = 0) BEGIN IF(@@TRANCOUNT > 0) ROLLBACK TRANSACTION; SET @statuscode = 20; RETURN; END SET @statuscode = 100; COMMIT TRANSACTION; END TRY BEGIN CATCH IF(@@TRANCOUNT > 0)ROLLBACK TRANSACTION; EXEC dbo.Error_ErrorHandler; SET @statuscode = 99; END CATCH --Version 2: BEGIN TRY SET NOCOUNT ON SET XACT_ABORT ON DELETE FROM dbo.Address WHERE UserId = @userid; IF(@@ROWCOUNT = 0) BEGIN IF(@@TRANCOUNT > 0) ROLLBACK TRANSACTION; SET @statuscode = 20; RETURN; END SET @statuscode = 100; END TRY BEGIN CATCH IF (XACT_STATE()) = -1) ROLLBACK TRANSACTION; ELSE IF (XACT_STATE()) = 1) COMMIT TRANSACTION; EXEC dbo.Error_ErrorHandler; SET @statuscode = 99; END CATCH --Version 3: BEGIN TRY SET NOCOUNT ON SET XACT_ABORT ON BEGIN TRANSACTION DELETE FROM dbo.Address WHERE UserId = @userid; IF(@@ROWCOUNT = 0) BEGIN IF(@@TRANCOUNT > 0) ROLLBACK TRANSACTION; SET @statuscode = 20; RETURN; END SET @statuscode = 100; COMMIT TRANSACTION; END TRY BEGIN CATCH IF(@@TRANCOUNT > 0) ROLLBACK TRANSACTION; EXEC dbo.Error_ErrorHandler; SET @statuscode = 99; END CATCH --Version 4: BEGIN TRY SET NOCOUNT ON SET XACT_ABORT ON BEGIN TRANSACTION DELETE FROM dbo.Address WHERE UserId = @userid; IF(@@ROWCOUNT = 0) BEGIN IF(@@TRANCOUNT > 0) ROLLBACK TRANSACTION; SET @statuscode = 20; RETURN; END SET @statuscode = 100; COMMIT TRANSACTION; END TRY BEGIN CATCH IF (XACT_STATE()) = -1) ROLLBACK TRANSACTION; ELSE IF (XACT_STATE()) = 1) COMMIT TRANSACTION; EXEC dbo.Error_ErrorHandler; SET @statuscode = 99; END CATCH
From: Uri Dimant on 24 Mar 2010 06:18 Senna How about the below I am not sure why ROLLBACK if the (@@ROWCOUNT = 0) just set a status , am I missing something? BEGIN TRY SET NOCOUNT ON SET XACT_ABORT ON BEGIN TRANSACTION DELETE FROM dbo.Address WHERE UserId = @userid; IF(@@ROWCOUNT = 0) BEGIN IF(@@TRANCOUNT > 0) ROLLBACK TRANSACTION; SET @statuscode = 20; RETURN; END SET @statuscode = 100; COMMIT TRANSACTION; END TRY BEGIN CATCH IF(@@TRANCOUNT > 0)ROLLBACK TRANSACTION; EXEC dbo.Error_ErrorHandler; SET @statuscode = 99; END CATCH "Senna" <Senna(a)discussions.microsoft.com> wrote in message news:BE9CAE11-21D8-458C-AD07-77A5440BE384(a)microsoft.com... > Hi > > Am trying to code correct and are wondering which of these code blocks are > the most right. > > Thanks > > --Version 1: > BEGIN TRY > BEGIN TRANSACTION > DELETE FROM dbo.Address WHERE UserId = @userid; > > IF(@@ROWCOUNT = 0) > BEGIN > IF(@@TRANCOUNT > 0) ROLLBACK TRANSACTION; > SET @statuscode = 20; > RETURN; > END > > SET @statuscode = 100; > COMMIT TRANSACTION; > END TRY > BEGIN CATCH > IF(@@TRANCOUNT > 0)ROLLBACK TRANSACTION; > EXEC dbo.Error_ErrorHandler; > SET @statuscode = 99; > END CATCH > > --Version 2: > BEGIN TRY > SET NOCOUNT ON > SET XACT_ABORT ON > > DELETE FROM dbo.Address WHERE UserId = @userid; > > IF(@@ROWCOUNT = 0) > BEGIN > IF(@@TRANCOUNT > 0) ROLLBACK TRANSACTION; > SET @statuscode = 20; > RETURN; > END > > SET @statuscode = 100; > END TRY > BEGIN CATCH > IF (XACT_STATE()) = -1) > ROLLBACK TRANSACTION; > ELSE IF (XACT_STATE()) = 1) > COMMIT TRANSACTION; > > EXEC dbo.Error_ErrorHandler; > SET @statuscode = 99; > END CATCH > > --Version 3: > BEGIN TRY > SET NOCOUNT ON > SET XACT_ABORT ON > > BEGIN TRANSACTION > DELETE FROM dbo.Address WHERE UserId = @userid; > > IF(@@ROWCOUNT = 0) > BEGIN > IF(@@TRANCOUNT > 0) ROLLBACK TRANSACTION; > SET @statuscode = 20; > RETURN; > END > > SET @statuscode = 100; > COMMIT TRANSACTION; > END TRY > BEGIN CATCH > IF(@@TRANCOUNT > 0) ROLLBACK TRANSACTION; > EXEC dbo.Error_ErrorHandler; > SET @statuscode = 99; > END CATCH > > --Version 4: > BEGIN TRY > SET NOCOUNT ON > SET XACT_ABORT ON > > BEGIN TRANSACTION > DELETE FROM dbo.Address WHERE UserId = @userid; > > IF(@@ROWCOUNT = 0) > BEGIN > IF(@@TRANCOUNT > 0) ROLLBACK TRANSACTION; > SET @statuscode = 20; > RETURN; > END > > SET @statuscode = 100; > COMMIT TRANSACTION; > END TRY > BEGIN CATCH > IF (XACT_STATE()) = -1) > ROLLBACK TRANSACTION; > ELSE IF (XACT_STATE()) = 1) > COMMIT TRANSACTION; > > EXEC dbo.Error_ErrorHandler; > SET @statuscode = 99; > END CATCH
From: Senna on 24 Mar 2010 07:35 "Uri Dimant" wrote: > Senna > How about the below > I am not sure why ROLLBACK if the (@@ROWCOUNT = 0) just set a status , am I > missing something? Asked that in a previous question and got the answer: By Tom "Yes you should end the transaction before you leave. If you don't any locks held by the transaction will be kept until something ends the transaction. You should never return to the client without disposing all locks. In the case above if this procedure was called by another procedure which recognized that it needed to rollback the time the locks were held would likely be short and of not much consequence. The problem with doing this is someday it might get called from a client and the client could go home for the night holding locks and causing quite a mess." By Erland Sommarskog: "> In the below code I do a RETURN within a TRANSACTION, is this ok? Or do > I have to do a ROLLBACK before the return. What are the best practice > for this? If you start a transaction in a stored procedure, and then exit the procedure without committing or rolling it back, you will get error 266: Transaction count after EXECUTE indicates that a COMMIT or ROLLBACK TRANSACTION statement is missing. Previous count = 0, current count = 1. I would suggest that COMMIT TRANSACTION is the best alternative. The caller may have started a transaction on its own, and with COMMIT you don't pull the rug for the caller. However: > DELETE FROM dbo.Address WHERE UserId = @userid; > IF(@@ROWCOUNT = 0) > BEGIN > SET @statuscode = 20; > RETURN; > END > > UPDATE dbo.Customer SET Something = 1 WHERE UserId = @userid; If the business logic is such that there DELETE must not occur if the row is not present in the Customer table, you need to do a ROLLBACK here. > BEGIN CATCH > IF(@@TRANCOUNT > 0) > ROLLBACK TRANSACTION; And certainly if a run-time error occurs, since you no longer have control over the situation."
From: Uri Dimant on 24 Mar 2010 08:20 Senna It ois ok what they wrote but you do have the below, BEGIN CATCH IF @@TRANCOUNT > 0 ROLLBACK TRANSACTION; EXEC dbo.Error_ErrorHandler; SET @statuscode = 99 In your code you checked if no rows are affected then ROLLBACK, why? What do you ROLLBACK? Just trying to understand I would write .................. IF(@@ROWCOUNT = 0) SET @statuscode = 20; ELSE SET @statuscode = 100; COMMIT TRANSACTION BEGIN CATCH IF @@TRANCOUNT > 0 ROLLBACK TRANSACTION; EXEC dbo.Error_ErrorHandler; SET @statuscode = 99 END CATCH "Senna" <Senna(a)discussions.microsoft.com> wrote in message news:F6C5F4F3-9501-44A1-9074-6574D0A719FC(a)microsoft.com... > "Uri Dimant" wrote: >> Senna >> How about the below >> I am not sure why ROLLBACK if the (@@ROWCOUNT = 0) just set a status , am >> I >> missing something? > > Asked that in a previous question and got the answer: > > By Tom > "Yes you should end the transaction before you leave. If you don't any > locks held by the transaction will be kept until something ends the > transaction. You should never return to the client without disposing > all locks. In the case above if this procedure was called by another > procedure which recognized that it needed to rollback the time the > locks were held would likely be short and of not much consequence. The > problem with doing this is someday it might get called from a client > and the client could go home for the night holding locks and causing > quite a mess." > > By Erland Sommarskog: > "> In the below code I do a RETURN within a TRANSACTION, is this ok? Or do >> I have to do a ROLLBACK before the return. What are the best practice >> for this? > > If you start a transaction in a stored procedure, and then exit the > procedure without committing or rolling it back, you will get error > 266: > > Transaction count after EXECUTE indicates that a COMMIT or ROLLBACK > TRANSACTION statement is missing. Previous count = 0, current count = > 1. > > I would suggest that COMMIT TRANSACTION is the best alternative. The > caller may have started a transaction on its own, and with COMMIT you > don't pull the rug for the caller. > > However: > >> DELETE FROM dbo.Address WHERE UserId = @userid; >> IF(@@ROWCOUNT = 0) >> BEGIN >> SET @statuscode = 20; >> RETURN; >> END >> >> UPDATE dbo.Customer SET Something = 1 WHERE UserId = @userid; > > If the business logic is such that there DELETE must not occur if the > row is not present in the Customer table, you need to do a ROLLBACK > here. > >> BEGIN CATCH >> IF(@@TRANCOUNT > 0) >> ROLLBACK TRANSACTION; > > And certainly if a run-time error occurs, since you no longer have control > over the situation."
From: Senna on 25 Mar 2010 08:17
Yeah, I see what you mean, that makes more sense. Thanks. Any input regarding my first question? |