7 ms·
How I destroyed the company's DB (a stupid SQL mistake)
- moribvndvs 3y agoI’ve done it, too. One quick thing I did to mitigate the problem going forward was write a little GUI SQL workbench tool that connected to the prod database. It wrapped every command in transactions and, as paranoid as I could make it, rolled back. It was the only thing connected to the prod database besides the applications, and the user related to it was disabled unless someone in IT got a request to temporarily unfreeze it. If you wanted to commit, or it found an INSERT, UPDATE, or DELETE in the command, it would ask for a password plus a nonce emailed to you that you needed to enter. Sure, I didn’t get the full IDE support of a proper workbench, but that had the side effect of making me write/test commands elsewhere. Actually, making it require two personal passwords like missile keys on a submarine would have been a cool idea. Anyways, we used it for about 11 years and it saved us a lot of trouble.
- frafra 3y agoIt remembers me when I pasted a multiline SQL query started with `begin;`, but psql ignored the first due to a connection timeout, it automatically reconnected to the server, and proceeded to execute the following lines...
- awestroke 3y agoSQL really sucks. DELETE without WHERE should be a syntax error. Use `where id=id` if you really want everything gone.
- paulryanrogers 3y agoCould probably be done with a DB statment-level trigger. It's sometimes useful to flush tables for testing or work tables. TRUNCATE is even a special case command for such purposes.
- crazygringo 3y agoBecause of this, it's also customary to use a LIMIT just in case. For something like this, LIMIT 3 if you assume the query is correct (e.g. in production), or LIMIT 4 if you're running it manually -- so that if it reports 4 rows changed instead of 3, you know you messed up somewhere, but at least you only have up to 4 rows to fix by hand, rather than an entire table.
- AntonZ234 3y agoI’m used to using limits purely in select queries, honestly I didnt event know its applicable for update queries. Still, it would probably have been after the where, and ignored too. not using transactions is probably my biggest mistake here.
- mlyle 3y agoOf course, the LIMIT is no good here because the correctly-written statement is truncated.
- cortesoft 3y agoWould not have helped, since the limit would have applied to the second statement in this case. The issue wasn’t the statement, it was the tool thinking blank line == end ststement.
- syslog 3y agoA decent database has transactions, so you can rollback your fat-fingered queries.
- cafard 3y agoPostgres and SQL Server have always had transactions; but unless you frame your DML in BEGIN TRANSACTION/[COMMIT|ROLLBACK], each statement is committed as executed. If you broke in on Oracle, where you must explicitly commit statements (unless your client sets up auto-commit), this can take some getting used to.
- crazygringo 3y agoYeah, I'd definitely be in favor of a flag that didn't allow DELETE without WHERE. Too easy to accidentally hit enter before you finish writing the query, in certain circumstances. Disable the flag if you want pure SQL. And just use "WHERE true" otherwise.
- maxbohun 3y agoIt shouldn’t be error, but it has to warn you..are u sure to execute this query?…(by default)
- awestroke 3y agoWhy not an error? How often do you truncate a table? Often enough that you can’t afford to type ` where true `?
- Karellen 3y agoBut... this query used UPDATE orders SET is_deleted = true DELETE without WHERE wasn't part of the problem.
- awestroke 3y agoGood idea, let’s enforce WHERE on UPDATE as well
- 1st1 3y ago> The program I used (DBeaver) saw the empty 3rd line, and ignored the 4th line. This is weird. SQL has ; as a command separator, why treat empty lines specially?
- crazygringo 3y agoYes this is extremely strange and has nothing to do with SQL. This is a bizarre client implementation. I use blank lines to separate sections of long queries all the time for readability and have never had a problem with any tool ever.
- jayknight 3y agodbeaver does have an preference for blank lines being statement separators, but that being the default is baffling. This breaks lot of sql scripts that we have in our codebases.
- hiccuphippo 3y agoJetBrains IDEs also allows blank lines as separators but shows an outline over the query it is going to execute, which helps avoid the problem.
- AntonZ234 3y agoI don’t know why the decided for a default like that. In Editor > Editor SQL > SQL Processing There is a setting called 'Blank line is a statement delimiter' which is on by default for some reason
- fbdab103 3y agoI have been burned by this DBeaver behavior before. Nothing so destructive, but yeah not great. For "more serious" commands, I try to force myself to highlight the command, but should really investigate if there is a way to enable more strict command selection.
- teraflop 3y agoLaziness/simplicity, I guess. You can't safely use semicolons as delimeters unless you parse/tokenize the entire statement (to exclude quoted semicolons). The exact handling of quoted string literals can vary depending on the SQL dialect, or even the configuration, and getting it wrong could cause even more unpredictable behavior. For instance, on MySQL I don't think it's possible to reliably identify statement boundaries without knowing the server-side value of the NO_BACKSLASH_ESCAPES setting.
- debarshri 3y agoThis is the reason why you should use a Privilege Access Management or Data Access governance tools. You won't be allowed to run these queries in production as is with you proper session review and access review. Accidental operations can be avoided with data access governance tools.
- paulryanrogers 3y agoAre such tools built into RDBMS's?
- michaelt 3y agoThis is more of a premium feature you make available as part of your $$$$ enterprise offering.
- deleted 3y ago[deleted]
- laeri 3y agoWhich did you use and what kind of experiences did you have with them?
- debarshri 3y agoI am cofounder of Adaptive (https://adaptive.live https://adaptive.live), do check us out.
- ErikAugust 3y agoUPDATE orders SET is_deleted = true I mean, you didn't really delete the orders... does anyone DELETE things now a days?
- deleted 3y ago[deleted]
- OscarCunningham 3y agoI can't find the original post now, but there's a good suggestion that booleans should usually be timestamps: UPDATE orders SET deletion_timestamp = CURRENT_TIMESTAMP() WHERE deletion_timestamp IS NULL
- AntonZ234 3y agoInteresting, I’ve never heard about that pattern, only is_deleted everywhere
- kamikaz1k 3y agoThis makes more and more sense the more I think about it
- dingnuts 3y agoI can vouch for this pattern from personal experience. From an auditing perspective also, it can be really helpful to know WHEN something was removed.
- adhamsalama 3y agoThat's a good idea.
- FredPret 3y agoI love this pattern
- geekodour 3y agothis seems neat!
- deleted 3y ago[deleted]
- mvdtnz 3y agoNot that it's relevant to the issue at hand, but I'm curious about what kind of setup leads to "corrupt orders in the DB" and in what kind of business the correct response to such a situation is to simply delete the offending rows. Surely those orders are there for a reason?
- AntonZ234 3y agoThe answer is actually a bit complex. I simplified in the article, the orders are in fact internal ‘missons’, that our operations team needed to execute (I work at an Agtech company, and each such mission is a drone flight above a field). Due to some bug, there were duplicate missions created, and it messed up the systems and confusing the operations team.
- mvdtnz 3y agoAh that makes sense, thanks for the follow up.
- laeri 3y agoThat can happen, I had this case with a local payment gateway. When a user started a checkout session we created a database object that tracked the specific checkout with an id the payment gateway handed back. When the user aborted the checkout the corresponding order within the gateway however was only kept for around 30 minutes and then deleted. This meant a checkout row in our database tried to associate an external order with an id for an object that didn't exist anymore. This lead to some errors when we tried to get data for these checkout items and got back that they weren't found.
- michaelt 3y agoImagine your customers want to reserve some of your limited capacity, before they know precisely what they want to use it for. So you allow them to "check out" empty placeholder orders, to reserve their place in the queue. They edit the order later to add the details - and they very rarely forget, these being some of your most eager customers. (Yes it's an ugly hack, but business types love getting features on the cheap) Do that enough times and eventually someone will forget to edit the order, and at the front of the queue you'll find an order for nothing. Which downstream systems will quite reasonably not accept, as it's obviously corrupt.
- SteveNuts 3y agoI appreciate that the author calls out the swiss cheese model https://en.m.wikipedia.org/wiki/Swiss_cheese_model# https://en.m.wikipedia.org/wiki/Swiss_cheese_model# I really think everyone in IT should be familiar with it and keep it in mind when designing systems.
- kureikain 3y agoJust a few weeks ago we had an auth service. For some reason our CBO cannot login, reset password email takes a long time to come(another story for other days). So I generated an argon2 hash, manually loggin to db to set it. I want to run: ``` update users set hash = 'hash' where email = 'cbo email'; ``` Unfortunately, the `;` character was used as part of the hash so during copy and paste the whole thing become: ``` update users set hash = 'hash'; where email = 'cbo email'; ``` It ran immediately and reset password for all of our users. I had to make a new db from the point in time recovery to copy password back. always use transaction and commit the result will be the way from now on.
- daitangio 3y agoAlso always __test__ your database updates doing a select FIRST, and checking the output of your where condition. I learned this in the hard way, after a small mistake (but I was still a junior developer).
- unshavedyak 3y agoI wish there was a select prefix that would turn any delete or update into a query. So that you could remove a single keyword to actually execute it, further reducing the chance of unforeseen typos
- AntonZ234 3y agoDamn. Glad to hear I’m not alone… transactions is indeed the way to go here. Scary moments
- hobs 3y agoHow would the semicolon also generate a single quote? Something is fishy in your story :)
- kbenson 3y agoTransactions are good/best, but I also have a habit of prefixing my statements with "a " before pasting anything with a semicolon (or anything more than a few characters really), so in the accidental case a newline gets pasted it's a syntax error. Working in databases is like an old adage I heard from an aged motorcycle cop. There are two types of motorcycle riders. Those that have been in an accident, and those that will be.
- wantlotsofcurry 3y agoI never understood why there is no way to have a `dry-run` option on SQL operations. I feel like that would save so much time and dev anxiety.
- deleted 3y ago[deleted]
- VoidWhisperer 3y agoAlso another thing worth bringing up - why didnt the program they used have something to flag delete queries without WHERE? If you use mysql workbench for example, you cant run an UPDATE or DELETE without either providing a WHERE clause or explicitly disabling the option preventing UPDATE/DELETE without WHERE
- rahkiin 3y agoJetBrains does this as well. It also visually indicates the query that will run by changing its bgcolor so weirdly placed semicolons become apparent. It also does not insert semicolons on empty lines… This article describes human error but it was not helped by their tools.
- fbdab103 3y agoI use DBeaver, and I know I have received messages of this type when I try to run mutations without a WHERE. Maybe the author had previously disabled this check? I also configure databases to be "Production" within DBeaver, which adds a few extra "Are you sure checks?" so that might be why I see the message.
- maweaver 3y agoI've had the "joy" of doing data corrections on production databases before. Things I've learned: - ALWAYS do the work inside of a transaction, as he mentioned. Rollback is your friend. For final run I always do a rollback and "clean" run just to make sure nothing extra slips in. - Start by crafting your delete/updates as SELECTS. Make sure it targets the data you want then save the SELECT to run afterwards. - Export any data you plan to modify off to a spreadsheet and save it somewhere. This is another chance to check it's what you intended to change, it's a record of what was changed, and it can be used to revert the data if needed. - As the author mentioned, get another pair of eyes if possible. Besides making it less likely for things to go wrong, people are more understanding. - And it probably goes without saying, but don't just fix the data and move on. Do everything possible to track down the bug that caused the issue in the first place and fix it. This is another place where having previous data available in a spreadsheet (or even a complete database backup) can help with data mining after the fact.
- hnbear 3y agoI've done similar things, I think most people working long enough have. I've made it a point in teams I've run to reassure the junior staff with tales of how all the seniors have made egregious mistakes and survived to tell the tale. There's a part of one of the Netflix DevOps books about how only the really senior, really important staff screw up the most. It generally takes access, ability, and doing challenging things that the most senior and impactful staff are trying to accomplish to end up in a position where a mistake will cause big damage. That said, yes, teams should also build and deploy resilient systems with well practiced back-ups and restores, and make that common place enough that everyone can restore quickly. I think in my case the closest to this was a very similar case - fixing a customer record in the customer DB. I'd written the sql quickly, was in a GUI SQL client, and far too fast selected the query from the many I was working on, and hit the shortcut to run it. Then realized a split second later I'd mis-selected just the UPDATE and SET statements, and missed the WHERE. Every customer was name called John Smith. Whoops. I know he called out that he's in a start-up, but in my case I was at a big financial. The other lesson I learnt is to make very good friends with the infra teams, and have sufficient mutual respect and support that you (as front-office) do as they ask, and go out of your way to help with whatever projects they have (eg. upgrades, patching, migrations), and at the same time, don't bother them will silly questions, and in return be clear that when you do need something and say it's urgent, that it's genuinely urgent and needs immediate attention so that DB restores get handled ASAP. Be a great customer to them, and they'll be great service providers back.
- linguick 3y agoRule #1: never do auto commit