fix(db): classify only executable SQL, following literals that EXECUTE runs

Author: waleedlatif1

scripts/check-migrations-safety.test.ts

@@ -190,6 +190,45 @@ describe('migration safety audit', () => {
     expect(`${stdout}${stderr}`).toContain('sequence-owned-by-existing-table')
   }, 30_000)
 
+  it('does not read operations out of literals, comments, or dollar-quoted data', async () => {
+    const dir = migrationsDir(
+      [
+        '-- Next release: ALTER TABLE "jobs" RENAME COLUMN "a" TO "b".',
+        'COMMENT ON TABLE "jobs" IS $note$Then ALTER TABLE jobs DROP COLUMN old$note$;--> statement-breakpoint',
+        'COMMENT ON COLUMN "jobs"."id" IS \'kept until ALTER TABLE jobs DROP COLUMN id\';',
+      ].join('\n')
+    )
+    const { code, stdout, stderr } = await runAudit('--dir', dir)
+    const output = `${stdout}${stderr}`
+
+    expect(output).not.toContain('drop-column')
+    expect(output).not.toContain('rename')
+    expect(code).toBe(0)
+  }, 30_000)
+
+  it('still flags a real destructive operation', async () => {
+    const dir = migrationsDir('ALTER TABLE "jobs" DROP COLUMN "old";')
+    const { code, stdout, stderr } = await runAudit('--dir', dir)
+
+    expect(code).toBe(1)
+    expect(`${stdout}${stderr}`).toContain('drop-column')
+  }, 30_000)
+
+  /** Migration 0076 renames a column this way; the literal is SQL the block runs, not data. */
+  it.each([
+    ['rename', "DO $$ BEGIN EXECUTE 'ALTER TABLE jobs RENAME COLUMN metadata TO data'; END $$;"],
+    ['owner-to', "DO $$ BEGIN EXECUTE E'ALTER TABLE jobs OWNER TO postgres'; END $$;"],
+  ])(
+    'reads dynamic SQL that EXECUTE runs (%s)',
+    async (rule, sql) => {
+      const { code, stdout, stderr } = await runAudit('--dir', migrationsDir(sql))
+
+      expect(code).toBe(1)
+      expect(`${stdout}${stderr}`).toContain(rule)
+    },
+    30_000
+  )
+
   /**
    * A table created in another schema must not exempt the same-named existing table in `public`
    * from the downtime checks: its old rows still need a default for a NOT NULL column.

scripts/check-migrations-safety.ts

@@ -72,16 +72,30 @@ function qualifiedName(raw: string): string {
 /**
  * The statement with string literals and comments blanked, so only executable SQL is matched.
  * An escape string (`E'…'`) is read with its backslash escapes, so `\'` does not end it early.
- * A dollar-quoted body stays only where it runs, after `DO` or a function's `AS`, with its own
- * literals blanked in turn; dollar-quoted data is blanked like any other string.
+ * A dollar-quoted body stays only where it runs, after `DO` or a function's `AS`, and a literal
+ * stays where `EXECUTE` runs it as dynamic SQL, each with its own literals blanked in turn. Every
+ * other literal is data. Dynamic SQL assembled by concatenation or `format()` is not followed.
  */
 function executableSql(sql: string): string {
   return sql.replace(
-    /\$(\w*)\$([\s\S]*?)\$\1\$|(?<![\w$])[Ee]'(?:[^'\\]|\\[\s\S]|'')*'|'(?:[^']|'')*'|--[^\n]*|\/\*[\s\S]*?\*\//g,
-    (_match, tag: string | undefined, body: string | undefined, offset: number) => {
-      if (body === undefined) return ' '
-      const runs = /\b(?:DO|AS)\s*$|\bDO\s+LANGUAGE\s+\w+\s*$/i.test(sql.slice(0, offset))
-      return runs ? `$${tag}$${executableSql(body)}$${tag}$` : ' '
+    /\$(\w*)\$([\s\S]*?)\$\1\$|(?<![\w$])[Ee]'((?:[^'\\]|\\[\s\S]|'')*)'|'((?:[^']|'')*)'|--[^\n]*|\/\*[\s\S]*?\*\//g,
+    (
+      _match: string,
+      tag: string | undefined,
+      body: string | undefined,
+      escaped: string | undefined,
+      plain: string | undefined,
+      offset: number
+    ) => {
+      const before = sql.slice(0, offset)
+      if (body !== undefined) {
+        const runs = /\b(?:DO|AS)\s*$|\bDO\s+LANGUAGE\s+\w+\s*$/i.test(before)
+        return runs ? `$${tag}$${executableSql(body)}$${tag}$` : ' '
+      }
+      if (!/\bEXECUTE\s*$/i.test(before)) return ' '
+      if (escaped !== undefined) return ` ${executableSql(escaped.replace(/\\(.)/g, '$1'))} `
+      if (plain !== undefined) return ` ${executableSql(plain.replace(/''/g, "'"))} `
+      return ' '
     }
   )
 }
@@ -254,10 +268,12 @@ interface RawMatch {
  * migration, by schema-qualified name so a same-named table in another schema
  * is not mistaken for one — ops against a brand-new table have no old rows and no live
  * traffic, so they are always safe and skipped. `sawCommit` tracks whether a
- * `COMMIT;` breakpoint preceded a CONCURRENTLY index (see migrate.ts).
+ * `COMMIT;` breakpoint preceded a CONCURRENTLY index (see migrate.ts). Only
+ * executable SQL is classified, so words in literals, comments and dollar-quoted
+ * data never count as operations.
  */
 function classify(sql: string, createdTables: Set<string>, sawCommit: boolean): RawMatch[] {
-  const s = sql.replace(/\s+/g, ' ').trim()
+  const s = executableSql(sql).replace(/\s+/g, ' ').trim()
   const matches: RawMatch[] = []
 
   const alterTable = s.match(