From b5e5693db1f408ce2b6598a217e88721f440402e Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Thu, 5 Nov 2026 01:01:31 +0000 Subject: [PATCH] fix: hold back a sequence drop while a held-back table's default still uses it A column default names its sequence only inside a string literal (nextval('public.shared_seq'::regclass)), which the skeleton the check searches blanks. Without --allow-destructive, a dropped table was held back but the sequence its default drew on was dropped anyway, failed with "cannot drop sequence ... because other objects depend on it", and rolled back the other fixes in its batch. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/promote.ts | 18 ++++++++++++++++-- packages/cli/test/promote.test.ts | 18 ++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) 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'])