Call SQL Server stored procedure from C# and determine commit or rollback

Code:

---

CREATE PROCEDURE xsp_DeleteThis
        @pk int
AS
BEGIN

SET XACT_ABORT ON

        BEGIN TRANSACTION
               
                UPDATE tblX SET ... WHERE pk = @pk
                       
                UPDATE tblY SET ... WHERE pk = @pk
               
                UPDATE xtblZ SET ... WHERE pk = @pk

        COMMIT TRANSACTION

END

---

I have a few basic questions.

Is that a good way to write a stored procedure where I want all UPDATE's to be atomic?

Is the rollback implied by the directive SET XACT_ABORT ON?

In my C# code, what will I get back if the call fails/succeeds?

Thanks, I know these are simple questions but I am not getting the right hits when I google. And in case you're wondering why a delete sp updates...I am setting fields in each of the three tables that will indicate the record is "deleted" but not actually delete it.

What do you think will fail with your UPDATE statements?

I would rather use "If @@RowCount<>1..." and then handle an error in an update that hits no records.

But again - what are you expecting to fail?

That is incorrect use of SET XACT_ABORT - it does not go in a STORED PROCEDURE.

"The setting of SET XACT_ABORT is set at execute or run time and not at parse time."

https://msdn.microsoft.com/en-us/library/ms188792.aspx

I still want to know what "errors you are expecting".

But regardless - look at TRY/CATCH

https://msdn.microsoft.com/en-us/library/ms175976.aspx

and also understand that some errors are not catchable. Usually none of the ones you would expect :)

I don't expect any code that I write to fail. Yet occasionally it does :).

But in this particular scenario, you are right. So you recommend stripping out the transaction stuff and just running three updates? Also, RowCount may be more than 1. I know I mislead you calling the param PK, but it's actually more of an FK but not officially. So if I wanted to make sure I update at least one row before going onto the next update, I would check >0, I guess.

I don't know...I'm converting vb6 code and trying to make it better instead of just doing a brainless convert. The original code sent 1st update to db, sent 2nd update to db, sent 3rd, and that's it. I thought I would combine into an SP and call once and add error checking.

Wow - forum will not let me post code!! I'll post an image!!!

forum is broken

I know! That's why I posted this info in two parts! Was trying since yesterday. Go to Forum Feedback you will see it's a known issue.

Would I also want to SELECT GotError so it goes back to my code, so my code can know if sproc succeeded or failed? Or, can the code know if the commit or the rollback happened?

You could add a throw in the proc I guess. Or add an output to capture the error code and return that

I like to do a "SELECT @GotError" usually - and just look for that result.

Or I get more sophisticated with something like this

[code]Use Acctfiles
Go
SET QUOTED_IDENTIFIER ON
GO
SET ANSI_NULLS ON
GO
if exists (select * from dbo.sysobjects where id = object_id(N'[dbo].[infCashBookSummary_Action]') and OBJECTPROPERTY(id, N'IsProcedure') = 1)
drop procedure [dbo].[infCashBookSummary_Action]
GO
Create Procedure infCashBookSummary_Action
@ctrlval1 varchar(100)
,@ctrlval2 varchar(100)
,@ctrlval3 varchar(100)
,@CloseDate varchar(100)
,@EndDate varchar(100)
,@CashType varchar(100)
,@Net varchar(100)
,@Fiscal_Year int
,@Date_to_Run datetime
as

Set NoCount On

If IsNull(@ctrlval3,'')='Delay Activity'
Begin
Update CashClose_T
Set CashAction=@ctrlval3
, CloseDate=DateAdd(mm,1,CashDate)
Where CCId=@ctrlval1
Select '["reload", "display"]'
Return
End

If IsNull(@ctrlval3,'')='Restore Activity'
Begin
Update CashClose_T
Set CashAction=''
, CloseDate=CashDate
Where CCId=@ctrlval1
Select '["reload", "display"]'
Return
End

If IsNull(@ctrlval3,'')='Delete Activity'
Begin
Delete From CashClose_T
Where CCId=@ctrlval1
Select '["reload", "display"]'
Return
End

If IsNull(@ctrlval3,'')='Add Activity'
Begin
If IsNull(@EndDate,'')=''
Begin
Select '["errormessage", "End Date cannot be blank"]'
Return
End
If IsDate(@EndDate)=0
Begin
Select '["errormessage", "End Date must be a valid date"]'
Return
End
If DatePart(yyyy,@Date_to_Run)<>DatePart(yyyy,@EndDate)
or DatePart(MM,@Date_to_Run)<>DatePart(MM,@EndDate)
Begin
Select '["errormessage", "Date is not in the correct month and year"]'
Return
End
If IsNull(@CashType,'')=''
Begin
Select '["errormessage", "Type cannot be blank"]'
Return
End
If IsNull(@Net,'')=''
Begin
Select '["errormessage", "Net cannot be blank - must enter a number"]'
Return
End
If IsNumeric(@Net)=0
Begin
Select '["errormessage", "Net must be a valid number"]'
Return
End
If Cast(@Net as money)=0
Begin
Select '["errormessage", "Net must be a non-zero entry"]'
Return
End
Insert into CashClose_T values (@Fiscal_Year, @CloseDate, @CloseDate, 'Y', '', 0, @EndDate, @CashType, @Net, null, null, null, 4, GetDate())
Select '["reload", "display"]'
Return
End

Select '["errormessage", "No Action with command: ' + IsN

we use Return 0 for success or Return 1 for fails at the end of our sprocs:

Code:

---

        end
end try

begin catch
        exec dbo.USP_RAISE_ERROR; -- <- custom sproc for handling errors
        return 1;
end catch


return 0;

---

then from code the return value parameter can be checked.

-tg

One thing I can suggest is to not have COMMIT / ROLLBACK statements in your stored procedures if the procedure is being called from a client application.
Let the client application control the transaction handling in its code.

Quote:

---

Originally Posted by abhijit

One thing I can suggest is to not have COMMIT / ROLLBACK statements in your stored procedures if the procedure is being called from a client application.
Let the client application control the transaction handling in its code.

---

I never hold a transaction open from the client side - I would recommend against that normally.

What benefit have you found in doing this?

I am always concerned about some client code hanging with an error message or something like that and holding that lock on the database being a bad thing.

Quote:

---

Originally Posted by szlamany

I never hold a transaction open from the client side - I would recommend against that normally.

What benefit have you found in doing this?

I am always concerned about some client code hanging with an error message or something like that and holding that lock on the database being a bad thing.

---

What if the client code has to fire several stored procedures?
You wouldn't want to commit in between.
It is easier to control the entire transaction through the client.

I never find myself firing sequential SPROC calls without WANTING them to each commit.

If I am saving a dozen grid rows to a table I want each row to commit.

If I need to have all of them be "inserted" at once I would load into a staging table and upon the success of that non-locking operation then call a single SPROC to process all those rows at once.

As much as you find it easier to control the transaction through the client I would argue as much for transaction control within the SPROC.

Different philosophies - both valid.

Like a lot of things, it depends... typically I try to keep transactions as small as possible... keep them in the db if I can... but there have been cases where it can only be done from the client. I used to work on a system where we were processing & updating data across multiple servers... so we'd spin up a distributed transaction from the client, perform the updates, then either commit or rollbakc.... actually where the transaction is created wasn't on the client,but from the app server as part of a services call... but still... it point was that hte transaction was outside the db. It really just depends on your needs.

-tg