How I destroyed the company's DB (a stupid SQL mistake) | Hacker News Reader

How I destroyed the company's DB (a stupid SQL mistake)

zaidesanton.substack.com

35 pointsAntonZ23471 comments

It 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...

> 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?

Yes 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.

dbeaver 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.

JetBrains IDEs also allows blank lines as separators but shows an outline over the query it is going to execute, which helps avoid the problem.

I 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

I 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.

If I saw this behavior in a tool I use I would honestly report it as a serious correctness bug. All sorts of things can produce spurious newlines and I’m like 90% sure the SQL standard does not permit them to be interpreted as delimiters.

Laziness/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.

I just checked in Postico (Mac) and got a syntax error when trying to execute a statement when the one a few lines above wasn't terminated.

Making newlines significant by default is... well, yeah. OP should beat themselves up a little bit less.

If you wrote `DELIMITER \n` in SQL (if it was allowed) it would be a total shitshow.

MS SQL Server Management Studio does the same but maybe it does so a bit more intelligently. I never had problems with it (but I'm always super careful when writing mutating queries) In any case, executing the following text

   select * from foo

   select * from bar

   update foo set val=1

will trigger to select statements and do an update.

Dbeaver has two ways to run queries through it's interface: single snippet, or entire query. Snippets are separated by two newlines. They likely used the single snippets command instead of entire query command.

I've actually had this happen in dbeaver. It has a feature (like some other db clients) where you can have a scratch SQL console.. you hit ctrl-enter to execute the statement the cursor is on, but it treats a newline as the end of a statement instead of continuing until it reaches a semicolon.

Just 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.

Damn. Glad to hear I’m not alone… transactions is indeed the way to go here.

Scary moments

Also 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).

I 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

How would the semicolon also generate a single quote? Something is fishy in your story :)

Transactions 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.

SQL really sucks. DELETE without WHERE should be a syntax error. Use `where id=id` if you really want everything gone.

Could 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.

Because 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.

Of course, the LIMIT is no good here because the correctly-written statement is truncated.

I’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.

Would 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.

A decent database has transactions, so you can rollback your fat-fingered queries.

Postgres 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.

Yeah, 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.

It shouldn’t be error, but it has to warn you..are u sure to execute this query?…(by default)

Why not an error? How often do you truncate a table? Often enough that you can’t afford to type ` where true `?

But... this query used

    UPDATE orders
    SET is_deleted = true

DELETE without WHERE wasn't part of the problem.

Good idea, let’s enforce WHERE on UPDATE as well

I'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.

I’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.

I'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.

I appreciate that the author calls out the 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.

Also 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

JetBrains 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.

I 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.

I 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.

Make all your API call have a dry-run parameter ?

So true

You could start a transaction?

ref: https://dba.stackexchange.com/questions/223425/how-to-test-q...

Not 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?

The 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.

Ah that makes sense, thanks for the follow up.

That 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.

Imagine 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.

This 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.

Are such tools built into RDBMS's?

This is more of a premium feature you make available as part of your $$$$ enterprise offering.

Which did you use and what kind of experiences did you have with them?

I am cofounder of Adaptive (https://adaptive.live), do check us out.

Rule #1: never do auto commit

UPDATE orders SET is_deleted = true

I mean, you didn't really delete the orders... does anyone DELETE things now a days?

I 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

Interesting, I’ve never heard about that pattern, only is_deleted everywhere

That's a good idea.

This makes more and more sense the more I think about it

I can vouch for this pattern from personal experience. From an auditing perspective also, it can be really helpful to know WHEN something was removed.

I love this pattern

this seems neat!

While that's kind of convenient in a lot of ways, it also makes querying the database really annoying, since you have to remember to add the filtering to every single query or you're screwed. Personally, I wish there was a database-implementation-acknowledged "deleted" flag, which could even expose a "history table"-like interface.

How does using a timestamp column make querying the database more annoying than using a boolean column? Aren't the places you have to use filters exactly the same either way?

I'm sorry, I wasn't being clear. Any column meaning "this row is to be ignored for most applications" is an annoying pattern.

Ah, now I see I responded to the wrong message.

Only for some tables. Depends on the usage.

Right, but as I had no way to understand which orders should be ‘hidden’ and which don’t, from the systems side it was the same as if I hard deleted everything.

> does anyone DELETE things now a days?

Yes. OLTPs work best when cold data is hard deleted from the hot instances. Backups and OLAP can keep things for longer, as needed. GDPR, right to be forgotten, and similar regulations may require deleting even scrubbed copies. (IANAL)

Some laws require a hard delete. But I agree, soft delete is much less risky

Even in a system where you don't completely delete deleted records, you may move them to a new table, eg thing_deleted in certain situations (this is a real pattern I've encountered with justified reasons where merely setting delete_at would be insufficient for business logic reasons), which tends to be an INSERT and a DELETE in a transaction

Reply on news.ycombinator.com