Merge pull request #269 from constructive-io/devin/1767720648-fix-failing-fixtures

fix(plpgsql-deparser): fix failing fixtures - EXCEPTION blocks, schema qualification, implicit variables

Author: pyramation

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
@@ -192,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$$"

packages/plpgsql-deparser/__tests__/plpgsql-deparser.test.ts

@@ -37,23 +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',
-      'plpgsql_array-20.sql',
     ]);
 
     it('should round-trip ALL generated fixtures (excluding known failures)', async () => {

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
@@ -165,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"
@@ -220,8 +227,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
@@ -366,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"

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)
@@ -457,8 +460,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 '';
@@ -567,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';
             }

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,
   });
 };