cancel
Showing results for 
Search instead for 
Did you mean: 
Read only

Transact SQL Procedure BEGIN TRANSACTION / COMMIT TRANSACTION Question

34,914

I have a questions about an SQL procedure. I'm replacing some code in the front end application with an SQL Procedure to allow other languages to use the same logic, shocking behaviour, I know. The procedure checks for the existence of a record, if it exists it increments it and returns the incremented value or inserts a new record where necessary and returns "1". Anyway, we have a couple of hundred users all potentially accessing this code, so I want to be absolutely certain that it was going to work how it's supposed to. My question is this: on the comment marked "HERE", is there any possibility that another connection could alter that value before the SELECT sets the variable? Is the row locked by the BEGIN TRANSACTION command in a way that will avoid concurrency issues? From the documentation, I gather that it does but I wanted to ask a person/people.

Also, is there a better/faster way for this code to work? Any tips would be appreciated.

The second/inner BEGIN/COMMIT section is probably unnecessary but, I'm new to SQL Procedures so I'm covering all bases.

 DECLARE @IDValue BIGINT

    BEGIN TRANSACTION
      IF EXISTS(SELECT * FROM "DBA"."UniqueIds" WHERE "IdName" = @IdName)
        BEGIN
           BEGIN TRANSACTION
             UPDATE "DBA"."UniqueIds" SET "Id" = "Id" + 1 WHERE "IdName" = @IdName
             //--------HERE--------
             SELECT @IDValue = "ID" FROM "DBA"."UniqueIds" WHERE "IdName" = @IdName
           COMMIT TRANSACTION 
        END
      ELSE
        BEGIN
          INSERT INTO "DBA"."UniqueIds" ("IdName", "Id") VALUES(@IdName, 1)
          SET @IDValue = 1
      END

   COMMIT TRANSACTION
   RETURN @IdValue
View Entire Topic
VolkerBarth
Contributor

Just a few remarks:

As you are basically trying to build a primary key generator, there are several different approaches, and personally, I would recommend DEFAULT AUTOINCREMENTs (or SEQUENCES for v12 and above) over a separate "key value table" like you are using currently because the former should behave better with many concurrent transactions - confine this help topic that gives an overview:

That being said, I guess your construct is safe even on the lower default isolation levels (0 or 1, depending on the API used): SQL Anywhere prevents two concurrent transactions to modify the same row by using exclusive write locks - writers always block other writers on the same row. So in my understanding, when transaction A has reached the update statement, it will block all other transactions that are trying to update the same row until A has been committed (or rolled back). As the SELECT follows the UPDATE, it should be safe. - As I stated, that's a guess.

As to the nested transactions: Note that only the outermost COMMIT will make the changes to the database permanent - the inner COMMIT will only decrease the nesting level so it will have no influence in your sample (and note that the stored procedure may be called within an outer transaction itself, so even the COMMIT at the end of the SP's body might not make the changes permanent... And that may mean you will prevent other transactions from getting a new ID value for a potential long timespan, thus reducing concurrency (or even introducing deadlocks, in case transaction A (after it has got a new ID) would try to update a second row that has already been modified by transaction B before B tried to gather a new ID...).


It should also be mentioned that the database server can use "server-side autocommit", i.e. a mode in which each DML statement (even within stored procedures) is treated as a separate transaction... EDIT Correction based on Breck's answers: ...unless it uses explicit BEGIN TRANSACTION statements, as in your case. So that would not lead to problems with your current implementations. In that mode, the STP would not be safe.

Did anybody tell you transactions are easy?

0 Likes

Thanks, I use autoinc in all my new code. This was only to get round some legacy stuff.