fix: deparser partition/FK fixes, enforced constraint builder default, walkSql body-error abort (#292 #346 #348 #349 #350 #351)

__fixtures__/generated/generated.json

@@ -21374,6 +21374,24 @@
   "misc/issues-20.sql": "CREATE TABLE test_exclude_where (\n  id uuid PRIMARY KEY,\n  database_id uuid NOT NULL,\n  status text NOT NULL DEFAULT 'pending',\n  EXCLUDE USING btree (database_id WITH =)\n    WHERE (status = 'pending')\n)",
   "misc/issues-21.sql": "CREATE TABLE test_named_exclude (\n  id uuid PRIMARY KEY,\n  database_id uuid NOT NULL,\n  status text NOT NULL DEFAULT 'pending',\n  CONSTRAINT one_pending_per_database\n    EXCLUDE USING btree (database_id WITH =)\n    WHERE (status = 'pending')\n)",
   "misc/issues-22.sql": "ALTER TABLE test_named_exclude ADD CONSTRAINT no_overlap EXCLUDE USING gist (room WITH =, during WITH &&)",
+  "misc/issue-349-fk-set-cols-1.sql": "ALTER TABLE ONLY child ADD CONSTRAINT child_fk FOREIGN KEY (a, b) REFERENCES parent(a, b) ON DELETE SET NULL (b)",
+  "misc/issue-349-fk-set-cols-2.sql": "ALTER TABLE ONLY child ADD CONSTRAINT child_fk FOREIGN KEY (a, b) REFERENCES parent(a, b) ON DELETE SET DEFAULT (b)",
+  "misc/issue-349-fk-set-cols-3.sql": "ALTER TABLE ONLY child ADD CONSTRAINT child_fk FOREIGN KEY (a, b) REFERENCES parent(a, b) ON UPDATE CASCADE ON DELETE SET NULL (a, b)",
+  "misc/issue-349-fk-set-cols-4.sql": "CREATE TABLE child (a int, b int, FOREIGN KEY (a, b) REFERENCES parent (a, b) ON DELETE SET NULL (b))",
+  "misc/issue-348-350-partition-cmd-1.sql": "ALTER TABLE ONLY public.measurement ATTACH PARTITION public.measurement_y2024 FOR VALUES FROM ('2024-01-01') TO ('2025-01-01')",
+  "misc/issue-348-350-partition-cmd-2.sql": "ALTER TABLE ONLY public.measurement DETACH PARTITION public.measurement_y2024",
+  "misc/issue-348-350-partition-cmd-3.sql": "ALTER TABLE public.measurement DETACH PARTITION public.measurement_y2024 CONCURRENTLY",
+  "misc/issue-348-350-partition-cmd-4.sql": "ALTER TABLE ONLY measurement ATTACH PARTITION measurement_y2024 FOR VALUES FROM ('2024-01-01') TO ('2025-01-01')",
+  "misc/issue-348-350-partition-cmd-5.sql": "ALTER TABLE ONLY o ATTACH PARTITION o_p FOR VALUES WITH (MODULUS 4, REMAINDER 0)",
+  "misc/issue-348-350-partition-cmd-6.sql": "ALTER TABLE ONLY o ATTACH PARTITION o_p FOR VALUES WITH (MODULUS 4, REMAINDER 1)",
+  "misc/issue-348-350-partition-cmd-7.sql": "ALTER TABLE ONLY s.o ATTACH PARTITION s.o_p DEFAULT",
+  "misc/issue-348-350-partition-cmd-8.sql": "ALTER TABLE ONLY o ATTACH PARTITION o_p FOR VALUES IN (1, 2)",
+  "misc/issue-348-350-partition-cmd-9.sql": "CREATE TABLE o_p0 PARTITION OF o FOR VALUES WITH (MODULUS 4, REMAINDER 0)",
+  "misc/issue-292-merge-when-1.sql": "MERGE INTO t AS target USING (SELECT 1 AS id) AS source ON target.id = source.id WHEN MATCHED THEN UPDATE SET name = 'x' WHEN NOT MATCHED THEN INSERT (id, name) VALUES (source.id, 'x')",
+  "misc/issue-292-merge-when-2.sql": "MERGE INTO t AS target USING (SELECT 1 AS id) AS source ON target.id = source.id WHEN MATCHED AND cond THEN DELETE",
+  "misc/issue-292-merge-when-3.sql": "MERGE INTO t AS target USING (SELECT 1 AS id) AS source ON target.id = source.id WHEN NOT MATCHED BY SOURCE THEN UPDATE SET name = 'x'",
+  "misc/issue-292-merge-when-4.sql": "MERGE INTO t AS target USING (SELECT 1 AS id) AS source ON target.id = source.id WHEN MATCHED THEN DO NOTHING",
+  "misc/issue-292-merge-when-5.sql": "MERGE INTO t AS target USING (SELECT 1 AS id) AS source ON target.id = source.id WHEN NOT MATCHED THEN INSERT DEFAULT VALUES",
   "misc/inflection-1.sql": "CREATE SCHEMA inflection",
   "misc/inflection-2.sql": "GRANT USAGE ON SCHEMA inflection TO PUBLIC",
   "misc/inflection-3.sql": "ALTER DEFAULT PRIVILEGES IN SCHEMA inflection \n GRANT EXECUTE ON FUNCTIONS  TO PUBLIC",

__fixtures__/kitchen-sink/misc/issue-292-merge-when.sql

@@ -0,0 +1,6 @@
+-- Ref: constructive-io/pgsql-parser#292
+MERGE INTO t AS target USING (SELECT 1 AS id) AS source ON target.id = source.id WHEN MATCHED THEN UPDATE SET name = 'x' WHEN NOT MATCHED THEN INSERT (id, name) VALUES (source.id, 'x');
+MERGE INTO t AS target USING (SELECT 1 AS id) AS source ON target.id = source.id WHEN MATCHED AND cond THEN DELETE;
+MERGE INTO t AS target USING (SELECT 1 AS id) AS source ON target.id = source.id WHEN NOT MATCHED BY SOURCE THEN UPDATE SET name = 'x';
+MERGE INTO t AS target USING (SELECT 1 AS id) AS source ON target.id = source.id WHEN MATCHED THEN DO NOTHING;
+MERGE INTO t AS target USING (SELECT 1 AS id) AS source ON target.id = source.id WHEN NOT MATCHED THEN INSERT DEFAULT VALUES;

__fixtures__/kitchen-sink/misc/issue-348-350-partition-cmd.sql

@@ -0,0 +1,11 @@
+-- Ref: constructive-io/pgsql-parser#348
+-- Ref: constructive-io/pgsql-parser#350
+ALTER TABLE ONLY public.measurement ATTACH PARTITION public.measurement_y2024 FOR VALUES FROM ('2024-01-01') TO ('2025-01-01');
+ALTER TABLE ONLY public.measurement DETACH PARTITION public.measurement_y2024;
+ALTER TABLE public.measurement DETACH PARTITION public.measurement_y2024 CONCURRENTLY;
+ALTER TABLE ONLY measurement ATTACH PARTITION measurement_y2024 FOR VALUES FROM ('2024-01-01') TO ('2025-01-01');
+ALTER TABLE ONLY o ATTACH PARTITION o_p FOR VALUES WITH (MODULUS 4, REMAINDER 0);
+ALTER TABLE ONLY o ATTACH PARTITION o_p FOR VALUES WITH (MODULUS 4, REMAINDER 1);
+ALTER TABLE ONLY s.o ATTACH PARTITION s.o_p DEFAULT;
+ALTER TABLE ONLY o ATTACH PARTITION o_p FOR VALUES IN (1, 2);
+CREATE TABLE o_p0 PARTITION OF o FOR VALUES WITH (MODULUS 4, REMAINDER 0);

__fixtures__/kitchen-sink/misc/issue-349-fk-set-cols.sql

@@ -0,0 +1,5 @@
+-- Ref: constructive-io/pgsql-parser#349
+ALTER TABLE ONLY child ADD CONSTRAINT child_fk FOREIGN KEY (a, b) REFERENCES parent(a, b) ON DELETE SET NULL (b);
+ALTER TABLE ONLY child ADD CONSTRAINT child_fk FOREIGN KEY (a, b) REFERENCES parent(a, b) ON DELETE SET DEFAULT (b);
+ALTER TABLE ONLY child ADD CONSTRAINT child_fk FOREIGN KEY (a, b) REFERENCES parent(a, b) ON UPDATE CASCADE ON DELETE SET NULL (a, b);
+CREATE TABLE child (a int, b int, FOREIGN KEY (a, b) REFERENCES parent (a, b) ON DELETE SET NULL (b));

packages/deparser/__tests__/kitchen-sink/misc-issue-292-merge-when.test.ts

@@ -0,0 +1,13 @@
+
+import { FixtureTestUtils } from '../../test-utils';
+const fixtures = new FixtureTestUtils();
+
+it('misc-issue-292-merge-when', async () => {
+  await fixtures.runFixtureTests([
+  "misc/issue-292-merge-when-1.sql",
+  "misc/issue-292-merge-when-2.sql",
+  "misc/issue-292-merge-when-3.sql",
+  "misc/issue-292-merge-when-4.sql",
+  "misc/issue-292-merge-when-5.sql"
+]);
+});

packages/deparser/__tests__/kitchen-sink/misc-issue-348-350-partition-cmd.test.ts

@@ -0,0 +1,17 @@
+
+import { FixtureTestUtils } from '../../test-utils';
+const fixtures = new FixtureTestUtils();
+
+it('misc-issue-348-350-partition-cmd', async () => {
+  await fixtures.runFixtureTests([
+  "misc/issue-348-350-partition-cmd-1.sql",
+  "misc/issue-348-350-partition-cmd-2.sql",
+  "misc/issue-348-350-partition-cmd-3.sql",
+  "misc/issue-348-350-partition-cmd-4.sql",
+  "misc/issue-348-350-partition-cmd-5.sql",
+  "misc/issue-348-350-partition-cmd-6.sql",
+  "misc/issue-348-350-partition-cmd-7.sql",
+  "misc/issue-348-350-partition-cmd-8.sql",
+  "misc/issue-348-350-partition-cmd-9.sql"
+]);
+});

packages/deparser/__tests__/kitchen-sink/misc-issue-349-fk-set-cols.test.ts

@@ -0,0 +1,12 @@
+
+import { FixtureTestUtils } from '../../test-utils';
+const fixtures = new FixtureTestUtils();
+
+it('misc-issue-349-fk-set-cols', async () => {
+  await fixtures.runFixtureTests([
+  "misc/issue-349-fk-set-cols-1.sql",
+  "misc/issue-349-fk-set-cols-2.sql",
+  "misc/issue-349-fk-set-cols-3.sql",
+  "misc/issue-349-fk-set-cols-4.sql"
+]);
+});

packages/deparser/src/deparser.ts

@@ -3079,6 +3079,15 @@ export class Deparser implements DeparserVisitor {
           deleteClause += 'SET DEFAULT';
           break;
         }
+        if ((node.fk_del_action === 'n' || node.fk_del_action === 'd') && node.fk_del_set_cols && node.fk_del_set_cols.length > 0) {
+          const setColumns = ListUtils.unwrapList(node.fk_del_set_cols)
+            .map(column => {
+              const stringNode = (column as { String: { sval?: string; str?: string } }).String;
+              return QuoteUtils.quoteIdentifier(stringNode.sval || stringNode.str || '');
+            })
+            .join(', ');
+          deleteClause += ` (${setColumns})`;
+        }
         if (context.isPretty()) {
           output.push('\n' + context.indent(deleteClause));
         } else {
@@ -3917,12 +3926,12 @@ export class Deparser implements DeparserVisitor {
   PartitionCmd(node: t.PartitionCmd, context: DeparserContext): string {
     const output: string[] = [];
 
-    if (node.concurrent) {
-      output.push('CONCURRENTLY');
+    if (node.name) {
+      output.push(this.RangeVar(node.name, context));
     }
 
-    if (node.name) {
-      output.push(this.visit(node.name as any, context));
+    if (node.concurrent) {
+      output.push('CONCURRENTLY');
     }
 
     if (node.bound) {
@@ -3947,9 +3956,9 @@ export class Deparser implements DeparserVisitor {
             .join(', ');
           output.push(`(${upperValues})`);
         }
-      } else if (node.bound.strategy === 'h' && node.bound.modulus !== undefined && node.bound.remainder !== undefined) {
+      } else if (node.bound.strategy === 'h' && node.bound.modulus !== undefined) {
         output.push('FOR VALUES WITH');
-        output.push(`(modulus ${node.bound.modulus}, remainder ${node.bound.remainder})`);
+        output.push(`(MODULUS ${node.bound.modulus}, REMAINDER ${node.bound.remainder ?? 0})`);
       } else if (node.bound.is_default) {
         output.push('DEFAULT');
       }
@@ -11644,4 +11653,3 @@ export class Deparser implements DeparserVisitor {
     return stringLiteralRegex.test(content);
   }
 }
-

packages/plpgsql-parser/__tests__/walk-sql-body-errors.test.ts

@@ -0,0 +1,67 @@
+import { loadModule, parseSync, walkSql } from '../src';
+
+beforeAll(async () => {
+  await loadModule();
+});
+
+const BROKEN_FUNCTION_SQL = `
+  CREATE FUNCTION broken_connectors() RETURNS void
+  LANGUAGE plpgsql AS $$
+  BEGIN
+    UPDATE connectors SET instance_id = ;
+  END;
+  $$;
+`;
+
+describe('walkSql PL/pgSQL body errors', () => {
+  it('reports a broken PL/pgSQL body as an abort and records its statement index', () => {
+    const parsed = parseSync(BROKEN_FUNCTION_SQL);
+    expect(parsed.errors).toHaveLength(1);
+    expect(parsed.errors[0].stmtIndex).toBe(0);
+
+    const result = walkSql(BROKEN_FUNCTION_SQL, {});
+    expect(result.aborted).toBe(true);
+    expect(result.reason).toBeTruthy();
+    expect(result.reasons).toEqual([parsed.errors[0].message]);
+  });
+
+  it('walks valid PL/pgSQL bodies', () => {
+    const visited: string[] = [];
+    const result = walkSql(
+      `
+        CREATE FUNCTION valid_connectors() RETURNS void
+        LANGUAGE plpgsql AS $$
+        BEGIN
+          UPDATE connectors SET instance_id = 1;
+        END;
+        $$;
+      `,
+      (path) => {
+        if (path.tag.startsWith('PLpgSQL_')) {
+          visited.push(path.tag);
+        }
+      }
+    );
+
+    expect(result.aborted).toBe(false);
+    expect(visited).toContain('PLpgSQL_function');
+    expect(visited).toContain('PLpgSQL_stmt_execsql');
+  });
+
+  it('does not parse broken bodies when walkFunctionBodies is false', () => {
+    const result = walkSql(BROKEN_FUNCTION_SQL, {}, { walkFunctionBodies: false });
+    expect(result.aborted).toBe(false);
+  });
+
+  it('does not inspect non-PL/pgSQL function bodies', () => {
+    const result = walkSql(
+      `
+        CREATE FUNCTION sql_function() RETURNS integer
+        LANGUAGE sql AS $$ THIS IS NOT VALID SQL BODY TEXT $$;
+      `,
+      {},
+    );
+
+    expect(result.aborted).toBe(false);
+  });
+});

packages/plpgsql-parser/src/parse.ts

@@ -12,6 +12,7 @@ import type {
   ParsedFunction,
   ParsedItem,
   ParsedScript,
+  ParsedScriptError,
   ParsedStatement,
   ParseOptions
 } from './types';
@@ -68,15 +69,19 @@ function getStatementSql(sqlBuffer: Buffer, rawStmt: any): string {
   return sqlBuffer.slice(start, end).toString('utf8');
 }
 
-function extractFunctionInfo(stmt: any, stmtIndex: number, stmtSql: string): ParsedFunction | null {
+function extractFunctionInfo(
+  stmt: any,
+  stmtIndex: number,
+  stmtSql: string
+): { fn: ParsedFunction | null; error?: string } {
   const createFunctionStmt = stmt?.CreateFunctionStmt;
-  if (!createFunctionStmt) return null;
+  if (!createFunctionStmt) return { fn: null };
 
   const language = getLanguageFromOptions(createFunctionStmt.options);
-  if (language !== 'plpgsql') return null;
+  if (language !== 'plpgsql') return { fn: null };
 
   const body = getBodyFromOptions(createFunctionStmt.options);
-  if (!body) return null;
+  if (!body) return { fn: null };
 
   try {
     // Parse only this statement's SQL. Parsing the full script would return
@@ -86,20 +91,25 @@ function extractFunctionInfo(stmt: any, stmtIndex: number, stmtSql: string): Par
     const { ast: hydrated, stats, errors } = hydratePlpgsqlAst(plpgsqlRaw);
 
     return {
-      kind: 'plpgsql-function',
-      stmt: createFunctionStmt,
-      stmtIndex,
-      language: language || 'plpgsql',
-      body,
-      plpgsql: {
-        raw: plpgsqlRaw,
-        hydrated,
-        stats,
-        errors
+      fn: {
+        kind: 'plpgsql-function',
+        stmt: createFunctionStmt,
+        stmtIndex,
+        language: language || 'plpgsql',
+        body,
+        plpgsql: {
+          raw: plpgsqlRaw,
+          hydrated,
+          stats,
+          errors
+        }
       }
     };
   } catch (err) {
-    return null;
+    return {
+      fn: null,
+      error: err instanceof Error ? err.message : String(err)
+    };
   }
 }
 
@@ -109,6 +119,7 @@ export function parse(sql: string, options: ParseOptions = {}): ParsedScript {
   const sqlResult: ParseResult = parseSqlSync(sql);
   const items: ParsedItem[] = [];
   const functions: ParsedFunction[] = [];
+  const errors: ParsedScriptError[] = [];
   const sqlBuffer = Buffer.from(sql, 'utf8');
 
   if (sqlResult.stmts) {
@@ -117,12 +128,15 @@ export function parse(sql: string, options: ParseOptions = {}): ParsedScript {
       const stmt = rawStmt?.stmt;
 
       if (stmt && isPlpgsqlFunction(stmt) && hydrate) {
-        const fnInfo = extractFunctionInfo(stmt, i, getStatementSql(sqlBuffer, rawStmt));
-        if (fnInfo) {
-          items.push(fnInfo);
-          functions.push(fnInfo);
+        const result = extractFunctionInfo(stmt, i, getStatementSql(sqlBuffer, rawStmt));
+        if (result.fn) {
+          items.push(result.fn);
+          functions.push(result.fn);
           continue;
         }
+        if (result.error) {
+          errors.push({ stmtIndex: i, message: result.error });
+        }
       }
 
       const stmtItem: ParsedStatement = {
@@ -137,7 +151,8 @@ export function parse(sql: string, options: ParseOptions = {}): ParsedScript {
   return {
     sql: sqlResult,
     items,
-    functions
+    functions,
+    errors
   };
 }
 

packages/plpgsql-parser/src/traverse.ts

@@ -40,8 +40,9 @@ export interface WalkSqlOptions extends WalkOptions {
  * });
  * ```
  *
- * Unparseable input is reported as an abort rather than a thrown error, so a
- * validator can treat "rejected" and "could not be understood" uniformly.
+ * Unparseable SQL or PL/pgSQL function bodies are reported as an abort rather
+ * than a thrown error, so a validator can treat "rejected" and "could not be
+ * understood" uniformly.
  */
 export function walkSql(
   sql: string,
@@ -62,5 +63,13 @@ export function walkSql(
     return { aborted: true, reason, reasons: [reason] };
   }
 
+  if (walkFunctionBodies && parsed.errors.length > 0) {
+    return {
+      aborted: true,
+      reason: parsed.errors[0].message,
+      reasons: parsed.errors.map(error => error.message)
+    };
+  }
+
   return walk(parsed, visitors, { ...options, walkFunctionBodies });
 }

packages/plpgsql-parser/src/types.ts

@@ -33,10 +33,16 @@ export interface ParsedStatement {
 
 export type ParsedItem = ParsedFunction | ParsedStatement;
 
+export interface ParsedScriptError {
+  stmtIndex: number;
+  message: string;
+}
+
 export interface ParsedScript {
   sql: ParseResult;
   items: ParsedItem[];
   functions: ParsedFunction[];
+  errors: ParsedScriptError[];
 }
 
 export interface ParseOptions {

packages/scripts/src/invert.ts

@@ -493,7 +493,7 @@ function invertAlterTable(node: AnyNode, warnings: string[]): Emitted[] {
       }
       out.push(alterWith({
         subtype: 'AT_DetachPartition',
-        def: { PartitionCmd: { name: { RangeVar: clone(partition) } } },
+        def: { PartitionCmd: { name: clone(partition) } },
         behavior: 'DROP_RESTRICT'
       }));
       break;