Comment by leesalminen
8 years ago
Agreed. Several years ago as a junior dev I was tasked with adding a new feature- only allowing a user to have 1 active session.
So, we added a "roadblock" post auth with 2 actions- log out other sessions and log out this session.
Well, the db query for the first action (log out other sessions) was missing a where clause...a user_id!
Tickets started pouring in saying users were logged out and didn't know why. Luckily the on-call dev knew there was a recent release and was able to identify the missing where clause and added it within the hour.
The feature made it through code review, so the team acknowledged that everyone was at fault. Instead of being reprimanded, we decided to revamp our code review process.
I never made that kind of mistake again. To this day, I'm a little paranoid about update/delete queries.
We all make this mistake eventually, often in far more spectacular fashion. My lessons learned are
1) Always have a USE statement (or equivalent);
2) Always start UPDATE or DELETE queries by writing them as SELECT;
3) Get in the habit of writing the WHERE clause first;
4) If your SQL authoring environment supports the dangerous and seductive feature where you can select some text in the window and then run only that selected text — beware! and
5) While developing a query to manipulate real data, consider topping the whole thing with BEGIN TRANSACTION (or equivalent), with both COMMIT and ROLLBACK at the end, both commented out (this is the one case where I use the run-selected-area feature: after evaluating results, select either the COMMIT or the ROLLBACK, and run-selected).
Not all of these apply to writing queries that will live in an application, and I don't do all these things all the time — but I try to take this stance when approaching writing meaningful SQL.
#5 is the big one. I left WHERE off an UPDATE on "prod" data once. Fortunately it wasn't mission critical data that I ended up wiping and I was able to recover it from a backup DB. I never did anything in SQL without wrapping it in a transaction again.
I will note that depending on your DB settings and systems, if you leave the transaction "hanging" without rolling back or committing, it can lock up that table in the DB for certain accessors. This is only for settings with high levels of isolation such as SERIALIZABLE, however. I believe if you're set to READ_UNCOMMITTED (this is SQL Server), you can happily leave as many hanging transactions as you want.
> 2) Always start UPDATE or DELETE queries by writing them as SELECT;
On that point, I'd love a database or SQL spec extension that provided a 'dry-run' mode for UPDATE or DELETE and which would report how many rows would be impacted.
-- 123338817 rows will be affected
Oooops made an error!
> I'd love a database or SQL spec extension that provided a 'dry-run' mode for UPDATE or DELETE and which would report how many rows would be impacted.
I mean, if your DB supports transactions, you are in luck.
Start a transaction (that may be different among vendors - BEGIN TRANSACTION or SET AUTOCOMMIT=0 etc) and run your stuff.
If everything looks good, commit.
If you get OOOps number of records, just rollback.
MySQL Workbench won't let you run an UPDATE or DELETE without a WHERE by default.
Not sure why this isn't default in many tools. That's 99% for hazards.
2 replies →
We have to manually whitelist raw queries from active record that do full table scan so this also helps with mistakes like this.
You are not alone.
There is even a song in Spanish about forgetting to add a WHERE in a DELETE: https://www.youtube.com/watch?v=i_cVJgIz_Cs
This is amazing.
> Luckily the on-call dev knew there was a recent release and was able to identify the missing where clause and added it within the hour.
Raises questions about deployment. Didn't the on-call have a previous build they could rollback to? Customers shouldn't have been left with an issue while someone tried to hunt down the bug (which "luckily" they located), instead the first step should have been a rollback to a known good build and then the bug tracked down before a re-release (e.g. review all changesets in the latest).
Yup, agreed entirely. I'm a bit fuzzy on the details of that decision...he very well may have rolled back and then deployed a change later in the evening. It was fixed by the time I checked my email after dinner.
UPDATE cashouts SET amount=0.00 <Accidental ENTER>
Oops. That was missing a 'WHERE user_id=X'. Did not lose the client at the time (this was 15+ years ago), but that was a rough month. Haven't made that mistake again. It happens to all of us at some point though.
I'm beginning to think this is a flaw in SQL. It's so easy to bust the entire table.
Could've had something like 'UPDATE ALL' required for queries without filtering.
just run BEGIN TRANSACTION before
I'm guessing this feature was never tested properly
we all assume code (or feature) that are not tested should be assumed to be broken