January 11, 2012 at 10:55 am
Jack Corbett (1/11/2012)
1. I haven't really used MERGE much so I don't think it of it, but it is probably the best solution in this case.
It has many fine features, including being automatically atomic, taking a lock on a non-existent row if required, handling NULL codes properly, doing an UPDATE only to a variable, performing the minimum reads and writes...
Jack Corbett (1/11/2012)
2. What's the race condition in 2?
A concurrent process could insert the same code after we have decided it's not in the table, but before we insert.
Jack Corbett (1/11/2012)
3. I would use the select case to set @ID at the end because I'd use it as an output parameter instead of returning a result set.
Yes that's just for the demo. BTW the output table is never written or read if the desired row exists.
Paul White
SQLPerformance.com
SQLkiwi blog
@SQL_Kiwi
January 11, 2012 at 10:57 am
Gianluca Sartori (1/11/2012)
@Paul: why HOLDLOCK? Is there a chance of intra-query non-repeatable reads in MERGE statements?
http://weblogs.sqlteam.com/dang/archive/2009/01/31/UPSERT-Race-Condition-With-MERGE.aspx
You broke my code! That doesn't allow me to insert a NULL code. That INTERSECT was there for a reason :w00t:
Paul White
SQLPerformance.com
SQLkiwi blog
@SQL_Kiwi
January 11, 2012 at 11:06 am
SQL Kiwi (1/11/2012)
Gianluca Sartori (1/11/2012)
@Paul: why HOLDLOCK? Is there a chance of intra-query non-repeatable reads in MERGE statements?http://weblogs.sqlteam.com/dang/archive/2009/01/31/UPSERT-Race-Condition-With-MERGE.aspx
You broke my code! That doesn't allow me to insert a NULL code. That INTERSECT was there for a reason :w00t:
That's a great read, thank you for the link!
Sorry for breaking your code. :blush:
AFAIK, HOLDLOCK means SERIALIZABLE: doesn't that cut concurrency at all?
-- Gianluca Sartori
January 11, 2012 at 11:08 am
SQL Kiwi (1/11/2012)
http://sqlblog.com/blogs/paul_white/archive/2011/06/22/undocumented-query-plans-equality-comparisons.aspx
Another great read.
I must have missed it when you first published it.
Thanks a lot!
-- Gianluca Sartori
January 11, 2012 at 11:11 am
Gianluca Sartori (1/11/2012)
AFAIK, HOLDLOCK means SERIALIZABLE: doesn't that cut concurrency at all?
Not really, it only applies to the one table, and there would ordinarily be an index on the CODE column to make the search a seek, so we lock either the non-existent value or the single value that does exist. Ok, technically perhaps a very small key range of values if the index on CODE is not unique. And thank you for the kind comment about the blog article.
Paul White
SQLPerformance.com
SQLkiwi blog
@SQL_Kiwi
January 11, 2012 at 11:13 am
Thanks Gail and Paul. I always learn something when y'all post.
Oh, and as far as inserting a NULL code, you shouldn't have NULL's:-P
Jack Corbett
Consultant - Straight Path Solutions
Check out these links on how to get faster and more accurate answers:
Forum Etiquette: How to post data/code on a forum to get the best help
Need an Answer? Actually, No ... You Need a Question
January 11, 2012 at 11:18 am
Jack Corbett (1/11/2012)
Oh, and as far as inserting a NULL code, you shouldn't have NULL's:-P
:laugh: But seriously, the example code did not specify NOT NULL as a constraint for that column, so in the default configuration, the column would be NULLable.
Paul White
SQLPerformance.com
SQLkiwi blog
@SQL_Kiwi
January 11, 2012 at 11:24 am
SQL Kiwi (1/11/2012)
Jack Corbett (1/11/2012)
Oh, and as far as inserting a NULL code, you shouldn't have NULL's:-P:laugh: But seriously, the example code did not specify NOT NULL as a constraint for that column, so in the default configuration, the column would be NULLable.
I saw that as well, but I had to play devil's advocate. I actually didn't know why you used INTERSECT at first, I actually assumed you had done some testing where for some reason it was performance trick, but the NULL problem definitely makes sense.
Jack Corbett
Consultant - Straight Path Solutions
Check out these links on how to get faster and more accurate answers:
Forum Etiquette: How to post data/code on a forum to get the best help
Need an Answer? Actually, No ... You Need a Question
January 12, 2012 at 1:49 am
Thanks for all your contributions. Race condition is a possibility where this proc is used, and so is @@ROWCOUNT being affected by triggers.
I think I will rationalise the proc using the following construct:
SELECT @ID = pKey FROM myTable WHERE CODE=@CODE;
IF (@ID IS NULL)
BEGIN
INSERT INTO myTable (CODE)
VALUES (@CODE) ;
SELECT @ID = pKey FROM myTable WHERE CODE=@CODE;
END
Mainly because its easy to understand by the other developers, and it will always get the right value in @ID, and will probably be more efficient than the OUTPUT or MERGE methods, given an index on CODE.
I've never come across use of OUTPUT in an INSERT statement in the code here, and MERGE is new and not something I'd expect them to understand easily.
January 12, 2012 at 6:37 am
If that is what you've concluded from the previous, you've not been paying attention.
You've just been explained how merge + output is faster than your piece of code and how merge + output does not have some of the potential serious flaws that your construct does have. I'd suggest to teach your developers some new tricks instead of cripling your code for their 'reading comfort'.
January 12, 2012 at 6:49 am
Tom Brown (1/12/2012)
...and will probably be more efficient than the OUTPUT or MERGE methods, given an index on CODE.
I do understand your practical reasons for preferring option 2 (though if we were to meet to discuss it in person, I would try very hard to change your mind). You should be aware, though, that the code you posted there is still quite vulnerable to a race condition. As Gail mentioned earlier, you would need a transaction and a higher isolation level to make it safe (there are other options too, but more complex). I would also counsel you to change the SELECT @ID = line:
DECLARE @myTable TABLE (pk int, code int)
DECLARE @ID int
SET @ID = -456789
SELECT @ID = pk FROM @myTable WHERE CODE=999
-- Not NULL!
SELECT @ID
-- Or SET
SELECT @ID = (SELECT pk FROM @myTable WHERE CODE=999)
-- NULL
SELECT @ID
Not a problem today, but it is a trap that remains open for someone to fall into one day (perhaps when using the same construction in another situation).
Paul White
SQLPerformance.com
SQLkiwi blog
@SQL_Kiwi
January 12, 2012 at 7:33 am
I already get grief here for writing 'newfangled clever stuff that no-one understands' (I obviously spend too much time reading SSC :-D). It would be a sisyphean struggle to get MERGE understood by all developers, and used correctly.
If I can remove the ROWCOUNT and IDENTITY issues, and come up with something they can easily understand, then its a win.
And some important missing data. CODE now has a unique index, It was one of those 'assumed' "things the application could never let duplicates happen". I have been guilty of the same assumption :blush: - and I now understand whey some of the above remarks were made.
January 12, 2012 at 8:10 am
Tom Brown (1/12/2012)
And some important missing data. CODE now has a unique index, It was one of those 'assumed' "things the application could never let duplicates happen". I have been guilty of the same assumption :blush: - and I now understand whey some of the above remarks were made.
Nevertheless, even with the unique index:
-- No row found
SELECT @ID = pKey FROM myTable WHERE CODE=@CODE;
IF (@ID IS NULL)
BEGIN
-- Unique key violation! Someone else inserted this code
-- after we ran the SELECT but before we arrived here :(
INSERT INTO myTable (CODE)
VALUES (@CODE) ;
SELECT @ID = pKey FROM myTable WHERE CODE=@CODE;
END
Better:
BEGIN TRANSACTION
SET @ID = (SELECT pKey FROM myTable WITH (REPEATABLEREAD) WHERE CODE=@CODE);
IF (@ID IS NULL)
BEGIN
INSERT INTO myTable (CODE)
VALUES (@CODE) ;
SET @ID = (SELECT pKey FROM myTable WHERE CODE=@CODE);
END
COMMIT TRANSACTION
Paul White
SQLPerformance.com
SQLkiwi blog
@SQL_Kiwi
January 12, 2012 at 8:22 am
Thanks Paul - now I get it. (its been a long day)
-- Must be very early over there - or are you not in NZ at the moment.
January 12, 2012 at 8:27 am
Tom Brown (1/12/2012)
-- Must be very early over there - or are you not in NZ at the moment.
It is 4:26am on Friday 🙂
Live in NZ, work remotely in the US, at the moment.
Paul White
SQLPerformance.com
SQLkiwi blog
@SQL_Kiwi
Viewing 15 posts - 16 through 29 (of 29 total)
You must be logged in to reply to this topic. Login to reply