4 ms·

> ignore_row_on_dupkey_index I was thinking of the hints you used to have to use with a merge statement in order to get an atomic upsert - holdlock in sql serv

by gfody 4y ago

> ignore_row_on_dupkey_index I was thinking of the hints you used to have to use with a merge statement in order to get an atomic upsert - holdlock in sql server and lock table in oracle. My recollection of the terribly old newsgroup thread where it was (presumably you) said merge doesn't perform atomic upserts without these hints and since people were primarily wanting merge to perform atomic upserts and postgres doesn't support hints it made more sense to just make something for upserts hence the on conflict update stuff. Do I have it wrong?

petergeoghegan 4y ago
The Oracle docs introduce MERGE as follows: "Use the MERGE statement to select rows from one or more sources for update or insertion into a table or view. You can specify conditions to determine whether to update or insert into the target table or view. This statement is a convenient way to combine multiple operations. It lets you avoid multiple INSERT, UPDATE, and DELETE DML statements." MERGE doesn't claim to work as an atomic statement, and doesn't work as an atomic statement in practice. In other words you can get a unique violation error where a "true upsert" would update the row instead. You can work around this "limitation" in MERGE by retrying the statement until it succeeds, but at that point you might as well use a regular INSERT instead. The syntax for INSERT .. ON CONFLICT constrains the problem in certain ways, which is quite deliberate. It's a pretty faithful representation of what's really going on. Hiding that creates a lot of subtle but real problems.
- gfody 4y ago
  the docs also say "every statement is atomic" and the ambiguity around whether merge is one statement or two (or three) ought to be settled by explicitly stating our atomicity expectations with a transaction. anyway my experience has been that adding the lock hints "fixes" the problem with pk violations during concurrent upserts and it seemed to be commonly known and widely used afaict. fwiw I agree with not adopting "merge w/hints" as the syntax for upsert I just thought the decision had more to do with the aversion to hints but I stand corrected (also from reading some of the links here I see that even merge w/hints has a potential deadlock if you get unlucky with the range locks it uses under the hood - I never hit that in practice myself, but it seems clear that sql should have a standard more constrained upsert so that further supports your original decision)
  - petergeoghegan 4y ago
    > "every statement is atomic" Statement-level atomicity doesn't mean that you cannot get race conditions like the one that I described - it just means that the statement succeeds or fails as a whole. It's quite clear that you can get those race conditions in both Oracle and SQL Server's MERGE (same with the new Postgres implementation). Last I checked neither SQL Server nor Oracle address the fact that MERGE pretty much doesn't do what many of its users imagine it can do. (There is a narrow theoretical sense in which it does meet expectations provided serializable isolation level is used, but in practice that isn't worth much to users.) > anyway my experience has been that adding the lock hints "fixes" the problem with pk violations during concurrent upserts and it seemed to be commonly known and widely used afaict That's not my impression: https://www.mssqltips.com/sqlservertip/3074/use-caution-with-sql-servers-merge-statement/ https://www.mssqltips.com/sqlservertip/3074/use-caution-with...
    - gfody 4y ago
      there it is, if I had a nickel for every time that link has been thrown at me.. twice in this thread alone ;) we are now in agree-to-disagree territory because my experience has been different and I don't like to leave performance on the table no matter what the consultants say (I suspect consultants are overexposed to the kind of code written by teams who ultimately need a consultant to save them - that has never been my team)
dspillett 4y ago
> use with a merge statement in order to get an atomic upsert - holdlock in sql server Because of all the potential edge cases that MERGE didn't handle properly when introduced to SQL Server (2008?), I've avoided its use. Despite several revisions over the years there are apparently still significant behaviours that I'd consider bugs¹, some of which have been closed as WONTFIX² so are a fact of life you just have to accept if you use MERGE in SQL Server. [1] see https://sqlsunday.com/2021/05/04/how-merge-can-deadlock-you/ https://sqlsunday.com/2021/05/04/how-merge-can-deadlock-you/ & https://michaeljswart.com/2021/08/what-to-avoid-if-you-want-to-use-merge/ https://michaeljswart.com/2021/08/what-to-avoid-if-you-want-... and many other similar articles [2] https://www.mssqltips.com/sqlservertip/3074/use-caution-with-sql-servers-merge-statement/ https://www.mssqltips.com/sqlservertip/3074/use-caution-with...
- gfody 4y ago
  I'm in an agree-to-disagree state with various sql server consultants on this matter. I disagree that merge is harmful and should be avoided. Using merge w/hints for upserts has a decent performance advantage over the alternatives and comes with some theoretical risk of deadlocks that depend on implementation details of your use case and/or the shape of your data (I've yet to encounter it in practice, I tend to batch upserts and limit concurrency and suspect the folks having trouble do the opposite)

by gfody 4y ago

petergeoghegan 4y ago

The Oracle docs introduce MERGE as follows: "Use the MERGE statement to select rows from one or more sources for update or insertion into a table or view. You can specify conditions to determine whether to update or insert into the target table or view. This statement is a convenient way to combine multiple operations. It lets you avoid multiple INSERT, UPDATE, and DELETE DML statements." MERGE doesn't claim to work as an atomic statement, and doesn't work as an atomic statement in practice. In other words you can get a unique violation error where a "true upsert" would update the row instead. You can work around this "limitation" in MERGE by retrying the statement until it succeeds, but at that point you might as well use a regular INSERT instead. The syntax for INSERT .. ON CONFLICT constrains the problem in certain ways, which is quite deliberate. It's a pretty faithful representation of what's really going on. Hiding that creates a lot of subtle but real problems.

gfody 4y ago
the docs also say "every statement is atomic" and the ambiguity around whether merge is one statement or two (or three) ought to be settled by explicitly stating our atomicity expectations with a transaction. anyway my experience has been that adding the lock hints "fixes" the problem with pk violations during concurrent upserts and it seemed to be commonly known and widely used afaict. fwiw I agree with not adopting "merge w/hints" as the syntax for upsert I just thought the decision had more to do with the aversion to hints but I stand corrected (also from reading some of the links here I see that even merge w/hints has a potential deadlock if you get unlucky with the range locks it uses under the hood - I never hit that in practice myself, but it seems clear that sql should have a standard more constrained upsert so that further supports your original decision)
- petergeoghegan 4y ago
  > "every statement is atomic" Statement-level atomicity doesn't mean that you cannot get race conditions like the one that I described - it just means that the statement succeeds or fails as a whole. It's quite clear that you can get those race conditions in both Oracle and SQL Server's MERGE (same with the new Postgres implementation). Last I checked neither SQL Server nor Oracle address the fact that MERGE pretty much doesn't do what many of its users imagine it can do. (There is a narrow theoretical sense in which it does meet expectations provided serializable isolation level is used, but in practice that isn't worth much to users.) > anyway my experience has been that adding the lock hints "fixes" the problem with pk violations during concurrent upserts and it seemed to be commonly known and widely used afaict That's not my impression: https://www.mssqltips.com/sqlservertip/3074/use-caution-with-sql-servers-merge-statement/ https://www.mssqltips.com/sqlservertip/3074/use-caution-with...
  - gfody 4y ago
    there it is, if I had a nickel for every time that link has been thrown at me.. twice in this thread alone ;) we are now in agree-to-disagree territory because my experience has been different and I don't like to leave performance on the table no matter what the consultants say (I suspect consultants are overexposed to the kind of code written by teams who ultimately need a consultant to save them - that has never been my team)

dspillett 4y ago

> use with a merge statement in order to get an atomic upsert - holdlock in sql server Because of all the potential edge cases that MERGE didn't handle properly when introduced to SQL Server (2008?), I've avoided its use. Despite several revisions over the years there are apparently still significant behaviours that I'd consider bugs¹, some of which have been closed as WONTFIX² so are a fact of life you just have to accept if you use MERGE in SQL Server. [1] see https://sqlsunday.com/2021/05/04/how-merge-can-deadlock-you/ https://sqlsunday.com/2021/05/04/how-merge-can-deadlock-you/ & https://michaeljswart.com/2021/08/what-to-avoid-if-you-want-to-use-merge/ https://michaeljswart.com/2021/08/what-to-avoid-if-you-want-... and many other similar articles [2] https://www.mssqltips.com/sqlservertip/3074/use-caution-with-sql-servers-merge-statement/ https://www.mssqltips.com/sqlservertip/3074/use-caution-with...

gfody 4y ago
I'm in an agree-to-disagree state with various sql server consultants on this matter. I disagree that merge is harmful and should be avoided. Using merge w/hints for upserts has a decent performance advantage over the alternatives and comes with some theoretical risk of deadlocks that depend on implementation details of your use case and/or the shape of your data (I've yet to encounter it in practice, I tend to batch upserts and limit concurrency and suspect the folks having trouble do the opposite)