diff --git a/packages/cli/src/promote.ts b/packages/cli/src/promote.ts index 2e3d95e..ee7fd0d 100644 --- a/packages/cli/src/promote.ts +++ b/packages/cli/src/promote.ts @@ -357,7 +357,7 @@ function holdBackDropsStillInUse( skipped: PlannedWork['skipped'], downs: ReadonlyMap = new Map(), ): PlannedSql[] { - const kept = keptBack.map(k => ({ issueId: k.issueId, skeleton: sqlSkeleton(k.definition) })) + const kept = keptBack.map(k => ({ issueId: k.issueId, skeleton: usesOf(k.definition) })) let remaining = statements // Until nothing more is held back: what one held-back drop keeps can keep @@ -378,7 +378,7 @@ function holdBackDropsStillInUse( + 're-run with --allow-destructive to apply both', }) const definition = downs.get(statement.issueId) - if (definition) kept.push({ issueId: user.issueId, skeleton: sqlSkeleton(definition) }) + if (definition) kept.push({ issueId: user.issueId, skeleton: usesOf(definition) }) changed = true return false }) @@ -386,6 +386,20 @@ function holdBackDropsStillInUse( return remaining } +/** + * What a definition can be searched for to see which objects it uses: its + * skeleton, plus the sequences it names in string literals. A column default + * names its sequence only inside one — `nextval('public.shared_seq'::regclass)` + * — which the skeleton blanks, so a sequence a held-back table still drew on + * was dropped anyway, and failed with `cannot drop sequence ... because other + * objects depend on it`. + */ +function usesOf(definition: string): string { + const sequences = [...definition.matchAll(SEQUENCE_LITERAL)].map(m => m[1] ?? m[2]) + return [sqlSkeleton(definition), ...sequences].join(' ') +} +const SEQUENCE_LITERAL = /nextval\(\s*'((?:[^']|'')*)'|'((?:[^']|'')*)'\s*::\s*regclass/gi + /** The names a fix drops, when all it does is drop types, domains, routines or sequences. */ function droppedSupportingObjects(sql: string): string[] { const parts = splitSqlStatements(sql).filter(s => !isCommentOnly(s)) diff --git a/packages/cli/test/promote.test.ts b/packages/cli/test/promote.test.ts index 927770c..d1d8dc7 100644 --- a/packages/cli/test/promote.test.ts +++ b/packages/cli/test/promote.test.ts @@ -759,6 +759,24 @@ describe('planWork and drops something held back still uses', () => { expect(plan.skipped.map(s => s.issueId).sort()).toEqual(['schema-alter-1', 'schema-drop-type-2', 'schema-drop-type-3']) }) + it('holds back a sequence a held-back table\'s default still draws on', () => { + // A default names its sequence only in a string literal, as pg_dump writes it. + const plan = planWork(makeScanResult({ + checks: [{ + check: 'schema', status: 'drifted', durationMs: 0, issues: [ + { id: 'schema-drop-1', check: 'schema', severity: 'critical', description: 'd', title: 'Extra table: public.b', + sql: { up: 'DROP TABLE "b";', down: "CREATE TABLE public.b (\n id integer DEFAULT nextval('public.shared_seq'::regclass) NOT NULL\n);" } }, + { id: 'schema-alter-2', check: 'schema', severity: 'warning', description: 'd', title: 'Table altered: public.a', + sql: { up: 'ALTER TABLE "a" ALTER COLUMN "id" DROP DEFAULT;\nALTER SEQUENCE "shared_seq" OWNED BY NONE;', down: `ALTER TABLE "a" ALTER COLUMN "id" SET DEFAULT nextval('shared_seq'::regclass);` } }, + { id: 'schema-drop-sequence-3', check: 'schema', severity: 'critical', description: 'd', title: 'Extra sequence: public.shared_seq', + sql: { up: 'DROP SEQUENCE IF EXISTS "shared_seq";', down: 'CREATE SEQUENCE "shared_seq" START WITH 1000;' } }, + ], + }], + })) + expect(plan.sqlStatements.map(s => s.issueId)).toEqual(['schema-alter-2']) + expect(plan.skipped.find(s => s.issueId === 'schema-drop-sequence-3')?.reason).toMatch(/Still used by what schema-drop-1 keeps/) + }) + it('drops both once destructive fixes are allowed', () => { const plan = planWork(withLegacyTable(), { allowDestructive: true }) expect(plan.sqlStatements.map(s => s.issueId).sort()).toEqual(['schema-drop-1', 'schema-drop-type-2', 'schema-drop-type-3'])