From 13cff5116616464ded51df21c28d2d61383aefbe Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Tue, 6 Jan 2026 17:32:59 +0000 Subject: [PATCH 1/5] fix(plpgsql-deparser): preserve schema qualification in %rowtype/%type references The deparser was stripping pg_catalog. prefix from ALL type names, but for %rowtype and %type references like pg_catalog.pg_class%rowtype[], the schema qualification is meaningful and should be preserved. Now only strips pg_catalog. for built-in types, not for %rowtype/%type refs. Fixes plpgsql_array-20.sql fixture (175/190 now passing, 15 remaining). --- .../plpgsql-deparser/__tests__/plpgsql-deparser.test.ts | 1 - packages/plpgsql-deparser/src/plpgsql-deparser.ts | 9 +++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/plpgsql-deparser/__tests__/plpgsql-deparser.test.ts b/packages/plpgsql-deparser/__tests__/plpgsql-deparser.test.ts index b99d74bb..3e361805 100644 --- a/packages/plpgsql-deparser/__tests__/plpgsql-deparser.test.ts +++ b/packages/plpgsql-deparser/__tests__/plpgsql-deparser.test.ts @@ -53,7 +53,6 @@ describe('PLpgSQLDeparser', () => { 'plpgsql_control-15.sql', 'plpgsql_control-17.sql', 'plpgsql_call-44.sql', - 'plpgsql_array-20.sql', ]); it('should round-trip ALL generated fixtures (excluding known failures)', async () => { diff --git a/packages/plpgsql-deparser/src/plpgsql-deparser.ts b/packages/plpgsql-deparser/src/plpgsql-deparser.ts index 232b5dd7..93186205 100644 --- a/packages/plpgsql-deparser/src/plpgsql-deparser.ts +++ b/packages/plpgsql-deparser/src/plpgsql-deparser.ts @@ -457,8 +457,13 @@ export class PLpgSQLDeparser { private deparseType(typeNode: PLpgSQLTypeNode): string { if ('PLpgSQL_type' in typeNode) { let typname = typeNode.PLpgSQL_type.typname; - // Clean up type names (remove pg_catalog prefix and quotes) - typname = typname.replace(/^pg_catalog\./, '').replace(/"/g, ''); + // Remove quotes + typname = typname.replace(/"/g, ''); + // Strip pg_catalog. prefix for built-in types, but preserve schema qualification + // for %rowtype and %type references where the schema is part of the table/variable reference + if (!typname.includes('%rowtype') && !typname.includes('%type')) { + typname = typname.replace(/^pg_catalog\./, ''); + } return typname.trim(); } return ''; From aec8d3dc608e6b22d7f6337469fb106c41160265 Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Tue, 6 Jan 2026 17:35:07 +0000 Subject: [PATCH 2/5] fix(plpgsql-deparser): filter out implicit sqlstate/sqlerrm variables PostgreSQL automatically adds sqlstate and sqlerrm variables for exception handling. These should not be explicitly declared in the output DECLARE section. Updated snapshots to reflect the fix. --- .../__tests__/__snapshots__/hydrate-demo.test.ts.snap | 2 -- packages/plpgsql-deparser/src/plpgsql-deparser.ts | 7 +++++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/plpgsql-deparser/__tests__/__snapshots__/hydrate-demo.test.ts.snap b/packages/plpgsql-deparser/__tests__/__snapshots__/hydrate-demo.test.ts.snap index cf789cb6..bf891b60 100644 --- a/packages/plpgsql-deparser/__tests__/__snapshots__/hydrate-demo.test.ts.snap +++ b/packages/plpgsql-deparser/__tests__/__snapshots__/hydrate-demo.test.ts.snap @@ -49,8 +49,6 @@ exports[`hydrate demonstration with big-function.sql should parse, hydrate, modi v_sql text; v_rowcount int := 0; v_lock_key bigint := CAST(CAST('x' || substr(md5(p_org_id::text), 1, 16) AS pg_catalog.bit(64)) AS bigint); - sqlstate CONSTANT text; - sqlerrm CONSTANT text; BEGIN BEGIN IF p_org_id IS NULL diff --git a/packages/plpgsql-deparser/src/plpgsql-deparser.ts b/packages/plpgsql-deparser/src/plpgsql-deparser.ts index 93186205..5c19f5b6 100644 --- a/packages/plpgsql-deparser/src/plpgsql-deparser.ts +++ b/packages/plpgsql-deparser/src/plpgsql-deparser.ts @@ -342,8 +342,11 @@ export class PLpgSQLDeparser { const localVars = datums.filter(datum => { if ('PLpgSQL_var' in datum) { const v = datum.PLpgSQL_var; - // Skip internal variables - if (v.refname === 'found' || v.refname.startsWith('__')) { + // Skip internal variables: + // - 'found' is the implicit FOUND variable + // - 'sqlstate' and 'sqlerrm' are implicit exception handling variables + // - variables starting with '__' are internal + if (v.refname === 'found' || v.refname === 'sqlstate' || v.refname === 'sqlerrm' || v.refname.startsWith('__')) { return false; } // Skip variables without lineno (usually parameters or internal) From 7e397d702ef28ab21d65285c76603e4c40b73497 Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Tue, 6 Jan 2026 17:35:28 +0000 Subject: [PATCH 3/5] chore: update pretty test snapshots for sqlstate/sqlerrm fix --- .../pretty/__snapshots__/plpgsql-pretty.test.ts.snap | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/plpgsql-deparser/__tests__/pretty/__snapshots__/plpgsql-pretty.test.ts.snap b/packages/plpgsql-deparser/__tests__/pretty/__snapshots__/plpgsql-pretty.test.ts.snap index f3c60777..84269a8e 100644 --- a/packages/plpgsql-deparser/__tests__/pretty/__snapshots__/plpgsql-pretty.test.ts.snap +++ b/packages/plpgsql-deparser/__tests__/pretty/__snapshots__/plpgsql-pretty.test.ts.snap @@ -19,8 +19,6 @@ exports[`lowercase: big-function.sql 1`] = ` v_sql text; v_rowcount int := 0; v_lock_key bigint := ('x' || substr(md5(p_org_id::text), 1, 16))::bit(64)::bigint; - sqlstate constant text; - sqlerrm constant text; begin begin if p_org_id IS NULL OR p_user_id IS NULL then @@ -220,8 +218,6 @@ exports[`uppercase: big-function.sql 1`] = ` v_sql text; v_rowcount int := 0; v_lock_key bigint := ('x' || substr(md5(p_org_id::text), 1, 16))::bit(64)::bigint; - sqlstate CONSTANT text; - sqlerrm CONSTANT text; BEGIN BEGIN IF p_org_id IS NULL OR p_user_id IS NULL THEN From 9d79484b92b4482d17dc51ace93f99f906602d57 Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Tue, 6 Jan 2026 17:38:50 +0000 Subject: [PATCH 4/5] fix(test-utils): filter out varno values during AST comparison varno values are assigned based on position in datums array and can change when implicit variables (like sqlstate/sqlerrm) are filtered out during deparse. This makes round-trip testing more robust. --- packages/plpgsql-deparser/test-utils/index.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/plpgsql-deparser/test-utils/index.ts b/packages/plpgsql-deparser/test-utils/index.ts index b1c1954a..f55b9e52 100644 --- a/packages/plpgsql-deparser/test-utils/index.ts +++ b/packages/plpgsql-deparser/test-utils/index.ts @@ -213,6 +213,9 @@ export const cleanPlpgsqlTree = (tree: any) => { location: noop, stmt_len: noop, stmt_location: noop, + // varno values are assigned based on position in datums array and can change + // when implicit variables (like sqlstate/sqlerrm) are filtered out during deparse + varno: noop, query: normalizeQueryWhitespace, }); }; From 64c900c001818867026225f81b7068bc1d939a6e Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Tue, 6 Jan 2026 17:45:06 +0000 Subject: [PATCH 5/5] fix(plpgsql-deparser): fix EXCEPTION block handling in deparser The deparser was not outputting EXCEPTION blocks because it was checking for block.exceptions.exc_list directly, but the parser wraps the exception block in a PLpgSQL_exception_block wrapper. This fix handles both the direct and wrapped formats: - { exc_list: [...] } (direct) - { PLpgSQL_exception_block: { exc_list: [...] } } (wrapped) This resolves 12 of the 15 known failing fixtures: - All 7 plpgsql_trap-* fixtures - plpgsql_transaction-19.sql, -20.sql, -21.sql - plpgsql_control-17.sql - plpgsql_call-44.sql Only 3 fixtures remain failing (down from 15): - plpgsql_varprops-13.sql (nested DECLARE inside FOR loop) - plpgsql_transaction-17.sql (CURSOR FOR loop with EXCEPTION block) - plpgsql_control-15.sql (labeled block with EXIT statement) --- .../__snapshots__/hydrate-demo.test.ts.snap | 9 +++++++++ .../__tests__/plpgsql-deparser.test.ts | 16 ++++------------ .../__snapshots__/plpgsql-pretty.test.ts.snap | 18 ++++++++++++++++++ .../plpgsql-deparser/src/plpgsql-deparser.ts | 11 ++++++++--- 4 files changed, 39 insertions(+), 15 deletions(-) diff --git a/packages/plpgsql-deparser/__tests__/__snapshots__/hydrate-demo.test.ts.snap b/packages/plpgsql-deparser/__tests__/__snapshots__/hydrate-demo.test.ts.snap index bf891b60..16ed9d7f 100644 --- a/packages/plpgsql-deparser/__tests__/__snapshots__/hydrate-demo.test.ts.snap +++ b/packages/plpgsql-deparser/__tests__/__snapshots__/hydrate-demo.test.ts.snap @@ -190,6 +190,15 @@ BEGIN message := format('rollup ok: gross=%s discount=%s tax=%s net=%s (discount_rate=%s tax_rate=%s)', v_gross, v_discount, v_tax, v_net, v_discount_rate, v_tax_rate); RETURN NEXT; RETURN; + EXCEPTION + WHEN unique_violation THEN + RAISE NOTICE 'unique_violation: %', sqlerrm; + RAISE EXCEPTION; + WHEN others THEN + IF p_debug THEN + RAISE NOTICE 'error: % (%:%)', sqlerrm, sqlstate, sqlerrm; + END IF; + RAISE EXCEPTION; END; RETURN; END$$" diff --git a/packages/plpgsql-deparser/__tests__/plpgsql-deparser.test.ts b/packages/plpgsql-deparser/__tests__/plpgsql-deparser.test.ts index 3e361805..96bf4291 100644 --- a/packages/plpgsql-deparser/__tests__/plpgsql-deparser.test.ts +++ b/packages/plpgsql-deparser/__tests__/plpgsql-deparser.test.ts @@ -37,22 +37,14 @@ describe('PLpgSQLDeparser', () => { // - Tagged dollar quote reconstruction ($tag$...$tag$ not supported) // - Exception block handling issues // TODO: Fix these underlying issues and remove from allowlist + // Remaining known failing fixtures: + // - plpgsql_varprops-13.sql: nested DECLARE inside FOR loop (loop variable scope issue) + // - plpgsql_transaction-17.sql: CURSOR FOR loop with EXCEPTION block + // - plpgsql_control-15.sql: labeled block with EXIT statement const KNOWN_FAILING_FIXTURES = new Set([ 'plpgsql_varprops-13.sql', - 'plpgsql_trap-1.sql', - 'plpgsql_trap-2.sql', - 'plpgsql_trap-3.sql', - 'plpgsql_trap-4.sql', - 'plpgsql_trap-5.sql', - 'plpgsql_trap-6.sql', - 'plpgsql_trap-7.sql', 'plpgsql_transaction-17.sql', - 'plpgsql_transaction-19.sql', - 'plpgsql_transaction-20.sql', - 'plpgsql_transaction-21.sql', 'plpgsql_control-15.sql', - 'plpgsql_control-17.sql', - 'plpgsql_call-44.sql', ]); it('should round-trip ALL generated fixtures (excluding known failures)', async () => { diff --git a/packages/plpgsql-deparser/__tests__/pretty/__snapshots__/plpgsql-pretty.test.ts.snap b/packages/plpgsql-deparser/__tests__/pretty/__snapshots__/plpgsql-pretty.test.ts.snap index 84269a8e..afe0e983 100644 --- a/packages/plpgsql-deparser/__tests__/pretty/__snapshots__/plpgsql-pretty.test.ts.snap +++ b/packages/plpgsql-deparser/__tests__/pretty/__snapshots__/plpgsql-pretty.test.ts.snap @@ -163,6 +163,15 @@ begin ); return next; return; + exception + when unique_violation then + raise notice 'unique_violation: %', SQLERRM; + raise exception; + when others then + if p_debug then + raise notice 'error: % (%:%)', SQLERRM, SQLSTATE, SQLERRM; + end if; + raise exception; end; return; end" @@ -362,6 +371,15 @@ BEGIN ); RETURN NEXT; RETURN; + EXCEPTION + WHEN unique_violation THEN + RAISE NOTICE 'unique_violation: %', SQLERRM; + RAISE EXCEPTION; + WHEN others THEN + IF p_debug THEN + RAISE NOTICE 'error: % (%:%)', SQLERRM, SQLSTATE, SQLERRM; + END IF; + RAISE EXCEPTION; END; RETURN; END" diff --git a/packages/plpgsql-deparser/src/plpgsql-deparser.ts b/packages/plpgsql-deparser/src/plpgsql-deparser.ts index 5c19f5b6..d2444e7c 100644 --- a/packages/plpgsql-deparser/src/plpgsql-deparser.ts +++ b/packages/plpgsql-deparser/src/plpgsql-deparser.ts @@ -575,12 +575,17 @@ export class PLpgSQLDeparser { } // Exception handlers - if (block.exceptions?.exc_list) { + // The exceptions property can be either: + // - { exc_list: [...] } (direct) + // - { PLpgSQL_exception_block: { exc_list: [...] } } (wrapped) + const excList = block.exceptions?.exc_list || + (block.exceptions as any)?.PLpgSQL_exception_block?.exc_list; + if (excList) { parts.push(kw('EXCEPTION')); - for (const exc of block.exceptions.exc_list) { + for (const exc of excList) { if ('PLpgSQL_exception' in exc) { const excData = exc.PLpgSQL_exception; - const conditions = excData.conditions?.map(c => { + const conditions = excData.conditions?.map((c: any) => { if ('PLpgSQL_condition' in c) { return c.PLpgSQL_condition.condname || c.PLpgSQL_condition.sqlerrstate || 'OTHERS'; }