7 ms·
Upsert anti-pattern in SQL Server
- mjevans 6y agoTitle possibly: "Stop writing upserts the wrong way (MS-SQL Server)" since it's less clear which flavor of SQL this is relevant?
- ksenzee 6y agoThe concept is applicable to relational databases in general, and it's a good idea. ETA: The point here is not the syntax, it's "don't leave race conditions sitting around in your upserts." Whatever RDBMS you're using, you need to make sure you're at the very least inside a transaction, and at best avoiding an unnecessary transaction.
- hobs 6y agoSQL Server is the name of the product, hence the inclusion in the title.
- bragh 6y agoIn a common forum that is not purely dedicated to Microsoft technology, MS product names really ought to be prefixed — context is important. For example, it applies to Entity Framework also: it has happened a few times to me that a non-MS developer assumed that ORMs in general were discussed, while it was related only to the MS implementation.
- deleted 6y ago[deleted]
- mumblemumble 6y agoThis is specific to Microsoft SQL Server, which has a rather fraught history in this department. I believe the best option in PostgreSQL is still INSERT ... ON CONFLICT ... In general, this is one of those spots where every single RDBMS behaves differently, so knowledge from one platform doesn't necessarily transfer well. (Which, in turn, is one of the reasons why it's not necessarily a good idea to try to be database-agnostic.)
- liability 6y agoSqlite has the same as Postgres; INSERT ... ON CONFLICT ... DO ... It's very ergonomic and intuitive. I understand why SQL Server needs to retain older forms, but why can't they also adopt these new forms that are more pleasant for the end user? I suppose the people buying licenses to SQL Server usually aren't the ones actually using it.
- mumblemumble 6y agoOfficially, MERGE is the method defined in the SQL standard, and is by far the most widely supported syntax. Everyone else uses different, DBMS-specific, non-portable syntaxes. A lot of them (e.g., PostgreSQL and Sqlite) have some similarities, but are still mutually incompatible. I think that Microsoft arguably made the right decision by choosing to conform to the standard. It's just kind of unfortunate that the ISO standard syntax is (in my personal opinion) not very good.
- liability 6y agoThe way I see it, the standard is should be taken as a bare minimum requirement. Ideally standards compliant SQL would work on any RDBMS, though I don't think that ideal has ever been realized. Supplementing the standard with extensions when the standard happens to have ergonomic problems seems to do more good than harm. IMHO writing SQL with the extensions of postgres or sqlite is a lot more enjoyable, which I think counts for a lot.
- 6y ago
- kardianos 6y agoTrue. Don't use MERGE in MS SQL Server. Use two separate statements.
- teh_klev 6y agoIt's better to back up assertions, even if you're correct: By the same author (who I highly respect): https://www.mssqltips.com/sqlservertip/3074/use-caution-with-sql-servers-merge-statement/ https://www.mssqltips.com/sqlservertip/3074/use-caution-with...
- TekMol 6y agoIt is such a common operation that I think MariaDB is doing the right thing by having a dedicated command for it, "INSERT ... ON DUPLICATE KEY UPDATE": https://mariadb.com/kb/en/insert-on-duplicate-key-update/ https://mariadb.com/kb/en/insert-on-duplicate-key-update/
- jakobdabo 6y agoAlso, REPLACE INTO...
- gweinberg 6y agoI've been burned with replace. The problem is that if you have fields with default values they will be set/reset to the default even if nothing changed. What I'd like to happen is that for any values that I don't supply, the table values remain unchanged.
- WesolyKubeczek 6y agoYou can kiss your ON DELETE CASCADE foreign-keyed data goodbye each time you use it.
- masklinn 6y agopostgres has insert … on conflict update since 9.5, and sqlite copied that.
- gfody 6y ago> I think everyone already knows my opinions about MERGE not everyone is stuck on a 15-year-old version of SQL Server. odds are you can go ahead and use modern language features. I can understand why a DBA consultant that frequently encounters production issues with old bugs would feel this way though.
- teh_klev 6y agoSQL Server didn't introduce MERGE until 2008, so not quite 15 years. Also SQL Server bugs can take a long time to be fixed, if at all: https://www.mssqltips.com/sqlservertip/3074/use-caution-with-sql-servers-merge-statement/ https://www.mssqltips.com/sqlservertip/3074/use-caution-with... In my own experience of working with MS SQL Server since 1998 (6.5), Microsoft are fairly conservative about changes to the core database engine, despite seeing new releases every year or so. New features tend to be stuff built on top of the engine. In real life you're far more likely to encounter older versions of SQL running mission critical apps because upgrades are that one time where you're likely to screw up the crown jewels (your data), and so therefore many organisations tend to be conservative about that too. Now it's nice to get to work on a greenfield application where you can kick off on a shiny new version of SQL, but there's a good chance those bugs and behaviours are still likely to be present because the core engine is pretty much the same.
- gfody 6y agowith all due respect to Aaron, I've had this link shown to me by developers not wanting to write a simple merge statement instead of some block of semantically weird shit full of lock hints enough times for it to be annoying. be careful, of course, but don't be superstitious - start with straight forward code and test.
- cogman10 6y agoThe problem with merge is that it fails under contention, not under simple testing. We've had multiple issues with SQL Server caused by the usage of MERGE statements. It's been bad enough that we've ended up banning them as an organization in favor of the block of "semantically weird shit". Why? Because that block has become common, clear, and most importantly, doesn't tend to cause weird deadlocks, race conditions, etc.
- bosswipe 6y agoIt's annoying that databases don't implement UPSERT natively. As far as I can tell the solution provided by every db requires the repetition of all the damn fields. To pick on the supposedly more ergonomic postgres method, here's a typical example, INSERT INTO students(id, firstname, lastname, gender, d_o_b, email) VALUES (1516, 'Gerard', 'Woodka', 'M', 'January 27 1995', 'gerardo_woodka@gmail.com') ON CONFLICT (id) DO UPDATE SET firstname = EXCLUDED.firstname, lastname = EXCLUDED.lastname, gender = EXCLUDED.gender, d_o_b = EXCLUDED.d_o_b, email = EXCLUDED.email; At least it doesn't force you to repeat the values like many other servers. But still the repetition is bug prone and hard to maintain as the table changes. And it's just annoying to type.
- nkozyra 6y agoI think you really want to repeat the fields because inserting versus updating could require dramatically different behavior.
- masklinn 6y agoThere are many cases where you only want to update a subset of the fields, or even not update anything, but it's also true that a shortcut to update everything but the conflicting fields would be nice.
- nkozyra 6y agoOh I get the convenience, it just seems like an easy way to shoot yourself in the foot, like an update without a limit clause.
- ufmace 6y agoI appreciate that it's a little annoying and repetitive, but I think in this case, it really is better, because the business logic for exactly what should be done with each value in the case of a conflict can vary so widely. It's good to have the full flexibility of SQL and all of the associated values to work with for the purpose of constructing the new value for the column.
- grugagag 6y agoI agree with this but this is a small nitpick compared to what kind of SQL queries Ive seen in production in different places I worked at. Not to mention the amount of redundant queries from ORMS performance hit which a lot of developers aren't even aware of.
- dathinab 6y agoAnd that is (part of) why RDB "failed" to keep people even for use-cases where they should be better then NoSQL. - To much hidden complexity which can leaf to bugs if you don't know about the hidden complexity. - To messy and/or misunderstood concurrency. - Many People having fundamental misunderstandings about how to do things and how far transactions are useful for given (often implicit default) options. One thing I had to learn the hard way is that many programmers which supposedly "know" about RDMS/SQL don't even understand isolation levels properly. Also is it just me or is this way easier in postgres (with on conflict ...).
- teh_klev 6y agoIt makes me weep when interviewing and I ask developers about fundamental stuff such as "what is ACID?"[0] None of this stuff is hard to learn, and I'm not exactly Mr Ace Developer. But time and again I've been asked to debug/fix data inconsistencies that were caused simply because the developer didn't know what a transaction was, whilst running multiple inserts and updates, and one of them failed along the way. [0]: https://en.wikipedia.org/wiki/ACID https://en.wikipedia.org/wiki/ACID
- ragnese 6y agoI don't know if it's worth a damn, but I have no idea what the acronym stands for. I'm sure I knew it at some point and I know it's a database compliance standard kinda thing. But I use the shit out of transactions. I rather have contention issues than non-atomic operations done to corrupt our data. When I get to Facebook scale I'll let you know what I think about eventually-consistent and NoSQL, etc. ;)
- dathinab 6y agoThe problem is people not even knowing that if you use the default isolation level (on pg, read-commited) that you can't do something like "read cell X, modify X in code (e.g. X+=1), write X back" without running into race conditions which you can either avoid by using a lock (e.g. a row lock) or a different isolation level. No need to know what ACID is an acronym for (I forget it all the time) or how the isolation levels are called, but you need to know what to look out for (mainly data dependencies between read an writes in the same transaction having race conditions you need to "team" and multiple read inconsistencies)
- Semaphor 6y agowhen I need to do some kind of upsert on SQL server, I tend to refer to this post: https://michaeljswart.com/2017/07/sql-server-upsert-patterns-and-antipatterns/ https://michaeljswart.com/2017/07/sql-server-upsert-patterns...
- mulmen 6y agoShouldn't the IF EXISTS be done after starting the transaction? What happens if the key is deleted before the transaction can start?
- tuatoru 6y agoI don't understand why INSERT ... ON CONFLICT ... DO is even a thing. Yesterday there was a item on a piece by rachelbythebay about checking for and dealing with unexpected conditions in your code as a practice that separated professionals from ... er, idiots. This would appear to be a prime example of that antipattern -- in general, not just in SQL Server. If a tuple snuck into your relation while you weren't looking, don't you need to rollback and find out the current state of things? edit: Rachel's piece: https://rachelbythebay.com/w/2020/09/02/check/ https://rachelbythebay.com/w/2020/09/02/check/
- manigandham 6y agoWhat do you mean by "you weren't looking"? The SQL statement which is doing what you tell it to do, when there's a conflict.
- tuatoru 6y agoI'm asking, why is there a conflict? Why don't you know whether or not that tuple is in the relation before you start? In OLTP, this practice (UPSERT) would cause all sorts of mayhem.
- Ancapistani 6y ago> Why don't you know whether or not that tuple is in the relation before you start? ... because that requires a redundant query, its associate lookup cost, and in order to be sure that tuple wasn't added or removed between the time your check and the insert/update executes, either a write lock on the table or on the row itself (if it exists). "Blind" inserts are the right way to go IMO.
- icedchai 6y agoIn practice, these conflicts can happen quite often during data synchronization. Say someone uploads a list of inventory, keyed by ITEM_ID. You want all your items to be either added (new) or modified (previously existing.) You could loop through them one-by-one, or you can just let the database handle it with a bulk upsert.
- deleted 6y ago[deleted]
- betimsl 6y agoYou also gain by executing one less transaction at the times of successful update. Quite a good pattern actually.
- adontz 6y agoActually for many rows I delete and insert. Much easier to write. DELETE FROM t WHERE id IN (SELECT id FROM new_data); INSERT INTO t SELECT id, other_field FROM new_data; It seems so natural to me that I do not have to repeat field list. Queries were mostly in context of ETL, so never considered performance issues. Never wrote such code for OLTP.
- shmoogy 6y agoThat's an interesting idea, I'll probably borrow this for ETL purposes.
- jimktrains2 6y agoThat'll update all ties though, including fields like created_at that you may not want updated.