5 ms·

We generally ban any IF NOT EXISTS in our team (we use MySQL) because it usually masks the real problem (why is the developer trying to create a table which alr

by kgeist 2y ago

We generally ban any IF NOT EXISTS in our team (we use MySQL) because it usually masks the real problem (why is the developer trying to create a table which already exists in the test environment? maybe there's a name conflict with another branch etc.)

shayonj 2y ago
exactly - i missed to talk about this in the post. I think using IF NOT EXISTS can be a sign of a higher degree issue as well. Being more mindful can help.
lisper 2y ago
> why is the developer trying to create a table which already exists Maybe idempotency was a design goal.
- kgeist 2y ago
  We have a special table which remembers which migrations were already run. So creating a second migration which tries to create the table again is a sign something is wrong. It's probably not what the developer intended and can result in wrong assumptions about the table's schema, for example (they expected their version would be used but the new migration is actually skipped).
  - arp242 2y ago
    In general I agree, but in MySQL/MariaDB this is quite tricky because you can't do a lot of stuff in transactions; definitely seen problems where someone does; -- migrate-2024-08-12-foo.sql create table [..] alter table [..] insert into migrations values ('migrate-2024-08-12-foo'); And the create works, but the alter fails. And then you try to run it again and now the create fails because you have half a migration. Herp derp. And you often can't split those two statements either: that would be "half a migration" that won't work with the code. As far as I know there isn't really a good solution for this. It can be quite a pain. In PostgreSQL and SQLite this is not an issue as you can run all the above in a transaction. You indeed rarely need "if not exists" there.
    - kgeist 2y ago
      For that exact reason, we have a rule that every DDL migration must be a separate migration in the framework. So if migration #2 out of 3 fails, only 1 migration will be marked as completed. If we see migration failed in CI/CD logs, it's immediately clear what exactly failed, and we can quickly revert what has managed to run (by applying the corresponding down migration), to return the DB to the previous valid state (if we decide to abort the release and fix the migrations first). Or we can rerun individual DDL migrations again, if it's a timeout issue. We design migrations in such a way so that new schemas/data never conflict with already running (old) code. And new code is activated only after all migrations completed successfully. So in practice half completed migrations is rarely an issue for us. But yeah, it requires more boilerplate. However, there's fewer surprises, compared to IF NOT EXISTS.
      - arp242 2y ago
        There's lots of cases where that doesn't really work, and can't logically work. An obvious example is: drop index old; create index new [..]; There's tons of examples like this where things can kind-of work with enough effort, but also not really – if the query takes 5 minutes without that index it's just going to timeout, so not that different from having broken code, and not much you can do about that too. You really want those two combined to be atomic. And in cases of "create table new; drop table old" using the old table is often 1) a major hassle, and 2) often doesn't actually work all that well as it's not a tested path. Either way, it seems MariaDB does support transactions for all of this if you disable "autocommit mode", judging from their docs? I haven't used MySQL/MariaDB in quite a few years, but I've never seen migrations for them done well without tons of hassle and it's a major reason I've typically recommended PostgreSQL. It's just eliminates an entire class of common headaches and errors.
        kgeist 2y ago
        For your index example, why not just do: create index new [..] drop index old; At every point, there's always an index to use. We usually have to split releases into several "subreleases": Release #1 (automatic migration step) - alter the table - add a new index which uses new columns (etc.) (code release step) - new code goes live and starts using the new index Release #2 (we call it "post release") - remove the old unused index Automation, CI/CD help here. Yeah, it's somewhat error prone, but you usually get used to it. Not saying it's perfect, but manageable. In any case, you can never make both SQL migration and code release atomic (if you use rolling updates with zero downtime) so you already have to split into such subreleases anyway.
  - 1a527dd5 2y ago
    We have a similar setup; but there are still scenarios where you need a IF NOT EXISTS. A classic example of a table on dev being close to empty, and a table on prod being huge and taking longer than the default timeout ~30 seconds. Depending on what the timeout is (connection timeout, transaction timeout etc) the DDL operation _may_ still be running. But the migration "process" has failed. So you end up having to slightly modify the attempted migration to account for the fact the thing already exists. It's an increasingly rare occurrence, probably a few times a year. Like anything the more we do it, the better we get at pulling left and preventing issues in the first place.
    - sgarland 2y ago
      SET statement_timeout = '1h'; CREATE INDEX... Or if you have a DDL user (which is smart; your app user should not be allowed to execute DDL), you can use `ALTER ROLE` for them to have whatever defaults you'd like. MySQL has a similar variable, `max_execution_time`.
      - 1a527dd5 2y ago
        Yeah we are aware of statement_timeout. But it's not something we want to actively promote. Most people copy and paste a previous migration and then amend it to their previous needs. So if the previous migration set the statement timeout to 1h, and the new migration is doing something we want to stop/prohibit like ADD COLUMN DEFAULT (which takes a lock) or SET NOT NULL on a huge table (which again can cause a lock). We want to prevent locking and prefer the migration to fail earlier and hit the default statement timeout. At which point a conversation occurs how best to do the DDL without effecting prod. In terms of DDL DML users; we have it split into three- DDL (Migrations) DML (Data only, e.g. correcting data) AppTier user (which is DML + a few more things)
        sgarland 2y ago
        Not trying to be snarky here (and this is by no means unique to your situation) but it sounds like what you have is a knowledge issue more than anything. Don’t apply things in prod you don’t understand. EDIT: by “you,” I mean people copy/pasting migrations and applying them, if that wasn’t clear. The rise of RDS et al. has utterly killed the majority of people’s knowledge of RDBMS, if they had it to start with. It’s a shame. Also a solid and enjoyable career for me, so I suppose it’s not all bad.
stubish 2y ago
I understand, and the same with PostgreSQL. The only time you might need IF NOT EXISTS is when you are juggling multiple DB update scripts from different development branches and want to avoid dependencies (and probably creating a new problem). EDIT: Also, accounting for indexes or other patches applied live on production when firefighting. The update also needs to happen in dev and staging environments, but not explode when the update gets run on production. What terrifies me though is the CONCURRENTLY, which indicates automated DB migrations doing stuff outside of transaction boundaries. This stuff should be rare, to the point it is applied manually under supervision. Any step outside the transaction boundary can fail and needs a recovery plan. And you can't automate it because part of the recovery plan might be to fix the networking glitch to see if your statement failed, succeeded but failed to report success, or is ongoing with an uncertain future. And if you have multiple migration steps, that is multiple intermediate database states that you probably haven't tested your app against (say, timeouts because you dropped a necessary index before creating a new one, hopefully one that doesn't take several hours to build).

by kgeist 2y ago

shayonj 2y ago

exactly - i missed to talk about this in the post. I think using IF NOT EXISTS can be a sign of a higher degree issue as well. Being more mindful can help.

lisper 2y ago

> why is the developer trying to create a table which already exists Maybe idempotency was a design goal.

kgeist 2y ago
We have a special table which remembers which migrations were already run. So creating a second migration which tries to create the table again is a sign something is wrong. It's probably not what the developer intended and can result in wrong assumptions about the table's schema, for example (they expected their version would be used but the new migration is actually skipped).
- arp242 2y ago
  In general I agree, but in MySQL/MariaDB this is quite tricky because you can't do a lot of stuff in transactions; definitely seen problems where someone does; -- migrate-2024-08-12-foo.sql create table [..] alter table [..] insert into migrations values ('migrate-2024-08-12-foo'); And the create works, but the alter fails. And then you try to run it again and now the create fails because you have half a migration. Herp derp. And you often can't split those two statements either: that would be "half a migration" that won't work with the code. As far as I know there isn't really a good solution for this. It can be quite a pain. In PostgreSQL and SQLite this is not an issue as you can run all the above in a transaction. You indeed rarely need "if not exists" there.
  - kgeist 2y ago
    For that exact reason, we have a rule that every DDL migration must be a separate migration in the framework. So if migration #2 out of 3 fails, only 1 migration will be marked as completed. If we see migration failed in CI/CD logs, it's immediately clear what exactly failed, and we can quickly revert what has managed to run (by applying the corresponding down migration), to return the DB to the previous valid state (if we decide to abort the release and fix the migrations first). Or we can rerun individual DDL migrations again, if it's a timeout issue. We design migrations in such a way so that new schemas/data never conflict with already running (old) code. And new code is activated only after all migrations completed successfully. So in practice half completed migrations is rarely an issue for us. But yeah, it requires more boilerplate. However, there's fewer surprises, compared to IF NOT EXISTS.
    - arp242 2y ago
      There's lots of cases where that doesn't really work, and can't logically work. An obvious example is: drop index old; create index new [..]; There's tons of examples like this where things can kind-of work with enough effort, but also not really – if the query takes 5 minutes without that index it's just going to timeout, so not that different from having broken code, and not much you can do about that too. You really want those two combined to be atomic. And in cases of "create table new; drop table old" using the old table is often 1) a major hassle, and 2) often doesn't actually work all that well as it's not a tested path. Either way, it seems MariaDB does support transactions for all of this if you disable "autocommit mode", judging from their docs? I haven't used MySQL/MariaDB in quite a few years, but I've never seen migrations for them done well without tons of hassle and it's a major reason I've typically recommended PostgreSQL. It's just eliminates an entire class of common headaches and errors.
      - kgeist 2y ago
        For your index example, why not just do: create index new [..] drop index old; At every point, there's always an index to use. We usually have to split releases into several "subreleases": Release #1 (automatic migration step) - alter the table - add a new index which uses new columns (etc.) (code release step) - new code goes live and starts using the new index Release #2 (we call it "post release") - remove the old unused index Automation, CI/CD help here. Yeah, it's somewhat error prone, but you usually get used to it. Not saying it's perfect, but manageable. In any case, you can never make both SQL migration and code release atomic (if you use rolling updates with zero downtime) so you already have to split into such subreleases anyway.
- 1a527dd5 2y ago
  We have a similar setup; but there are still scenarios where you need a IF NOT EXISTS. A classic example of a table on dev being close to empty, and a table on prod being huge and taking longer than the default timeout ~30 seconds. Depending on what the timeout is (connection timeout, transaction timeout etc) the DDL operation _may_ still be running. But the migration "process" has failed. So you end up having to slightly modify the attempted migration to account for the fact the thing already exists. It's an increasingly rare occurrence, probably a few times a year. Like anything the more we do it, the better we get at pulling left and preventing issues in the first place.
  - sgarland 2y ago
    SET statement_timeout = '1h'; CREATE INDEX... Or if you have a DDL user (which is smart; your app user should not be allowed to execute DDL), you can use `ALTER ROLE` for them to have whatever defaults you'd like. MySQL has a similar variable, `max_execution_time`.
    - 1a527dd5 2y ago
      Yeah we are aware of statement_timeout. But it's not something we want to actively promote. Most people copy and paste a previous migration and then amend it to their previous needs. So if the previous migration set the statement timeout to 1h, and the new migration is doing something we want to stop/prohibit like ADD COLUMN DEFAULT (which takes a lock) or SET NOT NULL on a huge table (which again can cause a lock). We want to prevent locking and prefer the migration to fail earlier and hit the default statement timeout. At which point a conversation occurs how best to do the DDL without effecting prod. In terms of DDL DML users; we have it split into three- DDL (Migrations) DML (Data only, e.g. correcting data) AppTier user (which is DML + a few more things)
      - sgarland 2y ago
        Not trying to be snarky here (and this is by no means unique to your situation) but it sounds like what you have is a knowledge issue more than anything. Don’t apply things in prod you don’t understand. EDIT: by “you,” I mean people copy/pasting migrations and applying them, if that wasn’t clear. The rise of RDS et al. has utterly killed the majority of people’s knowledge of RDBMS, if they had it to start with. It’s a shame. Also a solid and enjoyable career for me, so I suppose it’s not all bad.

stubish 2y ago

I understand, and the same with PostgreSQL. The only time you might need IF NOT EXISTS is when you are juggling multiple DB update scripts from different development branches and want to avoid dependencies (and probably creating a new problem). EDIT: Also, accounting for indexes or other patches applied live on production when firefighting. The update also needs to happen in dev and staging environments, but not explode when the update gets run on production. What terrifies me though is the CONCURRENTLY, which indicates automated DB migrations doing stuff outside of transaction boundaries. This stuff should be rare, to the point it is applied manually under supervision. Any step outside the transaction boundary can fail and needs a recovery plan. And you can't automate it because part of the recovery plan might be to fix the networking glitch to see if your statement failed, succeeded but failed to report success, or is ongoing with an uncertain future. And if you have multiple migration steps, that is multiple intermediate database states that you probably haven't tested your app against (say, timeouts because you dropped a necessary index before creating a new one, hopefully one that doesn't take several hours to build).