diff --git a/.github/scripts/check-pr-migration-order.mjs b/.github/scripts/check-pr-migration-order.mjs index b67dfb3fabe..d3130c31c5f 100644 --- a/.github/scripts/check-pr-migration-order.mjs +++ b/.github/scripts/check-pr-migration-order.mjs @@ -1,5 +1,6 @@ #!/usr/bin/env node import { execFileSync } from 'node:child_process'; +import { existsSync, readFileSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; const MIGRATIONS_DIRECTORY = 'packages/db/src/migrations/'; @@ -65,6 +66,35 @@ export function checkMigrationOrder(baseMigrationFiles, prMigrationFiles) { }; } +/** + * Judges whether the migrations a PR adds can be applied while the previous + * release is still running. `unknown` without an in-file override fails; + * `requires_staging` only produces a notice. + */ +export async function checkMigrationCompatibility(migrations, { priorMigrationSql, executorSource } = {}) { + // Type-stripped by Node: the policy job runs before dependencies are installed. + const { checkNewMigrationCompatibility, parseNonTransactionalMigrations } = await import( + new URL('../../packages/db/src/migration-compatibility.ts', import.meta.url).href + ); + // `executorSource` is the text of packages/db/src/client.ts. It names the + // migrations that run outside a transaction, which CONCURRENTLY needs. + const nonTransactionalMigrations = executorSource === undefined + ? undefined + : parseNonTransactionalMigrations(executorSource); + if (nonTransactionalMigrations === null) { + return { + passed: false, + errors: ['MIGRATIONS_WITH_TOP_LEVEL_STATEMENTS was not found in packages/db/src/client.ts, so CONCURRENTLY migrations cannot be checked.'], + notices: [], + }; + } + const { passed, errors, notices } = checkNewMigrationCompatibility(migrations, { + priorMigrationSql, + nonTransactionalMigrations, + }); + return { passed, errors, notices }; +} + function gitPaths(args) { return execFileSync('git', args, { encoding: 'utf8' }) .split('\0') @@ -78,7 +108,7 @@ function escapeWorkflowCommand(message) { .replaceAll('\n', '%0A'); } -function main() { +async function main() { const [baseSha, headSha] = process.argv.slice(2); const shaPattern = /^[0-9a-f]{40}$/i; if (!shaPattern.test(baseSha ?? '') || !shaPattern.test(headSha ?? '')) { @@ -95,18 +125,45 @@ function main() { ]).filter((file) => file.endsWith('.sql')); const result = checkMigrationOrder(baseMigrationFiles, prMigrationFiles); - if (result.passed) { - console.log(result.message); - return; + if (!result.passed) { + console.error( + `::error title=Migration numbers must follow the target branch::${escapeWorkflowCommand(result.message)}`, + ); + console.error(result.message); + process.exit(1); } + console.log(result.message); - console.error( - `::error title=Migration numbers must follow the target branch::${escapeWorkflowCommand(result.message)}`, + if (prMigrationFiles.length === 0) return; + // The checkout is the merge of the PR into the target branch, so it holds + // the target branch's migrations and the migrator source. + const root = fileURLToPath(new URL('../../', import.meta.url)); + const compatibility = await checkMigrationCompatibility( + [...prMigrationFiles].sort().map((file) => ({ + fileName: file, + sql: execFileSync('git', ['show', `${headSha}:${file}`], { encoding: 'utf8', maxBuffer: 64 * 1024 * 1024 }), + })), + { + priorMigrationSql: [...baseMigrationFiles].sort() + .filter((file) => existsSync(`${root}${file}`)) + .map((file) => readFileSync(`${root}${file}`, 'utf8')), + executorSource: readFileSync(`${root}packages/db/src/client.ts`, 'utf8'), + }, ); - console.error(result.message); - process.exit(1); + for (const notice of compatibility.notices) { + console.log(`::notice title=Migration compatibility::${escapeWorkflowCommand(notice)}`); + console.log(notice); + } + for (const error of compatibility.errors) { + console.error( + `::error title=Migration is not classified as safe under the running release::${escapeWorkflowCommand(error)}`, + ); + console.error(error); + } + if (!compatibility.passed) process.exit(1); + console.log('Every new migration is classified for application under the running release.'); } if (process.argv[1] === fileURLToPath(import.meta.url)) { - main(); + await main(); } diff --git a/.github/scripts/tests/check-pr-migration-order.test.mjs b/.github/scripts/tests/check-pr-migration-order.test.mjs index 48c1b2db8fc..f169a20de6f 100644 --- a/.github/scripts/tests/check-pr-migration-order.test.mjs +++ b/.github/scripts/tests/check-pr-migration-order.test.mjs @@ -1,6 +1,6 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; -import { checkMigrationOrder } from '../check-pr-migration-order.mjs'; +import { checkMigrationCompatibility, checkMigrationOrder } from '../check-pr-migration-order.mjs'; const migration = (name) => `packages/db/src/migrations/${name}.sql`; @@ -41,3 +41,84 @@ test('fails when a PR inserts a migration before the target branch tip', () => { assert.match(result.message, /0229_from_stale_branch\.sql/); assert.doesNotMatch(result.message, /- packages\/db\/src\/migrations\/0231_valid_but_after_stale\.sql/); }); + +test('fails a new migration the classifier cannot prove safe under the running release', async () => { + const result = await checkMigrationCompatibility([ + { fileName: migration('0231_additive'), sql: 'ALTER TABLE "issues" ADD COLUMN "note" text;' }, + { fileName: migration('0232_plain_index'), sql: 'CREATE INDEX "issues_note_idx" ON "issues" ("note");' }, + ]); + + assert.equal(result.passed, false); + assert.equal(result.errors.length, 1); + assert.match(result.errors[0], /0232_plain_index\.sql/); + assert.match(result.errors[0], /lock_hazard_create_index/); + assert.match(result.errors[0], /-- paperclip:migration-compat online_compatible: { + const result = await checkMigrationCompatibility([{ + fileName: migration('0232_plain_index'), + sql: '-- paperclip:migration-compat online_compatible: issues stays under 10k rows on every stack\nCREATE INDEX "issues_note_idx" ON "issues" ("note");', + }]); + + assert.equal(result.passed, true); + assert.match(result.notices[0], /online_compatible by override/); +}); + +test('fails an override that has no justification', async () => { + const result = await checkMigrationCompatibility([{ + fileName: migration('0232_plain_index'), + sql: '-- paperclip:migration-compat online_compatible:\nCREATE INDEX "issues_note_idx" ON "issues" ("note");', + }]); + + assert.equal(result.passed, false); + assert.match(result.errors[0], /needs a justification/); +}); + +test('reports requires_staging as a notice without failing', async () => { + const result = await checkMigrationCompatibility([ + { fileName: migration('0233_drop'), sql: 'ALTER TABLE "issues" DROP COLUMN "note";' }, + ]); + + assert.equal(result.passed, true); + assert.deepEqual(result.errors, []); + assert.match(result.notices[0], /0233_drop\.sql: requires_staging/); +}); + +test('passes when a PR adds no migrations', async () => { + assert.deepEqual(await checkMigrationCompatibility([]), { passed: true, errors: [], notices: [] }); +}); + +const executorSource = 'const MIGRATIONS_WITH_TOP_LEVEL_STATEMENTS = new Set([\n "0232_listed.sql",\n]);'; +const concurrentIndex = 'CREATE INDEX CONCURRENTLY IF NOT EXISTS "issues_note_idx" ON "issues" ("note");'; + +test('fails a CONCURRENTLY migration that the migrator would run in a transaction', async () => { + const result = await checkMigrationCompatibility( + [{ fileName: migration('0232_unlisted'), sql: concurrentIndex }, { fileName: migration('0232_listed'), sql: concurrentIndex }], + { executorSource }, + ); + + assert.equal(result.passed, false); + assert.equal(result.errors.length, 1); + assert.match(result.errors[0], /0232_unlisted\.sql: uses CONCURRENTLY/); + assert.match(result.errors[0], /Add "0232_unlisted\.sql" to MIGRATIONS_WITH_TOP_LEVEL_STATEMENTS in packages\/db\/src\/client\.ts/); +}); + +test('fails closed when the migrator source no longer names its non-transactional migrations', async () => { + const result = await checkMigrationCompatibility([], { executorSource: 'export const nothing = 1;' }); + + assert.equal(result.passed, false); + assert.match(result.errors[0], /MIGRATIONS_WITH_TOP_LEVEL_STATEMENTS was not found/); +}); + +test('treats CREATE TABLE IF NOT EXISTS as new only when the target branch never created the table', async () => { + const sql = 'CREATE TABLE IF NOT EXISTS "widgets" ("id" uuid);\nALTER TABLE "widgets" DROP COLUMN "title";'; + const fresh = await checkMigrationCompatibility([{ fileName: migration('0231_widgets'), sql }], { priorMigrationSql: [] }); + const existing = await checkMigrationCompatibility([{ fileName: migration('0231_widgets'), sql }], { + priorMigrationSql: ['CREATE TABLE "widgets" ("id" uuid, "title" text);'], + }); + + assert.deepEqual(fresh.notices, []); + assert.match(existing.notices[0], /0231_widgets\.sql: requires_staging/); +}); diff --git a/doc/DATABASE.md b/doc/DATABASE.md index 332d1836782..f50a6c8339f 100644 --- a/doc/DATABASE.md +++ b/doc/DATABASE.md @@ -1317,6 +1317,77 @@ When you change the data model: Then update every layer that the change affects: `packages/shared` types and validators, `server` routes and services, and `ui` API clients and pages. +### Migration compatibility with the running release + +Paperclip Cloud starts a new release while the previous one is still serving +against the same database. Every migration is therefore classified by +`packages/db/src/migration-compatibility.ts` as one of: + +| Class | Meaning | Examples | +| --- | --- | --- | +| `online_compatible` | The previous release keeps working and no lock is held for long. A deployer applies it without a pause. | `CREATE TABLE` and anything on a table created in the same migration, `CREATE INDEX CONCURRENTLY`, `ADD COLUMN` that is nullable or has a constant default, `ADD CONSTRAINT ... NOT VALID`, `CREATE TYPE`, `ALTER TYPE ... ADD VALUE` | +| `requires_staging` | The previous release breaks or its writes are rejected. Ship it in two releases: expand first, contract later. | `DROP TABLE`/`DROP COLUMN`, renames, `ALTER COLUMN ... TYPE`, `SET NOT NULL`, `DROP DEFAULT`, a `NOT NULL` column without a default, a validated `CHECK`, a `UNIQUE` constraint or unique index on an existing table, `TRUNCATE`, `DELETE` | +| `unknown` | The classifier cannot prove either. This includes every statement it does not recognize and every lock or duration hazard (reason codes starting with `lock_hazard_`). | plain `CREATE INDEX` on an existing table, a validated foreign key on an existing table, `VALIDATE CONSTRAINT`, `UPDATE` backfills, procedural `DO` blocks, `CREATE OR REPLACE FUNCTION`, triggers on existing tables, `DROP INDEX`, `DROP CONSTRAINT` | + +A migration takes the worst class of its statements. Lock and duration hazards +are `unknown`, not compatible with a warning. + +A table counts as created in the migration when the statement is a plain +`CREATE TABLE`, or a `CREATE TABLE IF NOT EXISTS` for a table that no earlier +migration created. `IF NOT EXISTS` on a table from an earlier migration proves +nothing, so later statements on that table are judged as changes to an existing +table. + +`drizzle-kit` emits validated foreign keys and plain indexes. For an existing +table, edit the generated SQL: append `NOT VALID` to the foreign key, and use +`CREATE INDEX CONCURRENTLY IF NOT EXISTS`. + +PostgreSQL rejects `CONCURRENTLY` inside a transaction, and the migrator runs +each migration file in one transaction. A migration that uses `CONCURRENTLY` +must therefore be listed in `MIGRATIONS_WITH_TOP_LEVEL_STATEMENTS` in +`packages/db/src/client.ts`, and every statement in it must be idempotent, +because a failed run starts again from the first statement. The pull-request +guard fails a new `CONCURRENTLY` migration that is not in that list. + +**The rule for a new migration.** A pull request fails when a migration it adds +is `unknown`. Rewrite the migration, split it into expand and contract +migrations, or, when you have verified the migration yourself, state the class +in the first lines of the migration file: + +```sql +-- paperclip:migration-compat online_compatible: +``` + +Use `requires_staging` in place of `online_compatible` when the previous +release cannot run against the migration. The justification is mandatory. The +override is read only from the migration file, so it is reviewed with the SQL +and travels with it. A contract migration (for example the `DROP COLUMN` that +follows an expand release) always classifies as `requires_staging`; mark it +`online_compatible` this way once no running release uses the column. +`requires_staging` does not fail the pull request yet; it prints a notice. + +Check a migration locally: + +```sh +cd packages/db +pnpm exec tsx scripts/migration-compatibility.ts src/migrations/0468_busy_wendell_rand.sql +pnpm exec tsx scripts/migration-compatibility.ts # distribution over all migrations +pnpm exec tsx scripts/migration-compatibility.ts --check # the pull-request guard +``` + +The guard runs in `check:migrations` (so in the `@paperclipai/db` typecheck and +build) against the merge base with `origin/master`, and in the pull-request +policy job through `.github/scripts/check-pr-migration-order.mjs`. Migrations +that were already on `master` when the classifier shipped are not annotated. + +Deployers read the result from the cloud migrator manifest +(`cloud-migrators/v1//manifest.json`, built by +`scripts/cloud-migrator-artifacts.mjs`). Its `migrationCompatibility.migrations` +array lists every packed migration in journal order with `idx`, `tag`, +`sqlSha256` (the hash the migrator records in the migration history), +`compatibility`, `classified` (the classifier's verdict before an override), +`override`, `overrideJustification`, and `reasons`. + ## Migration authoring checklist Older embedded databases can use `SQL_ASCII`. Test encoding-sensitive SQL on diff --git a/packages/db/package.json b/packages/db/package.json index 9bcdeb8881a..8b041ed6c69 100644 --- a/packages/db/package.json +++ b/packages/db/package.json @@ -38,7 +38,7 @@ "embedded-postgres" ], "scripts": { - "check:migrations": "tsx src/check-migration-numbering.ts && tsx src/check-migration-safety.ts", + "check:migrations": "tsx src/check-migration-numbering.ts && tsx src/check-migration-safety.ts && tsx scripts/migration-compatibility.ts --check", "build": "pnpm run check:migrations && tsc && rm -rf dist/migrations && mkdir -p dist/migrations/meta && cp src/migrations/*.sql dist/migrations/ && cp src/migrations/meta/_journal.json dist/migrations/meta/", "clean": "rm -rf dist", "typecheck": "pnpm run check:migrations && tsc --noEmit", diff --git a/packages/db/scripts/migration-compatibility.ts b/packages/db/scripts/migration-compatibility.ts new file mode 100644 index 00000000000..56dcc3281cf --- /dev/null +++ b/packages/db/scripts/migration-compatibility.ts @@ -0,0 +1,133 @@ +// Reports whether migrations can be applied while the previous release is +// still running. See "Migration compatibility with the running release" in +// doc/DATABASE.md. +// +// tsx scripts/migration-compatibility.ts summary of every migration +// tsx scripts/migration-compatibility.ts --json per-migration results as JSON +// tsx scripts/migration-compatibility.ts ... classify the named files +// tsx scripts/migration-compatibility.ts --check [--base ] +// fail when a migration added since the merge base with +// (default origin/master, then master) is `unknown` without an override +import { execFileSync } from "node:child_process"; +import { readFileSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { + checkNewMigrationCompatibility, + classifyMigrationSequence, + parseNonTransactionalMigrations, + summarizeMigrationCompatibility, + type MigrationCompatInput, +} from "../src/migration-compatibility.js"; + +const migrationsDir = fileURLToPath(new URL("../src/migrations", import.meta.url)); +const executorPath = fileURLToPath(new URL("../src/client.ts", import.meta.url)); + +function journalFiles(): string[] { + const journal = JSON.parse(readFileSync(path.join(migrationsDir, "meta", "_journal.json"), "utf8")) as { + entries: { idx: number; tag: string }[]; + }; + // Apply order: journal idx, then file name (the journal repeats an idx). + return [...journal.entries] + .sort((left, right) => left.idx - right.idx || left.tag.localeCompare(right.tag)) + .map((entry) => `${entry.tag}.sql`); +} + +function read(file: string): MigrationCompatInput { + return { fileName: path.basename(file), sql: readFileSync(file, "utf8") }; +} + +function git(args: string[]): string | null { + try { + return execFileSync("git", args, { cwd: migrationsDir, encoding: "utf8", stdio: ["ignore", "pipe", "ignore"] }); + } catch { + return null; + } +} + +/** Journal migrations split by whether the merge base has them, or null without git history. */ +function migrationsAddedSince(requestedBase: string | null): { base: string; prior: string[]; files: string[] } | null { + for (const ref of requestedBase ? [requestedBase] : ["origin/master", "master"]) { + const base = git(["merge-base", "HEAD", ref])?.trim(); + if (!base) continue; + // ls-tree lists the current directory (the migrations folder) of the base. + const listed = git(["ls-tree", "--name-only", base]); + if (!listed?.includes(".sql")) continue; + const existing = new Set(listed.split("\n")); + const journal = journalFiles(); + return { base, prior: journal.filter((file) => existing.has(file)), files: journal.filter((file) => !existing.has(file)) }; + } + return null; +} + +function check(requestedBase: string | null): number { + const added = migrationsAddedSince(requestedBase); + if (!added) { + // A source archive or shallow clone has no base to compare with. The pull + // request policy job runs the same guard with full history. + console.log("migration compatibility: no merge base available, skipped."); + return requestedBase ? 2 : 0; + } + const nonTransactionalMigrations = parseNonTransactionalMigrations(readFileSync(executorPath, "utf8")); + if (!nonTransactionalMigrations) { + console.error("error: MIGRATIONS_WITH_TOP_LEVEL_STATEMENTS was not found in packages/db/src/client.ts."); + return 1; + } + const result = checkNewMigrationCompatibility(added.files.map((file) => read(path.join(migrationsDir, file))), { + priorMigrationSql: added.prior.map((file) => read(path.join(migrationsDir, file)).sql), + nonTransactionalMigrations, + }); + for (const notice of result.notices) console.log(`notice: ${notice}`); + for (const error of result.errors) console.error(`error: ${error}`); + if (result.passed) { + console.log(`migration compatibility: ${added.files.length} new migration(s) since ${added.base.slice(0, 12)}, none unclassified.`); + } + return result.passed ? 0 : 1; +} + +function main(): number { + const args = process.argv.slice(2); + const flag = (name: string) => args.includes(name); + const baseIndex = args.indexOf("--base"); + const base = baseIndex === -1 ? null : args[baseIndex + 1] ?? null; + if (flag("--check")) return check(base); + + const named = args.filter((arg, index) => !arg.startsWith("--") && (baseIndex === -1 || index !== baseIndex + 1)); + const journal = journalFiles().map((file) => read(path.join(migrationsDir, file))); + // A named file is judged after the journal migrations that precede it, or + // after the whole journal when it is not in the journal yet. + const results = named.length === 0 + ? classifyMigrationSequence(journal.map((input) => input.sql)).map((result, index) => ({ fileName: journal[index].fileName, ...result })) + : named.map(read).map((input) => { + const position = journal.findIndex((entry) => entry.fileName === input.fileName); + const prior = (position === -1 ? journal : journal.slice(0, position)).map((entry) => entry.sql); + return { fileName: input.fileName, ...classifyMigrationSequence([input.sql], prior)[0] }; + }); + if (flag("--json")) { + console.log(JSON.stringify(results, null, 2)); + return 0; + } + if (named.length > 0) { + for (const result of results) { + console.log(`${result.fileName}: ${result.compatibility}${result.override ? " (override)" : ""}`); + for (const reason of result.reasons) { + const where = reason.statements.length ? ` [statement ${reason.statements.join(", ")}]` : ""; + console.log(` ${reason.compatibility} ${reason.code}${where}: ${reason.message}`); + } + } + return 0; + } + const summary = summarizeMigrationCompatibility(results); + const percent = (count: number) => `${((count / Math.max(summary.total, 1)) * 100).toFixed(1)}%`; + console.log(`${summary.total} migrations, ${summary.overrides} with an override`); + for (const [compatibility, count] of Object.entries(summary.byCompatibility)) { + console.log(` ${compatibility.padEnd(18)} ${String(count).padStart(4)} ${percent(count)}`); + } + console.log("Migrations per blocking reason:"); + for (const reason of summary.blockingReasons) { + console.log(` ${String(reason.migrations).padStart(4)} ${reason.compatibility.padEnd(16)} ${reason.code}`); + } + return 0; +} + +process.exitCode = main(); diff --git a/packages/db/src/migration-compatibility.test.ts b/packages/db/src/migration-compatibility.test.ts new file mode 100644 index 00000000000..68a38e15586 --- /dev/null +++ b/packages/db/src/migration-compatibility.test.ts @@ -0,0 +1,378 @@ +import { execFileSync } from "node:child_process"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; +import { + MIGRATION_COMPAT_REASON_CODES, + checkNewMigrationCompatibility, + classifyMigrationSequence, + classifyMigrationSql, + collectCreatedTables, + parseMigrationCompatOverride, + parseNonTransactionalMigrations, + summarizeMigrationCompatibility, + type MigrationCompatibility, + type MigrationCompatReasonCode, +} from "./migration-compatibility.js"; + +const BP = "--> statement-breakpoint\n"; +const NEW_TABLE = `CREATE TABLE "widgets" ("id" uuid PRIMARY KEY NOT NULL, "issue_id" uuid);${BP}`; + +function codes(sql: string): MigrationCompatReasonCode[] { + return classifyMigrationSql(sql).reasons.map((reason) => reason.code); +} + +type Case = readonly [name: string, sql: string, compatibility: MigrationCompatibility, code: MigrationCompatReasonCode]; + +const cases: readonly Case[] = [ + // Compatible: additive changes the previous release never touches. + ["create table", `CREATE TABLE "widgets" ("id" uuid PRIMARY KEY, "issue_id" uuid REFERENCES "issues"("id"));`, "online_compatible", "create_table"], + ["create table if not exists", `CREATE TABLE IF NOT EXISTS "widgets" ("id" uuid PRIMARY KEY);`, "online_compatible", "create_table_if_not_exists"], + ["create temp table", `CREATE TEMP TABLE scratch (old_id uuid) ON COMMIT DROP;`, "online_compatible", "create_temp_table"], + ["plain index on a table created here", `${NEW_TABLE}CREATE UNIQUE INDEX "widgets_uq" ON "widgets" USING btree ("issue_id");`, "online_compatible", "create_index_new_table"], + ["constraints on a table created here", `${NEW_TABLE}ALTER TABLE "widgets" ADD CONSTRAINT "widgets_issue_fk" FOREIGN KEY ("issue_id") REFERENCES "public"."issues"("id") ON DELETE cascade ON UPDATE no action;`, "online_compatible", "alter_new_table"], + ["trigger on a table created here", `${NEW_TABLE}CREATE TRIGGER widgets_touch BEFORE UPDATE OF issue_id ON "widgets" FOR EACH ROW EXECUTE FUNCTION touch();`, "online_compatible", "create_trigger_new_table"], + ["constant rows in a table created here", `${NEW_TABLE}INSERT INTO "widgets" ("id") VALUES ('00000000-0000-0000-0000-000000000001');`, "online_compatible", "seed_new_table"], + ["concurrent index", `CREATE INDEX CONCURRENTLY IF NOT EXISTS "issues_x_idx" ON "issues" USING btree ("company_id","id") WHERE "issues"."status" = 'done';`, "online_compatible", "create_index_concurrently"], + ["nullable column", `ALTER TABLE "issues" ADD COLUMN "time_zone" text;`, "online_compatible", "add_column"], + ["column with a constant default", `ALTER TABLE "issues" ADD COLUMN IF NOT EXISTS "attempts" integer DEFAULT 0 NOT NULL;`, "online_compatible", "add_column"], + ["column with a cast literal default", `ALTER TABLE "issues" ADD COLUMN "meta" jsonb NOT NULL DEFAULT '{"a": "b, c"}'::jsonb;`, "online_compatible", "add_column"], + ["column with a stable timestamp default", `ALTER TABLE "issues" ADD COLUMN "seen_at" timestamp with time zone DEFAULT now() NOT NULL;`, "online_compatible", "add_column"], + ["several additive actions", `ALTER TABLE "issues" ADD COLUMN "a" text, ADD COLUMN "b" numeric(24, 7) DEFAULT '0' NOT NULL;`, "online_compatible", "add_column"], + ["foreign key added NOT VALID", `ALTER TABLE "issues" ADD CONSTRAINT "issues_owner_fk" FOREIGN KEY ("owner_id") REFERENCES "users"("id") NOT VALID;`, "online_compatible", "add_constraint_not_valid"], + ["check added NOT VALID", `ALTER TABLE "issues" ADD CONSTRAINT "issues_kind_check" CHECK ("kind" IN ('a', 'b')) NOT VALID;`, "online_compatible", "add_constraint_not_valid"], + ["drop not null", `ALTER TABLE "issues" ALTER COLUMN "title" DROP NOT NULL;`, "online_compatible", "drop_not_null"], + ["create enum", `CREATE TYPE "public"."mood" AS ENUM('sad', 'ok');`, "online_compatible", "create_type"], + ["add enum value", `ALTER TYPE "public"."mood" ADD VALUE IF NOT EXISTS 'happy';`, "online_compatible", "add_enum_value"], + ["create sequence", `CREATE SEQUENCE "issue_numbers";`, "online_compatible", "create_sequence"], + ["create extension", `CREATE EXTENSION IF NOT EXISTS "pg_trgm";`, "online_compatible", "create_extension"], + ["new function", `CREATE FUNCTION touch() RETURNS trigger AS $fn$ BEGIN DELETE FROM issues; RETURN NEW; END; $fn$ LANGUAGE plpgsql;`, "online_compatible", "create_function"], + ["new view", `CREATE VIEW "open_issues" AS SELECT * FROM "issues";`, "online_compatible", "create_view"], + ["comment", `COMMENT ON COLUMN "issues"."title" IS 'DROP TABLE issues; is only text here';`, "online_compatible", "comment"], + ["no-op select", `SELECT 1;`, "online_compatible", "no_op"], + ["analyze", `ANALYZE "issues";`, "online_compatible", "analyze"], + ["extended statistics", `CREATE STATISTICS IF NOT EXISTS "s" ON ("a"), ("b") FROM "issues";`, "online_compatible", "create_statistics"], + ["guarded DO block around an additive statement", `DO $$ BEGIN\n IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'x' AND conrelid = 'issues'::regclass) THEN\n ALTER TABLE "issues" ADD CONSTRAINT "x" FOREIGN KEY ("a") REFERENCES "b"("id") NOT VALID;\n END IF;\nEND $$;`, "online_compatible", "add_constraint_not_valid"], + ["duplicate-tolerant DO block on a table created here", `${NEW_TABLE}DO $$ BEGIN\n ALTER TABLE "widgets" ADD CONSTRAINT "w_fk" FOREIGN KEY ("issue_id") REFERENCES "public"."issues"("id") ON DELETE cascade;\nEXCEPTION\n WHEN duplicate_object THEN null;\nEND $$;`, "online_compatible", "alter_new_table"], + + // Requires staging: the previous release breaks or its writes are rejected. + ["drop table", `DROP TABLE IF EXISTS "widgets" CASCADE;`, "requires_staging", "drop_table"], + ["drop column", `ALTER TABLE "issues" DROP COLUMN IF EXISTS "legacy";`, "requires_staging", "drop_column"], + ["rename table", `ALTER TABLE "issues" RENAME TO "tasks";`, "requires_staging", "rename_table"], + ["rename column", `ALTER TABLE "issues" RENAME COLUMN "title" TO "name";`, "requires_staging", "rename_column"], + ["alter column type", `ALTER TABLE "issues" ALTER COLUMN "number" SET DATA TYPE bigint USING "number"::bigint;`, "requires_staging", "alter_column_type"], + ["set not null on an existing column", `ALTER TABLE "issues" ALTER COLUMN "title" SET NOT NULL;`, "requires_staging", "set_not_null"], + ["not null column without a default", `ALTER TABLE "issues" ADD COLUMN "public_id" varchar(128) NOT NULL;`, "requires_staging", "add_not_null_column_without_default"], + ["drop default", `ALTER TABLE "issues" ALTER COLUMN "status" DROP DEFAULT;`, "requires_staging", "drop_default"], + ["validated check", `ALTER TABLE "issues" ADD CONSTRAINT "issues_kind_check" CHECK ("kind" IN ('a', 'b'));`, "requires_staging", "add_validated_check"], + ["unique constraint", `ALTER TABLE "issues" ADD CONSTRAINT "issues_uq" UNIQUE("company_id","number");`, "requires_staging", "add_unique_constraint"], + ["unique index on an existing table", `CREATE UNIQUE INDEX CONCURRENTLY "issues_uq" ON "issues" ("company_id","number");`, "requires_staging", "create_unique_index_existing_table"], + ["drop type", `DROP TYPE "public"."mood";`, "requires_staging", "drop_type"], + ["rename enum value", `ALTER TYPE "mood" RENAME VALUE 'ok' TO 'fine';`, "requires_staging", "rename_type"], + ["truncate", `TRUNCATE TABLE "issues";`, "requires_staging", "truncate"], + ["delete", `DELETE FROM "issues" WHERE "status" = 'cancelled';`, "requires_staging", "delete_rows"], + ["delete inside a CTE", `WITH ranked AS (SELECT id FROM issues) DELETE FROM "issues" i USING ranked r WHERE i.id = r.id;`, "requires_staging", "delete_rows"], + ["guarded DO block around a validated check", `DO $$ BEGIN\n ALTER TABLE "issues" ADD CONSTRAINT "c" CHECK ("a" > 0);\nEXCEPTION WHEN duplicate_object THEN null;\nEND $$;`, "requires_staging", "add_validated_check"], + + // Lock and duration hazards: unknown, reported under lock_hazard_ codes. + ["plain index on an existing table", `CREATE INDEX IF NOT EXISTS "issues_x_idx" ON "issues" USING btree ("company_id");`, "unknown", "lock_hazard_create_index"], + ["validated foreign key on an existing table", `ALTER TABLE "issues" ADD CONSTRAINT "issues_owner_fk" FOREIGN KEY ("owner_id") REFERENCES "public"."users"("id") ON DELETE set null ON UPDATE no action;`, "unknown", "lock_hazard_validated_foreign_key"], + ["validated foreign key in a guarded DO block", `DO $$ BEGIN\n IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'x') THEN\n ALTER TABLE "issues" ADD CONSTRAINT "x" FOREIGN KEY ("a") REFERENCES "b"("id");\n END IF;\nEND $$;`, "unknown", "lock_hazard_validated_foreign_key"], + ["validate constraint", `ALTER TABLE "issues" VALIDATE CONSTRAINT "issues_owner_fk";`, "unknown", "lock_hazard_validate_constraint"], + ["set not null on a column added here", `ALTER TABLE "issues" ADD COLUMN "kind" text DEFAULT 'task';${BP}ALTER TABLE "issues" ALTER COLUMN "kind" SET NOT NULL;`, "unknown", "lock_hazard_set_not_null"], + ["column with a volatile default", `ALTER TABLE "issues" ADD COLUMN "token" uuid DEFAULT gen_random_uuid() NOT NULL;`, "unknown", "lock_hazard_table_rewrite"], + ["generated column", `ALTER TABLE "issues" ADD COLUMN "n" bigint GENERATED ALWAYS AS IDENTITY;`, "unknown", "lock_hazard_table_rewrite"], + ["column with an inline foreign key", `ALTER TABLE "issues" ADD COLUMN "owner_id" uuid REFERENCES "users"("id");`, "unknown", "lock_hazard_add_column_constraint"], + ["update backfill", `UPDATE "issues" SET "kind" = 'task' WHERE "kind" IS NULL;`, "unknown", "lock_hazard_data_backfill"], + ["CTE backfill", `WITH ranked AS (SELECT id FROM issues) UPDATE "issues" i SET n = 1 FROM ranked r WHERE i.id = r.id;`, "unknown", "lock_hazard_data_backfill"], + ["batched backfill loop", `DO $$ DECLARE n integer; BEGIN LOOP UPDATE issues SET a = 1 WHERE id IN (SELECT id FROM issues LIMIT 1000); GET DIAGNOSTICS n = ROW_COUNT; EXIT WHEN n = 0; END LOOP; END $$;`, "unknown", "lock_hazard_data_backfill"], + ["copy from live tables into a new table", `${NEW_TABLE}INSERT INTO "widgets" ("id") SELECT "id" FROM "issues";`, "unknown", "lock_hazard_copy_into_new_table"], + ["lock table", `LOCK TABLE "issues" IN ACCESS EXCLUSIVE MODE;`, "unknown", "lock_hazard_lock_table"], + + // Unknown: not positively recognized, or it changes what old code experiences. + ["insert into an existing table", `INSERT INTO "issues" ("id") VALUES ('1');`, "unknown", "data_change_existing_table"], + ["set default", `ALTER TABLE "issues" ALTER COLUMN "status" SET DEFAULT 'todo';`, "unknown", "set_default"], + ["drop constraint", `ALTER TABLE "issues" DROP CONSTRAINT "issues_owner_fk";`, "unknown", "drop_constraint"], + ["drop index", `DROP INDEX IF EXISTS "issues_x_idx";`, "unknown", "drop_index"], + ["drop trigger", `DROP TRIGGER IF EXISTS t ON "issues";`, "unknown", "drop_object"], + ["constraint from an existing index", `ALTER TABLE "issues" ADD CONSTRAINT "issues_uq" UNIQUE USING INDEX "issues_uq_idx";`, "unknown", "add_constraint_other"], + ["replace function", `CREATE OR REPLACE FUNCTION touch() RETURNS trigger AS $$ BEGIN RETURN NEW; END; $$ LANGUAGE plpgsql;`, "unknown", "replace_function"], + ["replace view", `CREATE OR REPLACE VIEW "open_issues" AS SELECT * FROM "issues";`, "unknown", "replace_view"], + ["trigger on an existing table", `CREATE TRIGGER t BEFORE INSERT OR UPDATE OF a, b ON "issues" FOR EACH ROW EXECUTE FUNCTION touch();`, "unknown", "create_trigger_existing_table"], + ["create table as", `CREATE TABLE "issues_copy" AS SELECT * FROM "issues";`, "unknown", "create_table_as"], + ["procedural DO block", `DO $$ BEGIN EXECUTE format('ALTER TABLE %I ADD COLUMN x text', 'issues'); END $$;`, "unknown", "do_block_procedural"], + ["DO block with an unrecognized guard", `DO $$ BEGIN IF current_setting('x') = 'y' THEN ALTER TABLE "issues" ADD COLUMN "a" text; END IF; END $$;`, "unknown", "do_block_procedural"], + ["DO block that raises", `DO $$ BEGIN IF EXISTS (SELECT 1 FROM issues) THEN RAISE EXCEPTION 'no'; END IF; END $$;`, "unknown", "do_block_procedural"], + ["unrecognized alter table action", `ALTER TABLE "issues" ENABLE ROW LEVEL SECURITY;`, "unknown", "unrecognized_alter_table_action"], + ["unrecognized statement", `GRANT SELECT ON "issues" TO reader;`, "unknown", "unrecognized_statement"], + ["vacuum", `VACUUM FULL "issues";`, "unknown", "unrecognized_statement"], + ["empty file", `-- nothing here\n`, "unknown", "no_statements"], +]; + +describe("classifyMigrationSql", () => { + it.each(cases)("%s", (_name, sql, compatibility, code) => { + const result = classifyMigrationSql(sql); + expect(result.reasons.map((reason) => reason.code)).toContain(code); + expect(result.compatibility).toBe(compatibility); + expect(result.classified).toBe(compatibility); + expect(result.override).toBeNull(); + }); + + it("has a test case for every reason code", () => { + const covered = new Set([...cases.map((entry) => entry[3]), "invalid_override"]); + expect(MIGRATION_COMPAT_REASON_CODES.filter((code) => !covered.has(code))).toEqual([]); + }); + + it("takes the worst statement: staging over unknown over compatible", () => { + const additive = `ALTER TABLE "issues" ADD COLUMN "a" text;`; + const hazard = `CREATE INDEX "i" ON "issues" ("a");`; + const breaking = `ALTER TABLE "issues" DROP COLUMN "b";`; + expect(classifyMigrationSql([additive, hazard].join(BP)).compatibility).toBe("unknown"); + expect(classifyMigrationSql([additive, hazard, breaking].join(BP)).compatibility).toBe("requires_staging"); + expect(classifyMigrationSql([additive, hazard, breaking].join(BP)).reasons).toEqual([ + expect.objectContaining({ code: "add_column", compatibility: "online_compatible", statements: [1] }), + expect.objectContaining({ code: "lock_hazard_create_index", compatibility: "unknown", statements: [2] }), + expect.objectContaining({ code: "drop_column", compatibility: "requires_staging", statements: [3] }), + ]); + }); + + it("splits statements that share a chunk without a breakpoint", () => { + expect(codes(`ALTER TABLE "issues" ADD COLUMN "a" text;\nDROP TABLE "issues";`)).toEqual(["add_column", "drop_table"]); + }); + + it("classifies each action of a multi-action ALTER TABLE", () => { + expect(codes(`ALTER TABLE "issues" ADD COLUMN "a" text, DROP COLUMN "b", ALTER COLUMN "c" TYPE bigint;`)) + .toEqual(["add_column", "drop_column", "alter_column_type"]); + }); + + it("does not let comments or string contents hide or invent statements", () => { + expect(classifyMigrationSql(`-- DROP TABLE "issues";\n/* TRUNCATE issues; */\nALTER TABLE "issues" ADD COLUMN "a" text DEFAULT 'x; DROP TABLE issues;';`).compatibility) + .toBe("online_compatible"); + expect(classifyMigrationSql(`COMMENT ON TABLE "issues" IS E'it\\'s';${BP}DROP TABLE "issues";`).compatibility).toBe("requires_staging"); + expect(classifyMigrationSql(`COMMENT ON TABLE "issues" IS 'it''s';\nDROP TABLE "issues";`).compatibility).toBe("requires_staging"); + }); + + it("keeps a quoted identifier with an escaped quote distinct from other names", () => { + // "iss""ues" is a table named iss"ues, not issues. + const sql = `CREATE TABLE "iss""ues" (id uuid);${BP}DROP TABLE "issues";${BP}ALTER TABLE issues DROP COLUMN title;`; + expect(codes(sql)).toEqual(["create_table", "drop_table", "drop_column"]); + expect(classifyMigrationSql(sql).compatibility).toBe("requires_staging"); + expect(collectCreatedTables(sql)).toEqual(['"iss%22ues"']); + expect(classifyMigrationSql(`CREATE TABLE "iss""ues" (id uuid);${BP}CREATE INDEX i ON "iss""ues" (id);`).compatibility).toBe("online_compatible"); + expect(classifyMigrationSql(`CREATE TABLE "Issues" (id uuid);${BP}DROP TABLE issues;`).compatibility).toBe("requires_staging"); + }); + + it("treats quoted, unquoted and schema-qualified names of a new table alike", () => { + expect(classifyMigrationSql(`CREATE TABLE public.widgets (id uuid);${BP}CREATE INDEX "w" ON "widgets" (id);${BP}ALTER TABLE "public"."widgets" ADD COLUMN a text NOT NULL;`).compatibility) + .toBe("online_compatible"); + }); + + it("does not take CREATE TABLE IF NOT EXISTS as proof that a table is new", () => { + const sql = `CREATE TABLE IF NOT EXISTS issues (id uuid);${BP}ALTER TABLE issues DROP COLUMN title;${BP}CREATE INDEX "i" ON issues (id);`; + expect(classifyMigrationSql(sql)).toMatchObject({ compatibility: "requires_staging" }); + expect(codes(sql)).toEqual(["create_table_if_not_exists", "drop_column", "lock_hazard_create_index"]); + // An earlier migration created the table: it exists. + expect(classifyMigrationSql(sql, { existingTables: new Set(["issues"]) }).compatibility).toBe("requires_staging"); + // No earlier migration created it: the table is new, whatever the spelling. + expect(classifyMigrationSql(sql, { existingTables: new Set(["projects"]) }).compatibility).toBe("online_compatible"); + expect(classifyMigrationSql(sql.replace("issues (id uuid)", `"public"."issues" (id uuid)`), { existingTables: new Set() }).compatibility) + .toBe("online_compatible"); + // A temporary table always belongs to the migration session. + expect(classifyMigrationSql(`CREATE TEMP TABLE IF NOT EXISTS scratch (id uuid);${BP}DROP TABLE scratch;`).compatibility).toBe("online_compatible"); + }); + + it("classifies a sequence knowing the tables earlier migrations created", () => { + const create = `DO $$ BEGIN IF to_regclass('public.widgets') IS NULL THEN CREATE TABLE "widgets" ("id" uuid); END IF; END $$;`; + const repair = `CREATE TABLE IF NOT EXISTS "widgets" ("id" uuid);${BP}ALTER TABLE "widgets" ADD COLUMN "n" integer NOT NULL;`; + const rename = `ALTER TABLE "widgets" RENAME TO "gadgets";`; + const again = `CREATE TABLE IF NOT EXISTS "gadgets" ("id" uuid);${BP}CREATE INDEX "g" ON "gadgets" ("id");`; + expect(collectCreatedTables(create)).toEqual(["widgets"]); + expect(collectCreatedTables(rename)).toEqual(["gadgets"]); + expect(classifyMigrationSequence([repair, create, repair, rename, again]).map((result) => result.compatibility)) + .toEqual(["online_compatible", "online_compatible", "requires_staging", "requires_staging", "unknown"]); + expect(classifyMigrationSequence([repair], [create])[0].compatibility).toBe("requires_staging"); + }); + + it("reports whether a migration needs to run outside a transaction", () => { + expect(classifyMigrationSql(`CREATE INDEX CONCURRENTLY "i" ON "issues" ("a");`).usesConcurrently).toBe(true); + expect(classifyMigrationSql(`DROP INDEX CONCURRENTLY IF EXISTS "i";`).usesConcurrently).toBe(true); + expect(classifyMigrationSql(`COMMENT ON TABLE "issues" IS 'built CONCURRENTLY';${BP}CREATE INDEX "i" ON "issues" ("a");`).usesConcurrently).toBe(false); + }); + + it("does not treat an existing table as new because another table was created", () => { + expect(classifyMigrationSql(`${NEW_TABLE}CREATE INDEX "i" ON "widgets_archive" ("id");`).compatibility).toBe("unknown"); + }); + + it("never classifies an unrecognized ADD CONSTRAINT form as a column", () => { + expect(codes(`ALTER TABLE "issues" ADD CONSTRAINT "c" NOT NULL "title";`)).toEqual(["add_constraint_other"]); + }); + + it("classifies every statement inside a guarded DO block", () => { + const sql = `DO $$ BEGIN\n IF EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'x') THEN\n ALTER TABLE "issues" DROP CONSTRAINT "x";\n END IF;\n ALTER TABLE "issues" ADD CONSTRAINT "x" FOREIGN KEY ("a") REFERENCES "b"("id") NOT VALID;\nEND $$;`; + expect(codes(sql)).toEqual(["drop_constraint", "add_constraint_not_valid"]); + }); +}); + +describe("migration-compat override", () => { + const hazard = `CREATE INDEX "i" ON "issues" ("a");`; + + it("replaces the verdict and keeps the classifier's own result", () => { + const result = classifyMigrationSql(`-- paperclip:migration-compat online_compatible: issues has under 1k rows on every stack\n${hazard}`); + expect(result).toMatchObject({ + compatibility: "online_compatible", + classified: "unknown", + override: { compatibility: "online_compatible", justification: "issues has under 1k rows on every stack" }, + overrideError: null, + }); + expect(result.reasons.map((reason) => reason.code)).toEqual(["lock_hazard_create_index"]); + }); + + it("can mark a contract migration compatible and an additive one staged", () => { + expect(classifyMigrationSql(`-- Contract step.\n\n-- paperclip:migration-compat online_compatible: no release since v0.3 reads issues.legacy\nALTER TABLE "issues" DROP COLUMN "legacy";`)) + .toMatchObject({ compatibility: "online_compatible", classified: "requires_staging" }); + expect(classifyMigrationSql(`-- paperclip:migration-compat requires_staging: old servers crash on the new enum value\nALTER TYPE "mood" ADD VALUE 'x';`)) + .toMatchObject({ compatibility: "requires_staging", classified: "online_compatible" }); + }); + + it.each([ + ["missing justification", `-- paperclip:migration-compat online_compatible\n${hazard}`], + ["empty justification", `-- paperclip:migration-compat online_compatible: \n${hazard}`], + ["token justification", `-- paperclip:migration-compat online_compatible: ok\n${hazard}`], + ["unknown value", `-- paperclip:migration-compat unknown: we could not decide today\n${hazard}`], + ["unsupported value", `-- paperclip:migration-compat safe: this one is certainly fine\n${hazard}`], + ["missing colon", `-- paperclip:migration-compat online_compatible because it is fine\n${hazard}`], + ["two directives", `-- paperclip:migration-compat online_compatible: the table is tiny everywhere\n-- paperclip:migration-compat requires_staging: or maybe it is not\n${hazard}`], + ["after the first statement", `${hazard}\n-- paperclip:migration-compat online_compatible: the table is tiny everywhere\n`], + ["inside a block comment", `/* paperclip:migration-compat online_compatible: the table is tiny everywhere */\n${hazard}`], + ["on a breakpoint line", `--> statement-breakpoint paperclip:migration-compat online_compatible: the table is tiny\n${hazard}`], + ])("rejects an override with %s", (_name, sql) => { + const result = classifyMigrationSql(sql); + expect(result.override).toBeNull(); + expect(result.overrideError).toMatch(/paperclip:migration-compat/); + expect(result.compatibility).toBe("unknown"); + expect(result.reasons.map((reason) => reason.code)).toContain("invalid_override"); + }); + + it("never lets a rejected override improve the verdict", () => { + expect(classifyMigrationSql(`-- paperclip:migration-compat online_compatible\nDROP TABLE "issues";`).compatibility).toBe("requires_staging"); + expect(classifyMigrationSql(`-- paperclip:migration-compat online_compatible\nALTER TABLE "issues" ADD COLUMN "a" text;`).compatibility).toBe("unknown"); + }); + + it("reads nothing when the file has no directive", () => { + expect(parseMigrationCompatOverride(`-- An ordinary comment about compat.\n${hazard}`)).toEqual({ override: null, error: null }); + }); +}); + +describe("checkNewMigrationCompatibility", () => { + const additive = { fileName: "0500_additive.sql", sql: `ALTER TABLE "issues" ADD COLUMN "a" text;` }; + const unknown = { fileName: "0501_unknown.sql", sql: `CREATE INDEX "i" ON "issues" ("a");${BP}UPDATE "issues" SET a = 'x';` }; + const staged = { fileName: "0502_staged.sql", sql: `ALTER TABLE "issues" DROP COLUMN "a";` }; + + it("passes with no new migrations and with compatible ones", () => { + expect(checkNewMigrationCompatibility([])).toMatchObject({ passed: true, errors: [], notices: [] }); + expect(checkNewMigrationCompatibility([additive])).toMatchObject({ passed: true, errors: [], notices: [] }); + }); + + it("fails an unknown migration and tells the author what to do", () => { + const result = checkNewMigrationCompatibility([additive, unknown]); + expect(result.passed).toBe(false); + expect(result.errors).toHaveLength(1); + const [error] = result.errors; + expect(error).toContain("0501_unknown.sql"); + expect(error).toContain("lock_hazard_create_index (statement 1)"); + expect(error).toContain("lock_hazard_data_backfill (statement 2)"); + expect(error).toContain("CREATE INDEX CONCURRENTLY"); + expect(error).toContain("expand migration now and a contract migration in a later release"); + expect(error).toContain("-- paperclip:migration-compat online_compatible: { + const result = checkNewMigrationCompatibility([{ ...unknown, sql: `-- paperclip:migration-compat online_compatible: issues is small and the update is idempotent\n${unknown.sql}` }]); + expect(result).toMatchObject({ passed: true, errors: [] }); + expect(result.notices).toEqual([expect.stringMatching(/0501_unknown\.sql: online_compatible by override \(classifier said unknown\): issues is small/)]); + }); + + it("fails an override without a justification, even on a compatible migration", () => { + const result = checkNewMigrationCompatibility([{ ...additive, sql: `-- paperclip:migration-compat online_compatible:\n${additive.sql}` }]); + expect(result.passed).toBe(false); + expect(result.errors).toEqual([expect.stringMatching(/0500_additive\.sql: .*needs a justification/)]); + }); + + it("fails a CONCURRENTLY migration that the migrator would run inside a transaction", () => { + const concurrent = { fileName: "packages/db/src/migrations/0503_index.sql", sql: `CREATE INDEX CONCURRENTLY IF NOT EXISTS "i" ON "issues" ("a");` }; + const result = checkNewMigrationCompatibility([concurrent], { nonTransactionalMigrations: new Set(["0313_private_task_access.sql"]) }); + expect(result.passed).toBe(false); + expect(result.errors).toEqual([expect.stringMatching(/0503_index\.sql: uses CONCURRENTLY[\s\S]*Add "0503_index\.sql" to MIGRATIONS_WITH_TOP_LEVEL_STATEMENTS in packages\/db\/src\/client\.ts/)]); + expect(checkNewMigrationCompatibility([concurrent], { nonTransactionalMigrations: new Set(["0503_index.sql"]) }).passed).toBe(true); + // Without the list (a caller that cannot read the migrator) the check is skipped. + expect(checkNewMigrationCompatibility([concurrent]).passed).toBe(true); + }); + + it("judges new migrations after the ones already on the target branch", () => { + const repair = { fileName: "0504_repair.sql", sql: `CREATE TABLE IF NOT EXISTS "widgets" ("id" uuid);${BP}CREATE INDEX "w" ON "widgets" ("id");` }; + expect(checkNewMigrationCompatibility([repair], { priorMigrationSql: [] }).passed).toBe(true); + expect(checkNewMigrationCompatibility([repair], { priorMigrationSql: [`CREATE TABLE "widgets" ("id" uuid);`] }).passed).toBe(false); + expect(checkNewMigrationCompatibility([repair]).passed).toBe(false); + // The second new migration sees the table the first one created. + expect(checkNewMigrationCompatibility([repair, { ...repair, fileName: "0505_again.sql" }], { priorMigrationSql: [] }).errors) + .toEqual([expect.stringContaining("0505_again.sql")]); + }); + + it("does not fail requires_staging, and prints a notice instead", () => { + const result = checkNewMigrationCompatibility([staged]); + expect(result).toMatchObject({ passed: true, errors: [] }); + expect(result.notices).toEqual([expect.stringMatching(/0502_staged\.sql: requires_staging[\s\S]*drop_column \(statement 1\)[\s\S]*does not fail the check yet/)]); + }); +}); + +describe("the shipped migrations", () => { + const dbDir = fileURLToPath(new URL("..", import.meta.url)); + const migrationsDir = `${dbDir}src/migrations`; + const journal = JSON.parse(readFileSync(`${migrationsDir}/meta/_journal.json`, "utf8")) as { entries: { idx: number; tag: string }[] }; + const files = [...journal.entries] + .sort((left, right) => left.idx - right.idx || left.tag.localeCompare(right.tag)) + .map((entry) => `${entry.tag}.sql`); + const classified = classifyMigrationSequence(files.map((file) => readFileSync(`${migrationsDir}/${file}`, "utf8"))); + const results = files.map((file, index) => ({ file, result: classified[index] })); + + it("classifies every file without an invalid override and with a consistent summary", () => { + expect(results.filter(({ result }) => result.overrideError).map(({ file }) => file)).toEqual([]); + const summary = summarizeMigrationCompatibility(results.map(({ result }) => result)); + expect(summary.total).toBe(results.length); + expect(Object.values(summary.byCompatibility).reduce((sum, count) => sum + count, 0)).toBe(results.length); + // The rules must keep recognizing the common additive shapes. + expect(summary.byCompatibility.online_compatible).toBeGreaterThan(results.length * 0.4); + }); + + it("keeps known hazardous migrations out of online_compatible", () => { + const verdict = (prefix: string) => results.find(({ file }) => file.startsWith(prefix))?.result.compatibility; + // 0224 adds a NOT NULL column without a default to a live table. + expect(verdict("0224_")).toBe("requires_staging"); + // 0313 backfills privacy columns in batched loops. + expect(verdict("0313_")).not.toBe("online_compatible"); + // 0466 drops a column the previous release wrote. + expect(verdict("0466_")).toBe("requires_staging"); + }); + + it("finds the migrator's non-transactional list, which holds every CONCURRENTLY migration", () => { + const listed = parseNonTransactionalMigrations(readFileSync(`${dbDir}src/client.ts`, "utf8")); + expect(listed?.has("0313_private_task_access.sql")).toBe(true); + expect(results.filter(({ file, result }) => result.usesConcurrently && !listed?.has(file)).map(({ file }) => file)).toEqual([]); + expect(parseNonTransactionalMigrations("const other = new Set([]);")).toBeNull(); + }); + + it("classifies the files named on the command line, after the migrations that precede them", () => { + const run = (...args: string[]) => execFileSync(process.execPath, ["--import", "tsx", "scripts/migration-compatibility.ts", ...args], { cwd: dbDir, encoding: "utf8" }); + const one = run("src/migrations/0224_unified_adapter_auth_sessions.sql"); + expect(one).toMatch(/^0224_unified_adapter_auth_sessions\.sql: requires_staging\n/); + expect(one).toContain("add_not_null_column_without_default"); + expect(one.match(/^\S+\.sql: /gm)).toHaveLength(1); + const two = JSON.parse(run("--json", "src/migrations/0468_busy_wendell_rand.sql", "src/migrations/0466_drop_project_environment.sql")) as { fileName: string; compatibility: string }[]; + expect(two.map(({ fileName, compatibility }) => [fileName, compatibility])).toEqual([ + ["0468_busy_wendell_rand.sql", "online_compatible"], + ["0466_drop_project_environment.sql", "requires_staging"], + ]); + }, 60_000); +}); diff --git a/packages/db/src/migration-compatibility.ts b/packages/db/src/migration-compatibility.ts new file mode 100644 index 00000000000..20c461bcf3c --- /dev/null +++ b/packages/db/src/migration-compatibility.ts @@ -0,0 +1,679 @@ +// Classifies whether a migration can be applied while the previous release's +// application code is still reading and writing the same database. +// +// This file is loaded with Node's built-in type stripping by the release and +// policy scripts, which run before dependencies are installed. Keep it free of +// imports and of TypeScript syntax that needs transpiling (enums, namespaces, +// parameter properties). + +export type MigrationCompatibility = "online_compatible" | "requires_staging" | "unknown"; + +export const MIGRATION_COMPATIBILITY_VALUES: readonly MigrationCompatibility[] = [ + "online_compatible", + "requires_staging", + "unknown", +]; + +export const MIGRATION_COMPAT_DIRECTIVE = "paperclip:migration-compat"; +export const MIGRATION_COMPAT_MIN_JUSTIFICATION_LENGTH = 10; + +type ReasonDefinition = { + readonly compatibility: MigrationCompatibility; + readonly message: string; +}; + +// Codes that start with `lock_hazard_` mark statements whose only known problem +// is how long they hold a lock that blocks the old release. They classify as +// `unknown`, not as compatible with a warning: a deployer that may not pause +// must not start a statement whose duration grows with table size. +const REASONS = { + create_table: { compatibility: "online_compatible", message: "Creates a table the previous release does not use." }, + create_table_if_not_exists: { compatibility: "online_compatible", message: "Creates a table only when it is missing. It may already exist, so later statements on it are judged as changes to an existing table." }, + create_temp_table: { compatibility: "online_compatible", message: "Creates a temporary table visible only to the migration session." }, + alter_new_table: { compatibility: "online_compatible", message: "Changes a table created earlier in this migration." }, + create_index_new_table: { compatibility: "online_compatible", message: "Builds an index on a table created earlier in this migration." }, + create_index_concurrently: { compatibility: "online_compatible", message: "Builds a non-unique index without blocking writes." }, + add_column: { compatibility: "online_compatible", message: "Adds a column that is nullable or has a constant default." }, + add_constraint_not_valid: { compatibility: "online_compatible", message: "Adds a NOT VALID constraint, which does not scan existing rows." }, + drop_not_null: { compatibility: "online_compatible", message: "Relaxes a NOT NULL column." }, + create_type: { compatibility: "online_compatible", message: "Creates a type the previous release does not use." }, + add_enum_value: { compatibility: "online_compatible", message: "Adds a value to an enum type." }, + create_sequence: { compatibility: "online_compatible", message: "Creates a sequence the previous release does not use." }, + create_extension: { compatibility: "online_compatible", message: "Installs an extension." }, + create_function: { compatibility: "online_compatible", message: "Creates a function that did not exist before." }, + create_view: { compatibility: "online_compatible", message: "Creates a view that did not exist before." }, + create_trigger_new_table: { compatibility: "online_compatible", message: "Creates a trigger on a table created earlier in this migration." }, + comment: { compatibility: "online_compatible", message: "Sets a catalog comment." }, + create_statistics: { compatibility: "online_compatible", message: "Creates extended planner statistics without blocking reads or writes." }, + analyze: { compatibility: "online_compatible", message: "Refreshes planner statistics without blocking reads or writes." }, + no_op: { compatibility: "online_compatible", message: "Has no effect on the schema or data." }, + seed_new_table: { compatibility: "online_compatible", message: "Writes rows to a table created earlier in this migration." }, + + drop_table: { compatibility: "requires_staging", message: "Drops a table the previous release may still use." }, + drop_column: { compatibility: "requires_staging", message: "Drops a column the previous release may still read or write." }, + rename_table: { compatibility: "requires_staging", message: "Renames a table the previous release still addresses by its old name." }, + rename_column: { compatibility: "requires_staging", message: "Renames a column the previous release still addresses by its old name." }, + alter_column_type: { compatibility: "requires_staging", message: "Changes a column type, which rewrites the table and can reject the previous release's values." }, + set_not_null: { compatibility: "requires_staging", message: "Sets NOT NULL on a column the previous release may still leave empty." }, + add_not_null_column_without_default: { compatibility: "requires_staging", message: "Adds a NOT NULL column without a default; inserts from the previous release fail." }, + drop_default: { compatibility: "requires_staging", message: "Drops a default the previous release may rely on when it inserts." }, + add_validated_check: { compatibility: "requires_staging", message: "Adds a validated CHECK constraint that the previous release's writes could violate." }, + add_unique_constraint: { compatibility: "requires_staging", message: "Adds a UNIQUE or PRIMARY KEY constraint to an existing table that the previous release's writes could violate." }, + create_unique_index_existing_table: { compatibility: "requires_staging", message: "Adds a unique index to an existing table that the previous release's writes could violate." }, + drop_type: { compatibility: "requires_staging", message: "Drops a type the previous release may still use." }, + rename_type: { compatibility: "requires_staging", message: "Renames a type or enum value the previous release still uses." }, + truncate: { compatibility: "requires_staging", message: "Truncates a table the previous release is using." }, + delete_rows: { compatibility: "requires_staging", message: "Deletes rows from a table the previous release is using." }, + + lock_hazard_create_index: { compatibility: "unknown", message: "Builds an index on an existing table without CONCURRENTLY, blocking writes for the whole build." }, + lock_hazard_validated_foreign_key: { compatibility: "unknown", message: "Adds a validated foreign key to an existing table, scanning it while writes are blocked. Add it NOT VALID instead." }, + lock_hazard_validate_constraint: { compatibility: "unknown", message: "Validates a constraint by scanning the whole table." }, + lock_hazard_set_not_null: { compatibility: "unknown", message: "Sets NOT NULL on a column added in this migration, scanning the table under an exclusive lock." }, + lock_hazard_table_rewrite: { compatibility: "unknown", message: "Adds a column whose default, identity, or generated expression rewrites the whole table under an exclusive lock." }, + lock_hazard_add_column_constraint: { compatibility: "unknown", message: "Adds a column with an inline constraint, which scans or indexes the table under an exclusive lock." }, + lock_hazard_data_backfill: { compatibility: "unknown", message: "Rewrites rows of an existing table; duration and row locks grow with table size, and the previous release keeps writing the old shape." }, + lock_hazard_copy_into_new_table: { compatibility: "unknown", message: "Copies rows from live tables into a new table; duration grows with the source while this migration's locks are held." }, + lock_hazard_lock_table: { compatibility: "unknown", message: "Takes an explicit table lock." }, + + data_change_existing_table: { compatibility: "unknown", message: "Inserts rows into a table the previous release is using." }, + set_default: { compatibility: "unknown", message: "Changes the default the previous release receives when it inserts." }, + drop_constraint: { compatibility: "unknown", message: "Drops a constraint; the previous release may rely on it for upserts or cascades." }, + drop_index: { compatibility: "unknown", message: "Drops an index; the previous release may rely on it for upserts or for query speed." }, + drop_object: { compatibility: "unknown", message: "Drops a database object the previous release may depend on." }, + add_constraint_other: { compatibility: "unknown", message: "Adds a constraint the classifier cannot prove safe." }, + replace_function: { compatibility: "unknown", message: "Replaces a function, which can change behaviour the previous release depends on." }, + replace_view: { compatibility: "unknown", message: "Replaces or materializes a view." }, + create_trigger_existing_table: { compatibility: "unknown", message: "Adds a trigger to an existing table, changing what the previous release's writes do." }, + create_table_as: { compatibility: "unknown", message: "Creates a table from a query; its duration grows with the source tables." }, + do_block_procedural: { compatibility: "unknown", message: "Runs a procedural DO block the classifier does not interpret." }, + unrecognized_alter_table_action: { compatibility: "unknown", message: "Uses an ALTER TABLE action the classifier does not recognize." }, + unrecognized_statement: { compatibility: "unknown", message: "Uses a statement the classifier does not recognize." }, + no_statements: { compatibility: "unknown", message: "Contains no SQL statements." }, + invalid_override: { compatibility: "unknown", message: "Has a malformed migration-compat override." }, +} as const satisfies Record; + +export type MigrationCompatReasonCode = keyof typeof REASONS; + +export const MIGRATION_COMPAT_REASON_CODES = Object.keys(REASONS) as MigrationCompatReasonCode[]; + +export type MigrationCompatReason = { + readonly code: MigrationCompatReasonCode; + readonly compatibility: MigrationCompatibility; + readonly message: string; + /** 1-based positions of the top-level statements that produced this reason. */ + readonly statements: readonly number[]; +}; + +export type MigrationCompatOverride = { + readonly compatibility: Exclude; + readonly justification: string; +}; + +export type MigrationCompatResult = { + /** The verdict a deployer acts on: the override when one is present and valid. */ + readonly compatibility: MigrationCompatibility; + /** The classifier's own verdict, before any override. */ + readonly classified: MigrationCompatibility; + readonly reasons: readonly MigrationCompatReason[]; + readonly override: MigrationCompatOverride | null; + /** Set when the file carries an override directive that cannot be honoured. */ + readonly overrideError: string | null; + /** True when a statement uses CONCURRENTLY, which cannot run in a transaction. */ + readonly usesConcurrently: boolean; +}; + +export type MigrationCompatOptions = { + /** + * Tables created by the migrations that apply before this one. Without it, + * CREATE TABLE IF NOT EXISTS is never proof that a table is new. + */ + readonly existingTables?: ReadonlySet; +}; + +type Statement = { + /** Lowercase, comments removed, literals masked as '?', dollar bodies as $?$. */ + readonly text: string; + readonly dollarBodies: readonly string[]; +}; + +type Context = { + readonly existingTables: ReadonlySet | null; + readonly createdTables: Set; + readonly addedColumns: Set; +}; + +const IDENT = String.raw`(?:"[^"]+"|[a-z_][a-z0-9_$]*)`; +const QUALIFIED = String.raw`(?:${IDENT}\.)?${IDENT}`; + +function dollarTagAt(sql: string, index: number): string | null { + const match = /^\$(?:[A-Za-z_][A-Za-z0-9_]*)?\$/.exec(sql.slice(index, index + 130)); + return match ? match[0] : null; +} + +/** Splits SQL into statements and normalizes each for keyword matching. */ +function parseStatements(sql: string): Statement[] { + const statements: Statement[] = []; + let text = ""; + let dollarBodies: string[] = []; + const flush = () => { + const normalized = text.replace(/\s+/g, " ").trim(); + if (normalized) statements.push({ text: normalized, dollarBodies }); + text = ""; + dollarBodies = []; + }; + for (let i = 0; i < sql.length;) { + const char = sql[i]; + const next = sql[i + 1]; + if (char === "-" && next === "-") { + const end = sql.indexOf("\n", i); + const comment = sql.slice(i, end === -1 ? sql.length : end); + // Drizzle's separator is a comment; it always ends a statement. + if (/^-->\s*statement-breakpoint/.test(comment)) flush(); + else text += " "; + i = end === -1 ? sql.length : end; + } else if (char === "/" && next === "*") { + let depth = 1; + i += 2; + while (i < sql.length && depth > 0) { + if (sql[i] === "/" && sql[i + 1] === "*") { depth += 1; i += 2; } + else if (sql[i] === "*" && sql[i + 1] === "/") { depth -= 1; i += 2; } + else i += 1; + } + text += " "; + } else if (char === "'") { + // E'...' strings escape quotes with a backslash. + const escapes = /(?:^|[^a-z0-9_$])e$/.test(text); + i += 1; + while (i < sql.length) { + if (escapes && sql[i] === "\\") i += 2; + else if (sql[i] === "'" && sql[i + 1] === "'") i += 2; + else if (sql[i] === "'") break; + else i += 1; + } + i += 1; + text += "'?'"; + } else if (char === '"') { + // A doubled quote is a literal quote inside the identifier. + let name = ""; + i += 1; + while (i < sql.length) { + if (sql[i] === '"' && sql[i + 1] === '"') { name += '"'; i += 2; } + else if (sql[i] === '"') break; + else { name += sql[i]; i += 1; } + } + i += 1; + // Plain lowercase names are equivalent quoted or not. Any other name + // stays quoted, with its own quotes encoded so it remains one token. + text += /^[a-z_][a-z0-9_$]*$/.test(name) ? name : `"${name.replaceAll("%", "%25").replaceAll('"', "%22")}"`; + } else if (char === "$" && dollarTagAt(sql, i)) { + const tag = dollarTagAt(sql, i)!; + const end = sql.indexOf(tag, i + tag.length); + dollarBodies.push(sql.slice(i + tag.length, end === -1 ? sql.length : end)); + text += " $?$ "; + i = end === -1 ? sql.length : end + tag.length; + } else if (char === ";") { + flush(); + i += 1; + } else { + text += char.toLowerCase(); + i += 1; + } + } + flush(); + return statements; +} + +function tableName(name: string): string { + return name.replace(/^only /, "").replace(/^public\./, ""); +} + +/** Returns the index after the parenthesis group that opens at `start`, or -1. */ +function closingParen(text: string, start: number): number { + let depth = 0; + for (let i = start; i < text.length; i += 1) { + if (text[i] === "(") depth += 1; + else if (text[i] === ")") { + depth -= 1; + if (depth === 0) return i + 1; + } + } + return -1; +} + +function splitTopLevelCommas(text: string): string[] { + const parts: string[] = []; + let depth = 0; + let current = ""; + for (const char of text) { + if (char === "(" || char === "[") depth += 1; + else if (char === ")" || char === "]") depth -= 1; + if (char === "," && depth === 0) { + parts.push(current.trim()); + current = ""; + } else current += char; + } + if (current.trim()) parts.push(current.trim()); + return parts; +} + +// A default that PostgreSQL stores in the catalog without rewriting the table: +// a literal, or a non-volatile timestamp function. +const CONSTANT_DEFAULT = new RegExp( + String.raw`^\(?(?:'\?'|-?\d+(?:\.\d+)?|true|false|null|now\(\)|current_timestamp|array\[\])\)?` + + String.raw`(?:::[a-z_][a-z0-9_ ]*(?:\(\d+(?:, ?\d+)?\))?(?:\[\])?)*$`, +); + +function classifyAddColumn(table: string, column: string, definition: string, context: Context): MigrationCompatReasonCode { + if (/\bgenerated\b|\b(?:small|big)?serial\b/.test(definition)) return "lock_hazard_table_rewrite"; + if (/\b(?:references|unique|primary key|check|exclude)\b/.test(definition)) return "lock_hazard_add_column_constraint"; + const notNull = /\bnot null\b/.test(definition); + const defaultMatch = /\bdefault (.+?)(?: not null| null)?$/.exec(definition); + if (defaultMatch && !CONSTANT_DEFAULT.test(defaultMatch[1].trim())) return "lock_hazard_table_rewrite"; + if (notNull && !defaultMatch) return "add_not_null_column_without_default"; + context.addedColumns.add(`${table}.${column}`); + return "add_column"; +} + +function classifyAlterTableAction(table: string, action: string, context: Context): MigrationCompatReasonCode { + let match: RegExpExecArray | null; + if ((match = new RegExp(String.raw`^add (?:constraint ${IDENT} )?(foreign key|check|unique|primary key|exclude)\b(.*)$`).exec(action))) { + if (/\bnot valid$/.test(match[2])) return "add_constraint_not_valid"; + if (match[1] === "foreign key") return "lock_hazard_validated_foreign_key"; + if (match[1] === "check") return "add_validated_check"; + if (match[1] === "exclude" || /\busing index\b/.test(match[2])) return "add_constraint_other"; + return "add_unique_constraint"; + } + if (/^add constraint\b/.test(action)) return "add_constraint_other"; + if ((match = new RegExp(String.raw`^add (?:column )?(?:if not exists )?(${IDENT}) (.+)$`).exec(action))) { + return classifyAddColumn(table, match[1], match[2], context); + } + if (/^drop constraint\b/.test(action)) return "drop_constraint"; + if (/^drop\b/.test(action)) return "drop_column"; + if (/^rename constraint\b/.test(action)) return "unrecognized_alter_table_action"; + if (/^rename to\b/.test(action)) return "rename_table"; + if (/^rename\b/.test(action)) return "rename_column"; + if (/^validate constraint\b/.test(action)) return "lock_hazard_validate_constraint"; + if ((match = new RegExp(String.raw`^alter (?:column )?(${IDENT}) (.+)$`).exec(action))) { + const change = match[2]; + if (/^(?:set data )?type\b/.test(change)) return "alter_column_type"; + if (change === "set not null") { + return context.addedColumns.has(`${table}.${match[1]}`) ? "lock_hazard_set_not_null" : "set_not_null"; + } + if (change === "drop not null") return "drop_not_null"; + if (/^set default\b/.test(change)) return "set_default"; + if (change === "drop default") return "drop_default"; + } + return "unrecognized_alter_table_action"; +} + +function classifyDoBlock(statement: Statement, context: Context, nested: boolean): MigrationCompatReasonCode[] { + if (nested || statement.dollarBodies.length !== 1 || !/^do (?:language plpgsql )?\$\?\$(?: language plpgsql)?$/.test(statement.text)) { + return ["do_block_procedural"]; + } + const pieces = parseStatements(statement.dollarBodies[0]); + const procedural = pieces.length === 0 || !/^begin\b/.test(pieces[0].text) || + pieces.some((piece) => /^(?:begin )?(?:declare|loop|for|foreach|while|execute|perform|raise|return|else|elsif)\b/.test(piece.text) || + /(?:^| )then (?:execute|perform|raise|return|loop|for|foreach|while)\b/.test(piece.text)); + if (procedural) { + const codes: MigrationCompatReasonCode[] = ["do_block_procedural"]; + if (pieces.some((piece) => /\b(?:update|insert into|delete from)\b/.test(piece.text))) codes.push("lock_hazard_data_backfill"); + return codes; + } + + // What remains is a guarded sequence: BEGIN ; ... [EXCEPTION WHEN ... + // THEN NULL;] END, where may sit inside IF [NOT] EXISTS (...) THEN ... + // END IF. A guard only makes a statement conditional, so the statement is + // classified as if it always runs. + const codes: MigrationCompatReasonCode[] = []; + let depth = 0; + let closed = false; + for (let index = 0; index < pieces.length; index += 1) { + let text = pieces[index].text; + if (index === 0) text = text.replace(/^begin\b ?/, ""); + if (closed) return ["do_block_procedural"]; + while (/^if\b/.test(text)) { + let conditionEnd = -1; + const exists = /^if (?:not )?exists ?\(/.exec(text); + const regclass = /^if to_regclass\('\?'\) is (?:not )?null/.exec(text); + if (exists) conditionEnd = closingParen(text, exists[0].length - 1); + else if (regclass) conditionEnd = regclass[0].length; + if (conditionEnd === -1 || !text.slice(conditionEnd).startsWith(" then ")) return ["do_block_procedural"]; + text = text.slice(conditionEnd + " then ".length); + depth += 1; + } + if (text === "" || text === "null") continue; + if (text === "end if") { + depth -= 1; + if (depth < 0) return ["do_block_procedural"]; + } else if (text === "end") { + if (depth !== 0 || index !== pieces.length - 1) return ["do_block_procedural"]; + closed = true; + } else if (/^exception when [a-z_ ]+ then null$/.test(text)) { + if (depth !== 0) return ["do_block_procedural"]; + } else { + codes.push(...classifyStatement({ text, dollarBodies: pieces[index].dollarBodies }, context, true)); + } + } + if (!closed || codes.length === 0) return ["do_block_procedural"]; + return codes; +} + +function classifyStatement(statement: Statement, context: Context, nested = false): MigrationCompatReasonCode[] { + const text = statement.text; + const isNew = (name: string) => context.createdTables.has(tableName(name)); + let match: RegExpExecArray | null; + + if (/^do\b/.test(text)) return classifyDoBlock(statement, context, nested); + + if ((match = new RegExp(String.raw`^create (?:(?:global |local )?(temp|temporary) |unlogged )?table (if not exists )?(${QUALIFIED})(.*)$`).exec(text))) { + const temporary = Boolean(match[1]); + const name = tableName(match[3]); + // Plain CREATE TABLE fails when the table exists, and a temporary table + // belongs to this session. IF NOT EXISTS proves nothing by itself: the + // table is new only when no earlier migration created it. + const provenNew = temporary || !match[2] || (context.existingTables !== null && !context.existingTables.has(name)); + let rest = match[4].trim(); + if (rest.startsWith("(")) { + const end = closingParen(rest, 0); + rest = end === -1 ? rest : rest.slice(end); + } + if (/\b(?:partition of|inherits|like)\b/.test(rest)) return ["unrecognized_statement"]; + if (/\bas (?:select|with|values|table)\b/.test(rest)) { + // A temporary copy only reads; it is still registered as migration-owned. + if (temporary) context.createdTables.add(name); + return ["create_table_as"]; + } + if (!provenNew) return ["create_table_if_not_exists"]; + context.createdTables.add(name); + return [temporary ? "create_temp_table" : "create_table"]; + } + + if ((match = new RegExp(String.raw`^create (unique )?index (concurrently )?(?:if not exists )?(?:${IDENT} )?on (?:only )?(${QUALIFIED})[ (]`).exec(text))) { + if (isNew(match[3])) return ["create_index_new_table"]; + if (match[1]) return ["create_unique_index_existing_table"]; + return [match[2] ? "create_index_concurrently" : "lock_hazard_create_index"]; + } + + if ((match = new RegExp(String.raw`^alter table (?:if exists )?(?:only )?(${QUALIFIED}) (.+)$`).exec(text))) { + if (isNew(match[1])) return ["alter_new_table"]; + const table = tableName(match[1]); + return splitTopLevelCommas(match[2]).map((action) => classifyAlterTableAction(table, action, context)); + } + + if ((match = new RegExp(String.raw`^drop table (?:if exists )?(.+?)(?: cascade| restrict)?$`).exec(text))) { + return splitTopLevelCommas(match[1]).every(isNew) ? ["alter_new_table"] : ["drop_table"]; + } + if (/^drop index\b/.test(text)) return ["drop_index"]; + if (/^drop (?:type|domain)\b/.test(text)) return ["drop_type"]; + if (/^drop\b/.test(text)) return ["drop_object"]; + if (/^truncate\b/.test(text)) return ["truncate"]; + + if ((match = new RegExp(String.raw`^delete from (?:only )?(${QUALIFIED})\b`).exec(text))) { + return [isNew(match[1]) ? "seed_new_table" : "delete_rows"]; + } + if ((match = new RegExp(String.raw`^update (?:only )?(${QUALIFIED})\b`).exec(text))) { + return [isNew(match[1]) ? "seed_new_table" : "lock_hazard_data_backfill"]; + } + if ((match = new RegExp(String.raw`^insert into (${QUALIFIED})\b(.*)$`).exec(text))) { + if (!isNew(match[1])) return ["data_change_existing_table"]; + // Copying from live tables can run long while this migration's locks are held. + return [/\b(?:select|with|table)\b/.test(match[2]) ? "lock_hazard_copy_into_new_table" : "seed_new_table"]; + } + if (/^with\b/.test(text)) { + if (/\bdelete from\b/.test(text)) return ["delete_rows"]; + if (/\b(?:update|insert into)\b/.test(text)) return ["lock_hazard_data_backfill"]; + return ["unrecognized_statement"]; + } + + if (/^create type\b/.test(text)) return ["create_type"]; + if (new RegExp(String.raw`^alter type ${QUALIFIED} add value\b`).test(text)) return ["add_enum_value"]; + if (new RegExp(String.raw`^alter type ${QUALIFIED} rename\b`).test(text)) return ["rename_type"]; + if (/^create (?:temp |temporary )?sequence\b/.test(text)) return ["create_sequence"]; + if (/^create extension\b/.test(text)) return ["create_extension"]; + if (/^create (?:function|procedure)\b/.test(text)) return ["create_function"]; + if (/^create or replace (?:function|procedure)\b/.test(text)) return ["replace_function"]; + if (/^create (?:temp |temporary )?view\b/.test(text)) return ["create_view"]; + if (/^create (?:or replace |materialized )(?:temp |temporary )?view\b/.test(text)) return ["replace_view"]; + if ((match = new RegExp(String.raw`^create (?:or replace )?(?:constraint )?trigger ${IDENT} .*? on (?:only )?(${QUALIFIED})\b`).exec(text))) { + return [isNew(match[1]) && !/^create or replace\b/.test(text) ? "create_trigger_new_table" : "create_trigger_existing_table"]; + } + if (/^comment on\b/.test(text)) return ["comment"]; + if (/^create statistics\b/.test(text)) return ["create_statistics"]; + if (/^analyze\b/.test(text)) return ["analyze"]; + if (text === "select 1") return ["no_op"]; + if (/^lock (?:table )?/.test(text)) return ["lock_hazard_lock_table"]; + return ["unrecognized_statement"]; +} + +function worst(values: readonly MigrationCompatibility[]): MigrationCompatibility { + if (values.includes("requires_staging")) return "requires_staging"; + if (values.includes("unknown")) return "unknown"; + return "online_compatible"; +} + +type OverrideParse = { readonly override: MigrationCompatOverride | null; readonly error: string | null }; + +/** + * Reads the override directive from the migration file's leading comment block. + * Nothing outside the file can supply or change an override. + */ +export function parseMigrationCompatOverride(sql: string): OverrideParse { + const lines = sql.split(/\r?\n/); + const directive = new RegExp(String.raw`^\s*--\s*${MIGRATION_COMPAT_DIRECTIVE}(?![\w-])(.*)$`); + const found: string[] = []; + let leading = true; + let misplaced = false; + for (const line of lines) { + const trimmed = line.trim(); + if (leading && trimmed !== "" && (!trimmed.startsWith("--") || trimmed.startsWith("-->"))) leading = false; + if (!line.includes(MIGRATION_COMPAT_DIRECTIVE)) continue; + const match = leading ? directive.exec(line) : null; + if (match) found.push(match[1]); + else misplaced = true; + } + const usage = `Write it as the file's first lines: "-- ${MIGRATION_COMPAT_DIRECTIVE} online_compatible: " (or requires_staging).`; + if (misplaced) return { override: null, error: `The ${MIGRATION_COMPAT_DIRECTIVE} directive must be a "--" comment in the leading comment block, before any SQL. ${usage}` }; + if (found.length === 0) return { override: null, error: null }; + if (found.length > 1) return { override: null, error: `The file has more than one ${MIGRATION_COMPAT_DIRECTIVE} directive. ${usage}` }; + const match = /^\s+([a-z_]+)\s*(?::\s*(.*))?$/.exec(found[0]); + const compatibility = match?.[1]; + if (!match || (compatibility !== "online_compatible" && compatibility !== "requires_staging")) { + return { override: null, error: `The ${MIGRATION_COMPAT_DIRECTIVE} directive must name online_compatible or requires_staging. ${usage}` }; + } + const justification = (match[2] ?? "").trim(); + if (justification.length < MIGRATION_COMPAT_MIN_JUSTIFICATION_LENGTH) { + return { override: null, error: `The ${MIGRATION_COMPAT_DIRECTIVE} directive needs a justification of at least ${MIGRATION_COMPAT_MIN_JUSTIFICATION_LENGTH} characters after the colon. ${usage}` }; + } + return { override: { compatibility, justification }, error: null }; +} + +/** + * Decides whether a migration can be applied while the previous release's app + * code is still reading and writing the database, without breaking that code + * and without long blocking locks. Anything the classifier does not positively + * recognize is `unknown`. + */ +export function classifyMigrationSql(sql: string, options: MigrationCompatOptions = {}): MigrationCompatResult { + const context: Context = { existingTables: options.existingTables ?? null, createdTables: new Set(), addedColumns: new Set() }; + const statementsByCode = new Map(); + const record = (code: MigrationCompatReasonCode, position: number) => { + const positions = statementsByCode.get(code) ?? []; + if (!positions.includes(position)) positions.push(position); + statementsByCode.set(code, positions); + }; + + const statements = parseStatements(sql); + if (statements.length === 0) record("no_statements", 0); + statements.forEach((statement, index) => { + for (const code of classifyStatement(statement, context)) record(code, index + 1); + }); + const { override, error } = parseMigrationCompatOverride(sql); + if (error) record("invalid_override", 0); + + const reasons: MigrationCompatReason[] = [...statementsByCode].map(([code, positions]) => ({ + code, + compatibility: REASONS[code].compatibility, + message: code === "invalid_override" && error ? error : REASONS[code].message, + statements: positions.filter((position) => position > 0), + })); + const classified = worst(reasons.filter((reason) => reason.code !== "invalid_override").map((reason) => reason.compatibility)); + return { + compatibility: override ? override.compatibility : worst(reasons.map((reason) => reason.compatibility)), + classified, + reasons, + override, + overrideError: error, + usesConcurrently: statements.some((statement) => /\bconcurrently\b/.test(statement.text)), + }; +} + +/** Names of the tables a migration creates or renames to, including inside DO blocks. */ +export function collectCreatedTables(sql: string): string[] { + const names: string[] = []; + const created = new RegExp(String.raw`\bcreate (?:unlogged )?table (?:if not exists )?(${QUALIFIED})`, "g"); + const renamed = new RegExp(String.raw`\brename to (${IDENT})`, "g"); + const visit = (statements: Statement[]) => { + for (const statement of statements) { + for (const match of statement.text.matchAll(created)) names.push(tableName(match[1])); + for (const match of statement.text.matchAll(renamed)) names.push(tableName(match[1])); + for (const body of statement.dollarBodies) visit(parseStatements(body)); + } + }; + visit(parseStatements(sql)); + return names; +} + +/** + * Classifies migrations in apply order. Each one is judged knowing the tables + * that `priorSql` and the earlier entries of `sqls` created. + */ +export function classifyMigrationSequence(sqls: readonly string[], priorSql: readonly string[] = []): MigrationCompatResult[] { + const existingTables = new Set(priorSql.flatMap(collectCreatedTables)); + return sqls.map((sql) => { + const result = classifyMigrationSql(sql, { existingTables }); + for (const table of collectCreatedTables(sql)) existingTables.add(table); + return result; + }); +} + +/** + * Reads, from the source of packages/db/src/client.ts, the migration files the + * migrator runs outside a transaction. Returns null when the list is not found. + */ +export function parseNonTransactionalMigrations(executorSource: string): Set | null { + const match = /MIGRATIONS_WITH_TOP_LEVEL_STATEMENTS\s*=\s*new Set(?:<[^>]*>)?\(\[([\s\S]*?)\]\)/.exec(executorSource); + if (!match) return null; + return new Set([...match[1].matchAll(/"([^"]+\.sql)"/g)].map((entry) => entry[1])); +} + +export type MigrationCompatInput = { readonly fileName: string; readonly sql: string }; + +export type MigrationCompatGuardResult = { + readonly passed: boolean; + /** One entry per migration that blocks the pull request. */ + readonly errors: readonly string[]; + /** Informational lines: staged migrations and accepted overrides. */ + readonly notices: readonly string[]; + readonly results: readonly { readonly fileName: string; readonly result: MigrationCompatResult }[]; +}; + +function describeReasons(result: MigrationCompatResult, compatibility: MigrationCompatibility): string[] { + return result.reasons + .filter((reason) => reason.compatibility === compatibility) + .map((reason) => ` - ${reason.code}${reason.statements.length ? ` (statement ${reason.statements.join(", ")})` : ""}: ${reason.message}`); +} + +export type MigrationCompatGuardOptions = { + /** + * SQL of every migration that applies before the new ones. Without it, + * CREATE TABLE IF NOT EXISTS never counts as creating a new table. + */ + readonly priorMigrationSql?: readonly string[]; + /** + * File names the migrator runs outside a transaction (see + * parseNonTransactionalMigrations). When given, a new migration that uses + * CONCURRENTLY must be in it. + */ + readonly nonTransactionalMigrations?: ReadonlySet; +}; + +/** + * The pull-request guard for migrations a change adds, in apply order. + * `unknown` without a valid override fails. `requires_staging` passes with a + * notice until deployers enforce staging. + */ +export function checkNewMigrationCompatibility( + migrations: readonly MigrationCompatInput[], + options: MigrationCompatGuardOptions = {}, +): MigrationCompatGuardResult { + const errors: string[] = []; + const notices: string[] = []; + const sqls = migrations.map((migration) => migration.sql); + const classified = options.priorMigrationSql + ? classifyMigrationSequence(sqls, options.priorMigrationSql) + : sqls.map((sql) => classifyMigrationSql(sql)); + const results = migrations.map(({ fileName }, index) => ({ fileName, result: classified[index] })); + for (const { fileName, result } of results) { + const baseName = fileName.split("/").pop() ?? fileName; + if (result.usesConcurrently && options.nonTransactionalMigrations && !options.nonTransactionalMigrations.has(baseName)) { + errors.push([ + `${fileName}: uses CONCURRENTLY, which PostgreSQL rejects inside a transaction, and the migrator runs each file in one transaction.`, + ` Add "${baseName}" to MIGRATIONS_WITH_TOP_LEVEL_STATEMENTS in packages/db/src/client.ts.`, + " Make every statement in the file idempotent (IF NOT EXISTS), because a failed run starts again from the first statement.", + ].join("\n")); + } + if (result.overrideError) { + errors.push(`${fileName}: ${result.overrideError}`); + } else if (result.compatibility === "unknown") { + errors.push([ + `${fileName}: cannot tell whether this migration is safe to apply while the previous release is still running.`, + ...describeReasons(result, "unknown"), + " Do one of the following:", + " 1. Rewrite it so every statement is recognized as compatible, for example CREATE INDEX CONCURRENTLY,", + " ADD CONSTRAINT ... NOT VALID, or a nullable column or constant default.", + " 2. Split it into an expand migration now and a contract migration in a later release.", + " 3. If you have verified it yourself, make these the first lines of the file:", + ` -- ${MIGRATION_COMPAT_DIRECTIVE} online_compatible: `, + ` or, if the previous release cannot run against it:`, + ` -- ${MIGRATION_COMPAT_DIRECTIVE} requires_staging: `, + " See \"Migration compatibility with the running release\" in doc/DATABASE.md.", + ].join("\n")); + } else if (result.compatibility === "requires_staging") { + notices.push([ + `${fileName}: requires_staging. The previous release cannot keep running against this migration, so a deployer that may not pause will hold it back.`, + ...(result.override ? [` - override: ${result.override.justification}`] : describeReasons(result, "requires_staging")), + " This does not fail the check yet. Prefer an expand migration now and the contract migration in a later release.", + ].join("\n")); + } else if (result.override) { + notices.push(`${fileName}: online_compatible by override (classifier said ${result.classified}): ${result.override.justification}`); + } + } + return { passed: errors.length === 0, errors, notices, results }; +} + +export type MigrationCompatSummary = { + readonly total: number; + readonly byCompatibility: Record; + readonly overrides: number; + /** Migrations per blocking reason code, most frequent first. */ + readonly blockingReasons: readonly { readonly code: MigrationCompatReasonCode; readonly compatibility: MigrationCompatibility; readonly migrations: number }[]; +}; + +export function summarizeMigrationCompatibility(results: readonly MigrationCompatResult[]): MigrationCompatSummary { + const byCompatibility: Record = { online_compatible: 0, requires_staging: 0, unknown: 0 }; + const counts = new Map(); + for (const result of results) { + byCompatibility[result.compatibility] += 1; + for (const reason of result.reasons) { + if (reason.compatibility !== "online_compatible") counts.set(reason.code, (counts.get(reason.code) ?? 0) + 1); + } + } + return { + total: results.length, + byCompatibility, + overrides: results.filter((result) => result.override).length, + blockingReasons: [...counts] + .map(([code, migrations]) => ({ code, compatibility: REASONS[code].compatibility, migrations })) + .sort((left, right) => right.migrations - left.migrations || left.code.localeCompare(right.code)), + }; +} diff --git a/scripts/cloud-migrator-artifacts.mjs b/scripts/cloud-migrator-artifacts.mjs index 5bd804ce8ca..b8ea476bc08 100644 --- a/scripts/cloud-migrator-artifacts.mjs +++ b/scripts/cloud-migrator-artifacts.mjs @@ -7,6 +7,7 @@ import { copyFileSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from " import os from "node:os"; import path from "node:path"; import { pathToFileURL } from "node:url"; +import { gunzipSync } from "node:zlib"; import { assertMetadata, tarManifest, versionFor } from "./preview-artifacts.mjs"; export const artifactBase = "https://d1p6rlowie26tp.cloudfront.net/cloud-migrators/v1"; @@ -30,10 +31,111 @@ function assertDescriptor(pin, extension) { } } +const compatibilityValues = ["online_compatible", "requires_staging", "unknown"]; +const migrationsPrefix = "package/dist/migrations/"; + +/** Regular files under `prefix` in a package archive, by archive path. */ +function tarFiles(bytes, prefix) { + const tar = gunzipSync(bytes, { maxOutputLength: 128 * 1024 * 1024 }); + const files = new Map(); + for (let offset = 0; offset + 512 <= tar.length;) { + const h = tar.subarray(offset, offset + 512); + if (h.every((v) => v === 0)) break; + const field = (start, size) => h.subarray(start, start + size).toString("utf8").split("\0")[0].trim(); + const sizeText = field(124, 12); + if (!/^[0-7]+$/.test(sizeText)) throw new Error("Invalid package archive."); + const size = Number.parseInt(sizeText, 8); + if (offset + 512 + size > tar.length) throw new Error("Truncated package archive."); + const name = `${field(345, 155) ? field(345, 155) + "/" : ""}${field(0, 100)}`; + if ([0, 48].includes(h[156]) && name.startsWith(prefix)) { + if (files.has(name)) throw new Error("Duplicate package archive entry."); + files.set(name, tar.subarray(offset + 512, offset + 512 + size)); + } + offset += 512 + Math.ceil(size / 512) * 512; + } + return files; +} + +/** The journal of the packed migrator, in apply order, with each file's bytes. */ +function packedMigrations(dbBytes) { + const files = tarFiles(dbBytes, migrationsPrefix); + const journal = files.get(`${migrationsPrefix}meta/_journal.json`); + if (!journal) throw new Error("Migrator package has no migration journal."); + const entries = JSON.parse(journal.toString("utf8")).entries; + if (!Array.isArray(entries) || entries.length === 0) throw new Error("Migrator package has an empty migration journal."); + // The migrator's apply order: journal idx, then file name. The shipped + // journal repeats an idx, so idx alone does not identify a migration. + return [...entries].sort((left, right) => left.idx - right.idx || (String(left.tag) < String(right.tag) ? -1 : 1)).map(({ idx, tag }) => { + const sql = files.get(`${migrationsPrefix}${tag}.sql`); + if (!sql) throw new Error(`Migrator package is missing migration ${tag}.`); + // The same digest the migrator records in the database's migration history. + return { idx, tag, sql, sqlSha256: createHash("sha256").update(sql).digest("hex") }; + }); +} + +/** + * Whether each packed migration can be applied while the previous release is + * still running. `classify` is classifyMigrationSequence from @paperclipai/db: + * it takes the SQL of every migration in apply order. The overrides it honours + * come only from the packed SQL itself. + */ +export function migrationCompatibility(dbBytes, classify) { + const packed = packedMigrations(dbBytes); + const results = classify(packed.map(({ sql }) => sql.toString("utf8"))); + if (!Array.isArray(results) || results.length !== packed.length) throw new Error("The classifier did not return one result per migration."); + return { + version: 1, + migrations: packed.map(({ idx, tag, sqlSha256 }, position) => { + const result = results[position]; + return { + idx, tag, sqlSha256, + compatibility: result.compatibility, + classified: result.classified, + override: Boolean(result.override), + ...(result.override ? { overrideJustification: result.override.justification } : {}), + reasons: result.reasons.map(({ code, compatibility, message, statements }) => ({ code, compatibility, message, statements: [...statements] })), + }; + }), + }; +} + +function assertMigrationCompatibility(value) { + const fail = () => { throw new Error("Invalid migration compatibility data."); }; + if (value?.version !== 1 || !Array.isArray(value.migrations) || value.migrations.length === 0) fail(); + const text = (candidate) => typeof candidate === "string" && candidate.length > 0 && candidate.length <= 2000; + value.migrations.forEach((migration, position) => { + const previous = value.migrations[position - 1]; + if (!Number.isSafeInteger(migration?.idx) || migration.idx < 0 || + (previous && (migration.idx < previous.idx || (migration.idx === previous.idx && !(migration.tag > previous.tag)))) || + typeof migration.tag !== "string" || !/^\d{4}_[A-Za-z0-9_]+$/.test(migration.tag) || + typeof migration.sqlSha256 !== "string" || !/^[0-9a-f]{64}$/.test(migration.sqlSha256) || + !compatibilityValues.includes(migration.compatibility) || !compatibilityValues.includes(migration.classified) || + typeof migration.override !== "boolean" || !Array.isArray(migration.reasons)) fail(); + // An override always carries its justification and never yields `unknown`. + if (migration.override ? !text(migration.overrideJustification) || migration.compatibility === "unknown" : migration.overrideJustification !== undefined) fail(); + for (const reason of migration.reasons) { + if (typeof reason?.code !== "string" || !/^[a-z_]+$/.test(reason.code) || !compatibilityValues.includes(reason.compatibility) || !text(reason.message) || + !Array.isArray(reason.statements) || !reason.statements.every((statement) => Number.isSafeInteger(statement) && statement > 0)) fail(); + } + }); +} + export function assertManifest(manifest, sha) { if (manifest?.version !== 1 || manifest.sourceSha !== sha || manifest.packageVersion !== versionFor(sha)) throw new Error("Artifact source identity mismatch."); for (const name of names) assertDescriptor(manifest.packages?.[name], "tgz"); assertDescriptor(manifest.lockfile, "json"); + // Additive since the classifier shipped; manifests published earlier lack it. + if (manifest.migrationCompatibility !== undefined) assertMigrationCompatibility(manifest.migrationCompatibility); +} + +/** The published classification must describe exactly the migrations in db.tgz. */ +function assertCompatibilityMatchesPackage(manifest, dbBytes) { + const published = manifest.migrationCompatibility?.migrations; + const packed = packedMigrations(dbBytes); + if (!published || published.length !== packed.length || packed.some(({ idx, tag, sqlSha256 }, position) => + published[position].idx !== idx || published[position].tag !== tag || published[position].sqlSha256 !== sqlSha256)) { + throw new Error("Migration compatibility data does not match the packed migrations."); + } } export function assertLockfile(lock, manifest) { @@ -61,8 +163,9 @@ export function assertLockfile(lock, manifest) { } } -export function buildBundle(directory, sha, { exec = execFileSync } = {}) { +export function buildBundle(directory, sha, { exec = execFileSync, classify } = {}) { versionFor(sha); + if (typeof classify !== "function") throw new Error("buildBundle needs the migration compatibility classifier."); directory = path.resolve(directory); const packages = {}; for (const name of names) { @@ -83,8 +186,11 @@ export function buildBundle(directory, sha, { exec = execFileSync } = {}) { lock.packages[""].dependencies = { "@paperclipai/db": versionFor(sha) }; for (const name of names) lock.packages[`node_modules/@paperclipai/${name}`].resolved = packages[name].url; const lockBytes = Buffer.from(JSON.stringify(lock) + "\n"); - const manifest = { version: 1, sourceSha: sha, packageVersion: versionFor(sha), packages, lockfile: descriptor(lockBytes, "json") }; + const dbBytes = readFileSync(path.join(directory, "db.tgz")); + const manifest = { version: 1, sourceSha: sha, packageVersion: versionFor(sha), packages, lockfile: descriptor(lockBytes, "json"), + migrationCompatibility: migrationCompatibility(dbBytes, classify) }; assertManifest(manifest, sha); + assertCompatibilityMatchesPackage(manifest, dbBytes); assertLockfile(lock, manifest); writeFileSync(path.join(directory, "package-lock.json"), lockBytes); writeFileSync(path.join(directory, "manifest.json"), JSON.stringify(manifest) + "\n"); @@ -103,6 +209,7 @@ export function validateBundle(directory, sha) { const bytes = readFileSync(path.join(directory, `${name}.tgz`)); verifyBytes(bytes, manifest.packages[name]); assertMetadata(tarManifest(bytes), `@paperclipai/${name}`, sha); + if (name === "db") assertCompatibilityMatchesPackage(manifest, bytes); } const bytes = readFileSync(path.join(directory, "package-lock.json")); verifyBytes(bytes, manifest.lockfile); @@ -160,6 +267,7 @@ export async function verifyPublished(sha, fetchImpl = fetch, { verifyProvenance const bytes = await download(manifest.packages[name].url, fetchImpl); verifyBytes(bytes, manifest.packages[name]); assertMetadata(tarManifest(bytes), `@paperclipai/${name}`, sha); + if (name === "db" && manifest.migrationCompatibility !== undefined) assertCompatibilityMatchesPackage(manifest, bytes); })); const lock = await download(manifest.lockfile.url, fetchImpl); verifyBytes(lock, manifest.lockfile); @@ -200,7 +308,12 @@ export async function publishBundle(directory, sha, { exec = execFileSync, fetch if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { const [command, directory, sha] = process.argv.slice(2); try { - if (command === "build") buildBundle(directory, sha); + if (command === "build") { + // Type-stripped by Node; the module has no imports, so the build job + // needs nothing beyond the checkout. + const { classifyMigrationSequence } = await import(new URL("../packages/db/src/migration-compatibility.ts", import.meta.url).href); + buildBundle(directory, sha, { classify: classifyMigrationSequence }); + } else if (command === "validate") validateBundle(directory, sha); else if (command === "verify-install") verifyInstall(directory, sha); else if (command === "publish") await publishBundle(directory, sha); diff --git a/scripts/cloud-migrator-artifacts.test.mjs b/scripts/cloud-migrator-artifacts.test.mjs index 61374f32060..59cc4a7b4c0 100644 --- a/scripts/cloud-migrator-artifacts.test.mjs +++ b/scripts/cloud-migrator-artifacts.test.mjs @@ -7,25 +7,45 @@ import { mkdtempSync, readFileSync, writeFileSync, rmSync } from "node:fs"; import os from "node:os"; import path from "node:path"; import { gzipSync } from "node:zlib"; -import { artifactBase, assertManifest, assertLockfile, buildBundle, descriptor, validateBundle, verifyPublished, publishBundle } from "./cloud-migrator-artifacts.mjs"; +import { createHash } from "node:crypto"; +import { artifactBase, assertManifest, assertLockfile, buildBundle, descriptor, migrationCompatibility, validateBundle, verifyPublished, publishBundle } from "./cloud-migrator-artifacts.mjs"; import { previewManifest, versionFor } from "./preview-artifacts.mjs"; +import { classifyMigrationSequence as classify } from "../packages/db/src/migration-compatibility.ts"; const sha = "a".repeat(40); +const migrations = { + "0000_first": 'CREATE TABLE IF NOT EXISTS "widgets" ("id" uuid PRIMARY KEY NOT NULL);\n--> statement-breakpoint\nCREATE INDEX "widgets_idx" ON "widgets" ("id");\n', + // IF NOT EXISTS does not make a table from an earlier migration new again. + "0001_second": 'CREATE TABLE IF NOT EXISTS "widgets" ("id" uuid);\n--> statement-breakpoint\nCREATE INDEX "widgets_again_idx" ON "widgets" ("id");\n', + "0002_third": '-- paperclip:migration-compat online_compatible: no release since 0.2 reads issues.legacy\nALTER TABLE "issues" DROP COLUMN "legacy";\n', +}; +function tarEntry(name, content) { + const bytes = Buffer.from(content); + const header = Buffer.alloc(512); + header.write(name); header.write(bytes.length.toString(8).padStart(11, "0"), 124, 11); header[156] = 48; + // A real tar header, so npm can install this fixture as well as inspect it. + header.fill(32, 148, 156); + const sum = header.reduce((a, b) => a + b, 0); + header.write(sum.toString(8).padStart(6, "0") + "\0 ", 148, 8); + const padded = Buffer.alloc(Math.ceil(bytes.length / 512) * 512); bytes.copy(padded); + return Buffer.concat([header, padded]); +} +function dbArchive(files = migrations) { + const journal = { version: "7", dialect: "postgresql", entries: Object.keys(files).map((tag, idx) => ({ idx, version: "7", when: idx, tag, breakpoints: true })) }; + return gzipSync(Buffer.concat([ + tarEntry("package/package.json", JSON.stringify(previewManifest({ name: "@paperclipai/db", dependencies: {} }, sha))), + tarEntry("package/dist/migrations/meta/_journal.json", JSON.stringify(journal)), + ...Object.entries(files).map(([tag, sql]) => tarEntry(`package/dist/migrations/${tag}.sql`, sql)), + Buffer.alloc(1024), + ])); +} function fixture(t) { const dir = mkdtempSync(path.join(os.tmpdir(), "migrator-artifact-test-")); t.after(() => rmSync(dir, { recursive: true, force: true })); - for (const name of ["db", "shared"]) { - const bytes = Buffer.from(JSON.stringify(previewManifest({ name: `@paperclipai/${name}`, dependencies: {} }, sha))); - const header = Buffer.alloc(512); - header.write("package/package.json"); header.write(bytes.length.toString(8).padStart(11, "0"), 124, 11); header[156] = 48; - // A real tar header, so npm can install this fixture as well as inspect it. - header.fill(32, 148, 156); - const sum = header.reduce((a, b) => a + b, 0); - header.write(sum.toString(8).padStart(6, "0") + "\0 ", 148, 8); - const padded = Buffer.alloc(Math.ceil(bytes.length / 512) * 512); bytes.copy(padded); - writeFileSync(path.join(dir, `${name}.tgz`), gzipSync(Buffer.concat([header, padded, Buffer.alloc(1024)]))); - } - const manifest = buildBundle(dir, sha, { exec: (cmd, args, options) => { + writeFileSync(path.join(dir, "db.tgz"), dbArchive()); + writeFileSync(path.join(dir, "shared.tgz"), gzipSync(Buffer.concat([ + tarEntry("package/package.json", JSON.stringify(previewManifest({ name: "@paperclipai/shared", dependencies: {} }, sha))), Buffer.alloc(1024)]))); + const manifest = buildBundle(dir, sha, { classify, exec: (cmd, args, options) => { assert.equal(cmd, "npm"); assert.ok(args.includes("--ignore-scripts")); const localRoot = JSON.parse(readFileSync(path.join(options.cwd, "package.json"))); assert.deepEqual(localRoot.dependencies, { "@paperclipai/db": "file:db.tgz", "@paperclipai/shared": "file:shared.tgz" }); @@ -53,6 +73,67 @@ test("bundle pins the exact source pair and complete lockfile without new npm lo assert.deepEqual(await verifyPublished(sha, fetchImpl), manifest); }); +test("manifest publishes how each packed migration behaves under the previous release", (t) => { + const { manifest } = fixture(t); + const digest = (tag) => createHash("sha256").update(migrations[tag]).digest("hex"); + assert.deepEqual(manifest.migrationCompatibility.version, 1); + assert.deepEqual(manifest.migrationCompatibility.migrations.map(({ reasons, ...entry }) => ({ ...entry, reasons: reasons.map((reason) => reason.code) })), [ + { idx: 0, tag: "0000_first", sqlSha256: digest("0000_first"), compatibility: "online_compatible", classified: "online_compatible", override: false, + reasons: ["create_table", "create_index_new_table"] }, + { idx: 1, tag: "0001_second", sqlSha256: digest("0001_second"), compatibility: "unknown", classified: "unknown", override: false, + reasons: ["create_table_if_not_exists", "lock_hazard_create_index"] }, + { idx: 2, tag: "0002_third", sqlSha256: digest("0002_third"), compatibility: "online_compatible", classified: "requires_staging", override: true, + overrideJustification: "no release since 0.2 reads issues.legacy", reasons: ["drop_column"] }, + ]); + const reason = manifest.migrationCompatibility.migrations[1].reasons[1]; + assert.deepEqual(Object.keys(reason), ["code", "compatibility", "message", "statements"]); + assert.deepEqual([reason.compatibility, reason.statements], ["unknown", [2]]); + // Everything a consumer pinned before this field existed is unchanged. + assert.deepEqual(Object.keys(manifest), ["version", "sourceSha", "packageVersion", "packages", "lockfile", "migrationCompatibility"]); + assert.equal(manifest.version, 1); +}); + +test("migration compatibility is optional for earlier manifests and fails closed when malformed or stale", async (t) => { + const { dir, manifest, files, fetchImpl } = fixture(t); + const earlier = structuredClone(manifest); delete earlier.migrationCompatibility; + assert.doesNotThrow(() => assertManifest(earlier, sha)); + for (const mutate of [ + (c) => { c.version = 2; }, + (c) => { c.migrations = []; }, + (c) => { c.migrations[0].compatibility = "safe"; }, + (c) => { c.migrations[0].classified = undefined; }, + (c) => { c.migrations[0].sqlSha256 = "abc"; }, + (c) => { c.migrations[2].idx = 0; }, + (c) => { c.migrations[1].tag = c.migrations[0].tag; c.migrations[1].idx = 0; }, + (c) => { c.migrations[0].override = true; }, + (c) => { c.migrations[2].overrideJustification = ""; }, + (c) => { c.migrations[2].compatibility = "unknown"; }, + (c) => { c.migrations[0].overrideJustification = "without an override"; }, + (c) => { c.migrations[1].reasons[0].statements = [0]; }, + (c) => { c.migrations[1].reasons[1].compatibility = "fine"; }, + (c) => { c.migrations[1].reasons[0].code = 7; }, + ]) { + const bad = structuredClone(manifest); mutate(bad.migrationCompatibility); + assert.throws(() => assertManifest(bad, sha), /Invalid migration compatibility data/); + } + assert.throws(() => buildBundle(dir, sha), /classifier/); + // A well-formed classification of different SQL must not be published or accepted. + const stale = structuredClone(manifest); + stale.migrationCompatibility = migrationCompatibility(dbArchive({ ...migrations, "0001_second": 'DROP TABLE "issues";\n' }), classify); + assertManifest(stale, sha); + files.set(`${artifactBase}/${sha}/manifest.json`, Buffer.from(JSON.stringify(stale))); + await assert.rejects(verifyPublished(sha, fetchImpl), /does not match the packed migrations/); + writeFileSync(path.join(dir, "manifest.json"), JSON.stringify(stale)); + assert.throws(() => validateBundle(dir, sha), /does not match the packed migrations/); + writeFileSync(path.join(dir, "manifest.json"), JSON.stringify(earlier)); + assert.throws(() => validateBundle(dir, sha), /does not match the packed migrations/); + assert.throws(() => migrationCompatibility(dbArchive({}), classify), /empty migration journal/); + assert.throws(() => migrationCompatibility(dbArchive(), () => []), /one result per migration/); + // The shipped journal repeats idx 178; order then follows the file name, as in the migrator. + const repeated = structuredClone(manifest); repeated.migrationCompatibility.migrations[1].idx = 0; + assert.doesNotThrow(() => assertManifest(repeated, sha)); +}); + test("source identity, content hashes, size, and origin fail closed", async (t) => { const { dir, manifest, files, fetchImpl } = fixture(t); for (const mutate of [ @@ -109,7 +190,7 @@ test("download failures and oversized objects never count as available", async ( test("real npm ci installs the new pair from pinned archives with an empty cache", async (t) => { const { dir } = fixture(t); // Real npm resolution uses local archives; neither package version exists on npm. - const manifest = buildBundle(dir, sha); + const manifest = buildBundle(dir, sha, { classify }); const lock = JSON.parse(readFileSync(path.join(dir, "package-lock.json"))); const requests = []; const server = createServer((req, res) => {