Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot run skipped migration without rolling back #164

Open
reffu42 opened this issue Aug 8, 2023 · 2 comments
Open

Cannot run skipped migration without rolling back #164

reffu42 opened this issue Aug 8, 2023 · 2 comments

Comments

@reffu42
Copy link

reffu42 commented Aug 8, 2023

Describe the bug
It doesn't seem like there's a way to force postgrator to run a script that has been skipped/missed without first rolling back (assuming you have an undo for every script in between) and then re-running.

To Reproduce

  1. Create a handful of scripts that make small changes to the database.
  2. Number them sequentially
  3. Skip a number and create one more (e.g. 001, 002, 003, 005)
  4. Run Postgrator
  5. Create a script for the skipped number (004)
  6. Run Postgrator again
  7. Note that because the script is less than the current "version" it does not run or get added to the schema_versions table

Expected behavior
Since the script has not yet been run and is not in the schema_versions table, it should be run (or at least make a command/option to "catch up" and run any missed scripts/numbers)

Additional context
This is useful in cases where you have a shared repository and multiple developers. It is also necessary if you have a "dev" and "prod" setup and need to commit a quick fix to the "prod" version while working on the "dev" version (e.g. script 039 is run in dev as part of a feature, but a bug in prod requires script 040 to fix it first. When 039 is pulled to prod, it will not run)

@Fran-Rg
Copy link

Fran-Rg commented Oct 30, 2023

The flaw is in the way the getRunnableMigrations is calculated: https://github.com/rickbergfalk/postgrator/blob/master/postgrator.js#L211-L234

It only checks for the versions higher than the latest applied.
If should also check for files that have not been applied which are lower than the latest migration.

Is it best practice? Not sure.
Is is unexpected behaviour? Not sure

@nenadalm
Copy link

nenadalm commented Oct 1, 2024

I prefer how it currently works - not applying any older versions than latest one.

It could then happen that older migrations get dependent on newer migrations and when you want to run them from scratch, it won't work (or it might "work", but schema might be different).

e.g.

-- 001
CREATE TABLE IF NOT EXISTS "log"("id" UUID, "info" TEXT);

--002
CREATE TABLE IF NOT EXISTS "log"("id" UUID, "text" TEXT);
  • it makes difference if migrations are run in order 001, 002 or 002, 001.

What I do is that after all migrations run, I check if there are any unapplied migrations and throw in that case. It's suggested that users rename old migrations so that they're run to prevent case where it seems it works in some staging environment and then it crashes on production in case order of migrations matter.

example with error if there are unapplied migrations:

import pg from "pg";
import Postgrator from "postgrator";
import { dirname } from "path";
import { fileURLToPath } from "url";
import { obtainMigrationsLock } from "./locks.mjs";

const __dirname = dirname(fileURLToPath(import.meta.url));

function difference(setA, setB) {
  const _difference = new Set(setA);
  for (const elem of setB) {
    _difference.delete(elem);
  }
  return _difference;
}

async function ensureNoUnappliedMigrations({ client, postgrator }) {
  const { rows } = await client.query(
    'SELECT "version" FROM "schemaversion" WHERE "version" != 0',
  );
  const appliedVersions = new Set(rows.map((r) => parseInt(r.version, 10)));
  const availableVersions = new Set(
    (await postgrator.getMigrations())
      .filter((m) => m.action === "do")
      .map((m) => m.version),
  );

  const missingMigrations = difference(availableVersions, appliedVersions);
  if (missingMigrations.size === 0) {
    return;
  }

  throw new Error(
    `Following migrations were not applied: ${Array.from(missingMigrations.values()).join(", ")} since they're older than current schema version.
    You should probably run "yarn run db:migrations:create" and move their contents into the new migration.`,
  );
}

/**
 * @param {number|'max'} version
 */
export async function migrate(version = "max") {
  const client = new pg.Client();

  try {
    await client.connect();
    await obtainMigrationsLock(client);

    const postgrator = new Postgrator({
      migrationPattern: __dirname + "/../migrations/*",
      driver: "pg",
      validateChecksums: false,
      schemaTable: "public.schemaversion",
      execQuery: (query) => client.query(query),
    });
    const appliedMigrations = await postgrator.migrate(version);
    if (appliedMigrations.length > 0) {
      console.log(appliedMigrations);
    }

    if (version === "max") {
      await ensureNoUnappliedMigrations({ client, postgrator });
    }
  } finally {
    client.end();
  }
}

it would be ofc possible to create script that renames these old migrations automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants