salsita / node-pg-migrate

Node.js database migration management for PostgreSQL
https://salsita.github.io/node-pg-migrate
MIT License
1.27k stars 176 forks source link

createIndex `concurrently` by default or enforced #709

Open benjamine opened 3 years ago

benjamine commented 3 years ago

assuming this tool most common use case is for running zero-downtime migrations, would it make sense for concurrently true be the default option on .createIndex? not sure this can be easily introduced without a breaking change on this library, but it could prevent potentially catastrophic mistakes :)

maybe another option that could be even nicer, is to have an initialization configuration that ensures create index concurrently is enforced.

references:

dolezel commented 3 years ago

I do not think it should be a default option. But sort of configuration is a good idea. Taking it further, I can imagine it can sort of work like shorthands, so it is defined in the migration file and all following migrations take it as default. You would be able to change the defaults later and all would be stored in code. You would be able to define these defaults for all migration methods. e.g.

export const defaults = {
  createIndex: { concurrently: true, method: 'btree' },
  createTable: { ifNotExists: true },
}

I need to think about it...

teebot commented 3 years ago

It seems Postgres does not allow CONCURRENTLY in transactions

pgm.createIndex(
    "User",
    [
      {
        opclass: "gin_trgm_ops",
        name: "index_users_on_email_trigram"
      }
    ],
    {
      method: "gin",
      concurrently: true
    }
  );

gives

Rolling back attempted migration ... error: CREATE INDEX CONCURRENTLY cannot run inside a transaction block

benjamine commented 3 years ago

@teebot that's correct, I think tools like ruby one I mentioned will warn you so you always create an index in a separate migration step because of that.