From: Senna on
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
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
"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
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
Yeah, I see what you mean, that makes more sense. Thanks.

Any input regarding my first question?