January 25, 2013 at 2:14 am
Hi Guys,
I've been doing a lot of reading (Books Online and articles on here) and can't manage to fix my problem so hoping one of you guys may be able to help. I have 2 Stored Procs, 1 calls the other.
Here is SP1
Create PROCEDURE [dbo].[usp_Permissions_Update_Denied_UserPermissions]
@ColleagueID int,
@PermissionID int
AS
BEGIN
-- SET NOCOUNT ON added to prevent extra result sets from
-- interfering with SELECT statements.
SET NOCOUNT ON;
BEGIN TRANSACTION
--if the permission exists in the permissions table (means its been granted)
IF EXISTS(select int_ID from dbo.tbl_Permissions_UserPermissions where int_ColleagueID = @ColleagueID and int_PermissionID = @PermissionID )
Declare @oldpermStatus int, @Reason int
--get the current status of the permission to be denied
Set @oldpermStatus = (select int_PermissionsStatus from dbo.tbl_Permissions_UserPermissions where int_ColleagueID = @ColleagueID and int_PermissionID = @PermissionID)
-- if the permstatus is 0 (means requested) then set the deny reason to 1 which is refused by admin
If (@oldpermStatus = 0)
Set @Reason = 1
else
-- set the deny reason to 3 which is request removal by authoriser
Set @Reason = 3
-- set the denied permmision to a status of 6 (which is ready to be deleted)
Update dbo.tbl_Permissions_UserPermissions
Set int_PermissionsStatus = 6
where int_ColleagueID = @ColleagueID and int_PermissionID = @PermissionID
IF @@ERROR <> 0 GOTO ErrBlock
-- find the ID of the permission and check it has a status of 6. if it does then move it to the denied table
IF exists(select int_ID from dbo.tbl_Permissions_UserPermissions where int_ColleagueID = @ColleagueID and int_PermissionsStatus = 6)
EXEC dbo.usp_Permissions_Admin_Move_Denied_UserPermissions @ColleagueID, @Reason
IF @@ERROR <> 0 GOTO ErrBlock
COMMIT TRANSACTION
Return
ErrBlock:
ROLLBACK TRANSACTION
Return
End
and here is SP2
Create PROCEDURE [dbo].[usp_Permissions_Admin_Move_Denied_UserPermissions]
-- Add the parameters for the stored procedure here
@ColleagueID int,
@Reason int
AS
BEGIN
-- SET NOCOUNT ON added to prevent extra result sets from
-- interfering with SELECT statements.
SET NOCOUNT ON;
-- Insert statements for procedure here
BEGIN TRANSACTION
Insert into dbo.tbl_Permissions_UserPermissionsDenied (int_ColleagueID,int_PermissionID,int_DenyStatus,dte_DeniedOn)
values (@ColleagueID,(Select int_PermissionID from dbo.tbl_Permissions_UserPermissions where int_ColleagueID = @ColleagueID and int_PermissionsStatus in (5)),@reason,getdate())
IF @@ERROR <> 0 GOTO ErrBlock
Delete from dbo.tbl_Permissions_UserPermissions where int_ColleagueID = @ColleagueID and int_PermissionsStatus in (6)
IF @@ERROR <> 0 GOTO ErrBlock
COMMIT TRANSACTION
Return
ErrBlock:
ROLLBACK
RETURN
End
My calling code is this: usp_Permissions_Update_Denied_UserPermissions 4,9
I have deliberately changed a bit of the code so that it forces a fail (NULL Insert shown below) and has to rollback. It does seem to do this but I get the following error every time:
Msg 515, Level 16, State 2, Procedure usp_Permissions_Admin_Move_Denied_UserPermissions, Line 21
Cannot insert the value NULL into column 'int_PermissionID', table 'dbo.tbl_Permissions_UserPermissionsDenied'; column does not allow nulls. INSERT fails.
The statement has been terminated.
Msg 266, Level 16, State 2, Procedure usp_Permissions_Admin_Move_Denied_UserPermissions, Line 0
Transaction count after EXECUTE indicates a mismatching number of BEGIN and COMMIT statements. Previous count = 2, current count = 0.
Msg 3902, Level 16, State 1, Procedure usp_Permissions_Update_Denied_UserPermissions, Line 46
The COMMIT TRANSACTION request has no corresponding BEGIN TRANSACTION.
Msg 3903, Level 16, State 1, Procedure usp_Permissions_Update_Denied_UserPermissions, Line 53
The ROLLBACK TRANSACTION request has no corresponding BEGIN TRANSACTION.
Could anyone please explain what it is I'm doing wrong as I've really got no clue!
Many thanks in advance,
Dave
January 25, 2013 at 2:28 am
Nested transactions are a lie (and a major pain)
The rollback in the inner procedure rolls back all the way to the first begin tran
Hence you enter the inner proc with a transaction open (tran count 1), start a second one (tran count now 2) hit rollback, that rolls back all open transactions (tran count 0, different from when you entered the proc), return to the outer procedure and hit commit, but there's no open transaction any longer.
Don't nest transactions (unless you really know what you're doing and how nested transactions work). Either do your transaction management in the outer proc or in the inner proc, not both.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
January 25, 2013 at 2:37 am
GilaMonster (1/25/2013)
Nested transactions are a lie (and a major pain)The rollback in the inner procedure rolls back all the way to the first begin tran
Hence you enter the inner proc with a transaction open (tran count 1), start a second one (tran count now 2) hit rollback, that rolls back all open transactions (tran count 0, different from when you entered the proc), return to the outer procedure and hit commit, but there's no open transaction any longer.
Don't nest transactions (unless you really know what you're doing and how nested transactions work). Either do your transaction management in the outer proc or in the inner proc, not both.
Thanks, GilaMonster. I have indeed learned that they are a pain!
I understand the concept of how nested transactions work and have tried using @@Trancount to see if there are still open transactions that need to commit..... but all to no avail! I was hoping someone might be able to say "ahhh you need to test for @@trancount here before you commit" ...
The problem with only doing the Rollback in 1 or the other is that each procedure can and does get called on its own so I'd really need the transaction in both.
Thanks,
Dave
January 25, 2013 at 3:04 am
Dave Hall (1/25/2013)
Thanks, GilaMonster. I have indeed learned that they are a pain!I understand the concept of how nested transactions work and have tried using @@Trancount to see if there are still open transactions that need to commit..... but all to no avail! I was hoping someone might be able to say "ahhh you need to test for @@trancount here before you commit" ...
Well, if you want... Not going to solve half the problems but....
The problem with only doing the Rollback in 1 or the other is that each procedure can and does get called on its own so I'd really need the transaction in both.
Then you're in for a world of hurt, odd bugs, annoying errors, etc. I would strongly recommend you take a long look at the design, see if you can make it so that transactions are only ever started and committed at one level, not nested.
If you have to go this way, then you need either to check the transaction count before you *begin* a transaction and have different behaviours in the procedure depending on whether there's already a transaction open or not, or you need to add savepoints and rollbacks to those savepoints if there's already a transaction open. Which one depends on what needs to roll back in the case of an error.
Neither option is clean and easy, both are going to result in lots of extra code for the transaction management.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
January 25, 2013 at 3:10 am
Instead , why cant you use SET XACT_ABORT ON in both the SP and remove all explicit transaction handling
-------Bhuvnesh----------
I work only to learn Sql Server...though my company pays me for getting their stuff done;-)
January 25, 2013 at 3:14 am
Also see this link too http://rusanu.com/2009/06/11/exception-handling-and-nested-transactions/
-------Bhuvnesh----------
I work only to learn Sql Server...though my company pays me for getting their stuff done;-)
January 25, 2013 at 3:17 am
Thanks, Gail. In a way it's a comfort to know its not just me being a pleb with this. It sounded so simple at the start!!!
I have just tested your suggestion with regards to have the transaction in 1 and not the other but hit a problem....
SP with transaction:
Create PROCEDURE [dbo].[usp_Permissions_Update_Denied_UserPermissions]
@ColleagueID int,
@PermissionID int
AS
BEGIN
-- SET NOCOUNT ON added to prevent extra result sets from
-- interfering with SELECT statements.
SET NOCOUNT ON;
BEGIN TRANSACTION
--if the permission exists in the permissions table (means its been granted)
IF EXISTS(select int_ID from dbo.tbl_Permissions_UserPermissions where int_ColleagueID = @ColleagueID and int_PermissionID = @PermissionID )
Declare @oldpermStatus int, @Reason int
--get the current status of the permission to be denied
Set @oldpermStatus = (select int_PermissionsStatus from dbo.tbl_Permissions_UserPermissions where int_ColleagueID = @ColleagueID and int_PermissionID = @PermissionID)
-- if the permstatus is 0 (means requested) then set the deny reason to 1 which is refused by admin
If (@oldpermStatus = 0)
Set @Reason = 1
else
-- set the deny reason to 3 which is request removal by authoriser
Set @Reason = 3
-- set the denied permmision to a status of 6 (which is ready to be deleted)
Update dbo.tbl_Permissions_UserPermissions
Set int_PermissionsStatus = 6
where int_ColleagueID = @ColleagueID and int_PermissionID = @PermissionID
IF @@ERROR <> 0 GOTO ErrBlock
-- find the ID of the permission and check it has a status of 6. if it does then move it to the denied table
IF exists(select int_ID from dbo.tbl_Permissions_UserPermissions where int_ColleagueID = @ColleagueID and int_PermissionsStatus = 6)
EXEC dbo.usp_Permissions_Admin_Move_Denied_UserPermissions @ColleagueID, @Reason
IF @@ERROR <> 0 GOTO ErrBlock
COMMIT TRANSACTION
Return
ErrBlock:
ROLLBACK TRANSACTION
Return
End
and the other:
Create PROCEDURE [dbo].[usp_Permissions_Admin_Move_Denied_UserPermissions]
-- Add the parameters for the stored procedure here
@ColleagueID int,
@Reason int
AS
BEGIN
-- SET NOCOUNT ON added to prevent extra result sets from
-- interfering with SELECT statements.
SET NOCOUNT ON;
-- Insert statements for procedure here
Insert into dbo.tbl_Permissions_UserPermissionsDenied (int_ColleagueID,int_PermissionID,int_DenyStatus,dte_DeniedOn)
values (@ColleagueID,(Select int_PermissionID from dbo.tbl_Permissions_UserPermissions where int_ColleagueID = @ColleagueID and int_PermissionsStatus in (5)),@reason,getdate())
Delete from dbo.tbl_Permissions_UserPermissions where int_ColleagueID = @ColleagueID and int_PermissionsStatus in (6)
End
I'm calling the 1st Proc like before and the transaction errors are gone (yay) BUT the Delete statement still executes and is not rolled back (boo).
I thought that if the call to the Proc was within a transaction then it would Rollback everything within (including actions carried out by a called SP)?
I really appreciate your insight and patience with this!
@Bhuvnes - I could do that, so I've read, but am trying to learn a little bit more..... perhaps running before I can walk here!
January 25, 2013 at 3:24 am
Dave Hall (1/25/2013)
I'm calling the 1st Proc like before and the transaction errors are gone (yay) BUT the Delete statement still executes and is not rolled back (boo).
Then i will sugges to you use TRy-Catch block
-------Bhuvnesh----------
I work only to learn Sql Server...though my company pays me for getting their stuff done;-)
January 25, 2013 at 3:27 am
I'd love to Bhuvnesh but I'm using SQL 2000 🙁
January 25, 2013 at 3:30 am
Dave Hall (1/25/2013)
I'd love to Bhuvnesh but I'm using SQL 2000 🙁
oh my bad i just realized that i am in sql 2000 window :Whistling:
-------Bhuvnesh----------
I work only to learn Sql Server...though my company pays me for getting their stuff done;-)
January 25, 2013 at 3:37 am
Dave Hall (1/25/2013)
I'm calling the 1st Proc like before and the transaction errors are gone (yay) BUT the Delete statement still executes and is not rolled back (boo).I thought that if the call to the Proc was within a transaction then it would Rollback everything within (including actions carried out by a called SP)?
Sure it will, but you have nothing that checks to see if the Insert throws an error or not. There's no checks for @@Error after the insert, so no one notices that it threw an error. Hence it looks, to your code, that the inner proc ran fine and hence gets committed.
@@Error - the error code of the previous statement that ran.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
January 25, 2013 at 3:56 am
Thank you, both for your help. I think perhaps I was trying to do too much for my first attempt at this!
Here is how I've changed the code and it now works:
SP1
ALTER PROCEDURE [dbo].[usp_Permissions_Update_Denied_UserPermissions]
@ColleagueID int,
@PermissionID int
AS
BEGIN
-- SET NOCOUNT ON added to prevent extra result sets from
-- interfering with SELECT statements.
SET NOCOUNT ON;
BEGIN TRANSACTION
Declare @RetVal int
--if the permission exists in the permissions table (means its been granted)
IF EXISTS(select int_ID from dbo.tbl_Permissions_UserPermissions where int_ColleagueID = @ColleagueID and int_PermissionID = @PermissionID )
Declare @oldpermStatus int, @Reason int
--get the current status of the permission to be denied
Set @oldpermStatus = (select int_PermissionsStatus from dbo.tbl_Permissions_UserPermissions where int_ColleagueID = @ColleagueID and int_PermissionID = @PermissionID)
-- if the permstatus is 0 (means requested) then set the deny reason to 1 which is refused by admin
If (@oldpermStatus = 0)
Set @Reason = 1
else
-- set the deny reason to 3 which is request removal by authoriser
Set @Reason = 3
-- set the denied permmision to a status of 6 (which is ready to be deleted)
begin
Update dbo.tbl_Permissions_UserPermissions
Set int_PermissionsStatus = 6
where int_ColleagueID = @ColleagueID and int_PermissionID = @PermissionID
IF @@ERROR <> 0 GOTO ErrBlock
end
-- find the ID of the permission and check it has a status of 6. if it does then move it to the denied table
IF exists(select int_ID from dbo.tbl_Permissions_UserPermissions where int_ColleagueID = @ColleagueID and int_PermissionsStatus = 6)
Exec @RetVal = dbo.usp_Permissions_Admin_Move_Denied_UserPermissions @ColleagueID, @Reason
IF @RetVal <> 0 GOTO ErrBlock
COMMIT TRANSACTION
Return
ErrBlock:
ROLLBACK TRANSACTION
Return
End
SP2
ALTER PROCEDURE [dbo].[usp_Permissions_Admin_Move_Denied_UserPermissions]
-- Add the parameters for the stored procedure here
@ColleagueID int,
@Reason int
AS
BEGIN
-- SET NOCOUNT ON added to prevent extra result sets from
-- interfering with SELECT statements.
SET NOCOUNT ON;
-- Insert statements for procedure here
Declare @RetVal int, @interror int
Set @interror = 0
Insert into dbo.tbl_Permissions_UserPermissionsDenied (int_ColleagueID,int_PermissionID,int_DenyStatus,dte_DeniedOn)
values (@ColleagueID,(Select int_PermissionID from dbo.tbl_Permissions_UserPermissions where int_ColleagueID = @ColleagueID and int_PermissionsStatus in (5)),@reason,getdate())
IF (@@ERROR <> 0) BEGIN
Set @interror = 1
END
If(@interror = 0) BEGIN
Delete from dbo.tbl_Permissions_UserPermissions where int_ColleagueID = @ColleagueID and int_PermissionsStatus in (6)
END
If (@interror = 0)
RETURN 0
else
RETURN 1
End
I'm sure there is a much nicer way and a better way of doing it but I don't know it!! 😛
January 25, 2013 at 5:31 am
Yup, that's about it for SQL 2000's error handling. Fun, isn't it?
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
Viewing 13 posts - 1 through 12 (of 12 total)
You must be logged in to reply to this topic. Login to reply