Do I really need a BEGIN TRAN and COMMIT in a SPROC

[szlamany]

Ok - got this SPROC

Create Procedure awc_PartGeneral_Add @Part varchar(100), @username varchar(100)--@SQLGuid varchar(36)
As
Set NoCount On
Declare @ND datetime
Declare @Seq int
Declare @NoteKey varchar(100)
Declare @SK varchar(100)
Set @ND=Convert(varchar(10),GetDate(), 101)
Set @Seq=IsNull((Select Max(Seq) From PartNotes_T Where PartId=@Part and NoteDate=@ND),0)+1
Insert into PartNotes_T values (@Part			-- PartId
			,@ND			-- NoteDate
			,@Seq			-- Seq
			,'N'			-- Face
			,15			-- TimeSpent
			,@username		-- AddedBy
			,''			-- ContactType
			,'OtherPhone'		-- MetWhere
			,''			-- MetWith
			,''			-- Note
			,GetDate()		-- TDate
			,'N'			-- Conservator
			)
Set @NoteKey=Scope_Identity()
Set @SK='~save~'+@NoteKey
Select	 '["save","add","undo"]'		"buttons"
	,'{"focus": "NoteDate"}'		"awcoptions"
Exec awc_PartGeneral_Edit @Part,@SK

and as you see I have no BEGIN TRAN and COMMIT...

I kind of always thought a SPROC was automatically contained in a TRANSACTION.

But it seems that two different users were maintaining the same "participant" and adding a note - and the SEQ number passed back to the UI was the "same" for both users.

I can only think that the Set @Seq=... is grabbing the same number because the INSERT had not happened yet - and that's only possible if the each SPROC line is not in a transaction.

Part of me does not want to believe the user - but they are saying it happened although not with enough technical info to be really firm.

I'm not sure how to even try to replicate the problem so I can see it really happen and then prove I fixed it...

[techgnome]

wow... can they do it again?
There is a "transaction" but it's an implied transaction and "commits" as the line runs... it's weird... but the point is that what you're seeing is possible but happens rarely... because the offsetting time for the calls would have to be off by microseconds... while one is inserting (which goes into the log), the other is reading the max(seq) (which is reading from the table)... this might be one of those crazy cases where a With (NoLock) hint on the read would help. that would allow it to ready from the dirty table which would include impending changes from the log as well. OR... wrap it in a transaction.

Something else that could cause it... depending on what clustered indexes you have on the table, the insert could cause a blip in the writing of the record, which would give someone else a small window to read the old data before the new one is inserted. I've seen that happen a few times...

-tg

[szlamany]

Do you really know for sure it's an implied-line-by-line transaction? That is really bad news for me to discover this far into my SPROC creating career!!

The PartNotes_T table is huge - and the SPROC is being called by a web method - so it's local to the SQL server and the users are in a browser - so the web methods calls could fire quickly in that regard, I guess...

Brings up a second point - I guess ASP.Net can run multiple threads for the same web method concurrently - something else I want to look further into...

[TysonLPrice]

You could tighten up the set with:

with (updlock)

Here is an example from something set up here.

set @pkClaim = isnull((select max(pkClaim) from Claim with (updlock)),0) + 1

[szlamany]

From what I just read at SqlServerCentral - the UPDLOCK is released immediately aftere the SELECT.

My problem is the following INSERT actually "consumes" said sequence number - so another user SELECT'ing the latest sequence number should never get an "already used value".

It was in that short window that I believe the two users got the same value.

I generally never use HINTS - am I not utilizing a powerful feature of SQL? I always thought that you let the sql engine manage locks and such as it sees fit...

[techgnome]

I always thought that you let the sql engine manage locks and such as it sees fit...

Generally speaking... yes... occasionally though, you have to help it along...

-tg

[abhijit]

Is the ASP .NET application controlling the transaction?

We are currently building an ASP .NET application and one of the rules that the architect of the application has put in is that all transactions need to be handled by the Data Access Layer classes and not the stored procedure.

Is it possible to get that column changed to an identity type column?

[techgnome]

Is it possible to get that column changed to an identity type column?

I wondered about that too... but when I looked at the SQL a little harder, I saw that it's a sequence number based on the client... so an Identity won't work.

-tg

[TysonLPrice]

I wondered about that too... but when I looked at the SQL a little harder, I saw that it's a sequence number based on the client... so an Identity won't work.

-tg

Could some kind of constraint be added to make a duplicate fail?

[szlamany]

There is already an identity column. This is a seq # to keep notes in order for the "date" for a particular client. Probably not really needed - but it's how the legacy data came along for this table.

I believe if I put a BEGIN TRAN and COMMIT at the top/bottom of the sproc it will be all fixed - don't you all feel that way also?

As for the DAL - I have always kind of left that layer in the SPROCS - I find that it's a robust enough "coding language" to handle all the data needs. My web methods simply push data into a SPROC for processing and get results back out. It's all ajax/jquery - I'm not using ASP.Net in the traditional sense. My page is static - perpetual - it does not re-load pages and such...