SELECT with UPDLOCK (SQL Server 2008)

[BruceG]

Hello,
We have a mulit-user application using SQL Server 2008 and VB 2010. There is a process in place to get the next record ID of our Order table, but it needs to be modified to ensure that two users are not getting the same value at the same time, which could obviously cause a primary key violation if the users are using that ID when they create new orders. I believe I know what to do to fix this, but I would appreciate confirmation from the DB gurus we have on this forum.

We keep the "LastOrderID" as a field in a one-record table, let's call it AppParms, and we essentially read that record to get the ID, add 1 to it, and update the table with that new ID.

Here is pseudocode for what is currently in place:
----------------------------------------------------------------------
Sql = "SELECT LastOrderID FROM AppParms"
' use reader object to execute the Sql and get the value of LastOrderID
TheNewID = LastOrderID + 1
' "TheNewID" is then used in the app to later ultimately insert the new order into the Order table
Sql = "UPDATE AppParms SET LastOrderID = " & TheNewID
' execute the SQL
----------------------------------------------------------------------

Here is what I believe I need to do to modify the above:
----------------------------------------------------------------------
' execute a "BeginTransaction" on the DB connection
Sql = "SELECT LastOrderID FROM AppParms (UPDLOCK)"
' use reader object to execute the Sql and get the value of LastOrderID
TheNewID = LastOrderID + 1
' "TheNewID" is then used in the app to later ultimately insert the new order into the Order table
Sql = "UPDATE AppParms SET LastOrderID = " & TheNewID
' execute the SQL
' execute a "Commit" on the transaction object
----------------------------------------------------------------------

Do you concur? If not, what would I need to do instead?

[BruceG]

Upon further research, I believe the HOLDLOCK hint should be used in conjunction with the UPDLOCK hint in the SELECT statement.
So, same theory as above, but modify the SELECT statement as follows:
Sql = "SELECT LastOrderID FROM AppParms WITH (UPDLOCK, HOLDLOCK)"

Still looking for confirmation on whether this the correct approach overall.
Thanks.

[TysonLPrice]

"Still looking for confirmation on whether this the correct approach overall"

In my opinion you should be using an identity column and not building your own primary key. Then you could do without the extra work.

[BruceG]

I understand what you are saying, but using an identity column is not an option in this case, and that is beyond my control. The example I posted is a simplified version, but there is more custom stuff going on in building the key. However, we still need the "Last ID + 1" piece; hence my post.

[FunkyDexter]

An identity column is not unneccessary. They're a handy shortcut but aren't applicable to all situations, e.g. compound keys.

Equally, I'm not a fan of transactions when they can be avoided, particularly if issued over the connection rather than on the server itself where it can be managed automatically. They've got an annoying habit of not being cleared properly and leaving resource locked on the DB. If you're 100% rigorous that shouldn't be a problem... but you won't be. You're human and sooner or later you'll forget to close one off. Or the tea lady will kick the plug out of the computer before it gets closed. Or some other problem you haven't considered yet.

A much better idea is to make the selection of the ID part of the update itself. That way the whole thing becomes nicely atomic with no risk. Try this:-

Update AppParms Set LastOrderID = Max(ID) + 1

Of course, you still faced with having to get the Order ID back and use it when you write to the orders table but there's quite a natty solution to that too: Don't hold it on the app parms table at all. Just use the max orderid from the orders table:-

Insert into 
Orders
(OrderID, Blah1, Blah2, Blah3)
Select
Max(OrderID) + 1, "Blah1", "Blah2", "Blah3"
From Orders

That way you will always use the next available orderid with no need to hold it and maintain anywhere else.

there is more custom stuff going on in building the key

That might invalidate my second suggestion but probably not. You just need to find a way of properly structuring the select bit.

[BruceG]

Hi Funky,
Thanks much for looking at this. I understand what you are saying about using the "+ 1" expression in the insert, but one thing that was not explicitly mentioned here is that I will need that new ID to populate other related tables - we have an Order Detail table, and also an Order Payment table (to handle multiple payment types). The point being, we need that new Order ID for the child tables. So I don't think we can get around the need for holding that new ID elsewhere (i.e. in a VB variable).

So, even given your reservation about transactions, would you concur that a reasonable solution in this case would be to use the UPDLOCK and HOLDLOCK hints on the SELECT statement, and place the SELECT and UPDATE statements within a transaction?

[FunkyDexter]

Ah, I feel your pain because I'm currently facing exactly the same scenario right at this moment.

What I'm doing is issuing a paramatised query but I'm making the sql string include the begin and commit transactions. Essentially a bit like putting a mini-sproc in a query. I'm still using the max value from the actual table (I can't think of a good reason not to) but I then select it straight back. I'm chucking it in a transaction because I don't want to get caught interleaving with another insert so I think some kind of transaction is inevitable here.

What I'm not doing, though is issuing connection.opentransaction (or whatever the method that opens a transaction on a connection is) because then I'm at risk of the vagaries of the tea lady. Basically, if the client app crashes I want to whole query, including the open and commit transaction to run or not. I don't want to open the transaction and then get a crash before I can close it.

Here's the c# code I've ended up with

public static DataRow InsertList(string Description)
        {
            string sql = "Begin Transaction;" 
                        + "Insert Into Lists_List (ID, Description) "
                        + "Select isnull(Max(ID), 0)+1, @Description From Lists_List;"
                        + "Select Max(ID) as NewID From Lists_List;"
                        + "Commit Transaction";
            SqlCommand cmd = new SqlCommand(sql, ConnectionsLibrary.MainDB);
            cmd.Parameters.AddWithValue("@Description", Description);
            SqlDataAdapter da = new SqlDataAdapter(cmd);
            DataTable dt = new DataTable();
            cmd.Connection.Open();
            da.Fill(dt);
            cmd.Connection.Close();
            return dt.Rows[0];
        }

I have to say, I'm not 100% sure this is the best way to handle it but I can't think of a better one. I'd personally prefer a method that gives a single sql statement but I'm not convinced one exists. If any of the other sql spods on the forum want to chuck in on this one I'll be as keen to hear from them as you are. This is working but I haven't had a chance to try it out in a high use or multi user set up yet.

[BruceG]

Interesting approach, sending the group of statements over like a mini-proc. I will remember that, as I believe it will come in handy. I may or may not be ablie to apply this to the current situation - I'm going to play around with it ...