QOD 6/10/2003
-
Not real happy with the choices on this QOD. The reason the developer is attempting to HOLDLOCK is so that the quantity does not change from underneath him. What he wants to do is ensure that he doesn't decrement the inventory count unless its greater than zero.
The BEST way to do that would be:
CREATE PROCEDURE
UpdateInventoryTable @InventoryID int
As
BEGIN
UPDATE Inventory
SET InventoryCount = InventoryCount - 1
WHERE InventoryID = @InventoryID
AND InventoryCount > 0
END
No tran (except implicit statement-level), no extra locks held.
If not rewriting the query in this way, to preserve the intent of the programmer, he should issue a (UPDLOCK) on the SELECT statement within the transaction to ensure no other processes decrement his InventoryCount to zero between the time of the SELECT and the UPDATE.
Neither of these choices were presented. The "correct" answer was correct in terms of efficiency, but not in preserving the integrity of the database. To see the point, imagine that two processes kicked off this procedure at the same instant. They did the select at the same time (allowed even with the HOLDLOCK on the SELECT, since its only a shared lock) and both got InventoryCount of 1. Then one proceeds to update the count to zero and end the transaction, THEN THE OTHER PROCEEDS TO UPDATE TO -1 and successfully commit the tran.
I rather wait for a right answer, thank you!
Please give me feedback and let me know if I missed something here. This is important to me as I've preached it to the SQL developers I work with for years.
-
I believe the question was what was causing the deadlock, which was indeed the HOLDLOCK hint. Your comments are indeed accurate, but did not address the question, which specifically asked what was causing the deadlock error. Of the answers given, there was only one really correct choice.
-
I completely agree with your statement. I also think they should have had better choices to pick from.
-
I agree with viacoboni.
The first choice would be to change the HOLDLOCK to and UPDLOCK.
Not to remove the lock completely.
Or rewrite the proc.
What's the business problem you're trying to solve? -
Also agreed with viacoboni. Sorry jpipes, but the question was "what is the best way to fix the problem". To me, getting around a deadlock error by making a change that potentially introduces data integrity problems is, one, not attempting to understand the root cause of the deadlock and, two, providing a bad example for newer SQL devs.
Heck, why not throw in a (nolock) hint while we're at it <g>
Off of soapbox now .. I'm taking these Q's wayyy too seriously 🙂
Randy
-
I also agree with viacoboni.
Removing the locking hint completely without also putting "where InventoryCount > 0" in the update statement will "fix the problem" but create another, more subtle, integrity problem. The "correct" answer would be poor practice because it will sometimes deliver an incorrect result.
I recommend to site administrators to revisit/re-score this question. As of this writing, more than 50% of responders gave the official "correct" answer which allows an integrity problem. Eventually they will get bit by this.
-
The people have spoken! Actually on further thought, I couldn't disagree with you on this one. I'll change the scoring of that one to 0 points and remove any hit to your overall score. It's nice it sparked some nice conversation though
Brian Knight
http://qa.sqlservercentral.com/columnists/bknight
Brian Knight
Free SQL Server Training Webinars -
I see lots of things are said now, but...I also had my bad moment when answering the cuestion, I also voted for the UPDLOCK, so, I also want to say that I agree with Viacoboni, and with Brian for changing the scoring 🙂
Julio
Julio -
Whatever you all say the correct and most appropriate answer is b which is to change to update lock.
This will keep data intergrity and solves the deadlock problem. If you implement the first one(a) you'll be in a dead position(without lock).
-
Aggree with Pitiyar and all you who say that changing HOLDLOCK to UPDLOCK would be the most appropriate measure.
-
Not to beat a dead horse but I just have to ask this question: what's wrong with taking the inventory count below 0? Here's a scenario I see:
20 salesmen selling widgets. Our stock is low but the truck's due "real soon now". So at some point a sale is made that takes our count < 0, say, e.g. to -10.
Truck comes in the next day with 1000 widgets. Now if our sales sp didn't let us decrement below 0, when the sp that increments inventory is run, it'll add the 1000 to 0 and our count will be too high (we really only have 990, since we sold 10 that we really didn't have at the time).
Is there a way you guys would work around this, or am I missing something here?
-
Allowing negative inventory is bad practice, particularly in ERP systems. The computer system should reflect actual ("real") data. It's impossible to have negative inventory; however, it is possible to have reservations against future inventory. I agree with the above comments that yesterday's QOD wasn't a very good question. Sure there was a deadlock, but the proposed solution wasnt' a very good one.
-
I agree the best answer would have been UPDLOCK, but that was not an option. Since B only specified a UPDATE locking hint, it was incorrect, since there is no such thing as a UPDATE locking hint.
Gregory Larsen, DBA
If you looking for SQL Server Examples check out my website at http://www.geocities.com/sqlserverexamples
Gregory A. Larsen, MVP
-
The question was: "Each time you execute the following stored procedure, which updates the inventory table, it issues a deadlock error. What is the best way to fix the problem?" As Greg says, UPDLOCK is the best way to fix problem. This question should be stricken since no alternative provided the correct answer.
-
ok, horse is now officially pulp. brian has already stricken the question.
Viewing 15 posts - 1 through 15 (of 18 total)
You must be logged in to reply to this topic. Login to reply