Merge pull request #306 from constructive-io/fix/cursor-args-raise-sqlstate

fix(plpgsql-deparser): bound cursor argument lists and RAISE SQLSTATE conditions

Author: pyramation

__fixtures__/plpgsql-generated/generated.json

@@ -152,6 +152,9 @@
   "plpgsql_deparser_fixes-51.sql": "-- Test 51: Exception handler with SQLSTATE condition (must emit SQLSTATE 'xxxxx')\nCREATE FUNCTION test_sqlstate_condition() RETURNS int\nLANGUAGE plpgsql AS $$\nBEGIN\n  RETURN 1;\nEXCEPTION\n  WHEN unique_violation OR SQLSTATE '23503' THEN\n    RETURN -1;\n  WHEN SQLSTATE 'P0001' THEN\n    RETURN -2;\nEND$$",
   "plpgsql_deparser_fixes-52.sql": "-- Test 52: Bare RAISE re-throw inside an exception handler (must stay bare)\nCREATE FUNCTION test_bare_raise_rethrow() RETURNS void\nLANGUAGE plpgsql AS $$\nBEGIN\n  PERFORM 1;\nEXCEPTION\n  WHEN OTHERS THEN\n    RAISE;\nEND$$",
   "plpgsql_deparser_fixes-53.sql": "-- Test 53: Array element and slice assignment (target must not be parenthesized)\nCREATE FUNCTION test_array_element_assignment() RETURNS int[]\nLANGUAGE plpgsql AS $$\nDECLARE\n  a int[] := ARRAY[1, 2, 3, 4, 5];\n  m int[][] := ARRAY[ARRAY[1, 2], ARRAY[3, 4]];\nBEGIN\n  a[2] := 20;\n  a[2:3] := ARRAY[9, 9];\n  m[1][2] := 42;\n  RETURN a;\nEND$$",
+  "plpgsql_deparser_fixes-54.sql": "-- Test 54: Bound cursor with explicit arguments (must emit the parameter list)\nCREATE FUNCTION test_bound_cursor_args() RETURNS void\nLANGUAGE plpgsql AS $$\nDECLARE\n  c CURSOR (key int, label text) FOR SELECT * FROM users WHERE id = key AND name = label;\n  r record;\nBEGIN\n  OPEN c(42, 'x');\n  FETCH c INTO r;\n  CLOSE c;\nEND$$",
+  "plpgsql_deparser_fixes-55.sql": "-- Test 55: RAISE with a SQLSTATE condition code (must emit SQLSTATE 'xxxxx', not a bare number)\nCREATE FUNCTION test_raise_sqlstate() RETURNS void\nLANGUAGE plpgsql AS $$\nBEGIN\n  RAISE SQLSTATE '22012';\nEND$$",
+  "plpgsql_deparser_fixes-56.sql": "-- Test 56: RAISE EXCEPTION with a named condition (must stay a bare identifier)\nCREATE FUNCTION test_raise_named_condition() RETURNS void\nLANGUAGE plpgsql AS $$\nBEGIN\n  RAISE EXCEPTION division_by_zero;\nEND$$",
   "plpgsql_control-1.sql": "--\n-- Tests for PL/pgSQL control structures\n--\n\n-- integer FOR loop\n\ndo $$\nbegin\n  -- basic case\n  for i in 1..3 loop\n    raise notice '1..3: i = %', i;\n  end loop;\n  -- with BY, end matches exactly\n  for i in 1..10 by 3 loop\n    raise notice '1..10 by 3: i = %', i;\n  end loop;\n  -- with BY, end does not match\n  for i in 1..11 by 3 loop\n    raise notice '1..11 by 3: i = %', i;\n  end loop;\n  -- zero iterations\n  for i in 1..0 by 3 loop\n    raise notice '1..0 by 3: i = %', i;\n  end loop;\n  -- REVERSE\n  for i in reverse 10..0 by 3 loop\n    raise notice 'reverse 10..0 by 3: i = %', i;\n  end loop;\n  -- potential overflow\n  for i in 2147483620..2147483647 by 10 loop\n    raise notice '2147483620..2147483647 by 10: i = %', i;\n  end loop;\n  -- potential overflow, reverse direction\n  for i in reverse -2147483620..-2147483647 by 10 loop\n    raise notice 'reverse -2147483620..-2147483647 by 10: i = %', i;\n  end loop;\nend$$",
   "plpgsql_control-2.sql": "-- BY can't be zero or negative\ndo $$\nbegin\n  for i in 1..3 by 0 loop\n    raise notice '1..3 by 0: i = %', i;\n  end loop;\nend$$",
   "plpgsql_control-3.sql": "do $$\nbegin\n  for i in 1..3 by -1 loop\n    raise notice '1..3 by -1: i = %', i;\n  end loop;\nend$$",

__fixtures__/plpgsql/plpgsql_deparser_fixes.sql

@@ -676,3 +676,29 @@ BEGIN
   m[1][2] := 42;
   RETURN a;
 END$$;
+
+-- Test 54: Bound cursor with explicit arguments (must emit the parameter list)
+CREATE FUNCTION test_bound_cursor_args() RETURNS void
+LANGUAGE plpgsql AS $$
+DECLARE
+  c CURSOR (key int, label text) FOR SELECT * FROM users WHERE id = key AND name = label;
+  r record;
+BEGIN
+  OPEN c(42, 'x');
+  FETCH c INTO r;
+  CLOSE c;
+END$$;
+
+-- Test 55: RAISE with a SQLSTATE condition code (must emit SQLSTATE 'xxxxx', not a bare number)
+CREATE FUNCTION test_raise_sqlstate() RETURNS void
+LANGUAGE plpgsql AS $$
+BEGIN
+  RAISE SQLSTATE '22012';
+END$$;
+
+-- Test 56: RAISE EXCEPTION with a named condition (must stay a bare identifier)
+CREATE FUNCTION test_raise_named_condition() RETURNS void
+LANGUAGE plpgsql AS $$
+BEGIN
+  RAISE EXCEPTION division_by_zero;
+END$$;

packages/plpgsql-deparser/__tests__/__snapshots__/deparser-fixes.test.ts.snap

@@ -180,6 +180,18 @@ exports[`plpgsql-deparser bug fixes PERFORM SELECT fix should strip SELECT keywo
 END"
 `;
 
+exports[`plpgsql-deparser bug fixes RAISE with SQLSTATE condition should emit SQLSTATE codes as SQLSTATE literals, not bare numbers 1`] = `
+"BEGIN
+  RAISE EXCEPTION SQLSTATE '22012';
+END"
+`;
+
+exports[`plpgsql-deparser bug fixes RAISE with SQLSTATE condition should keep named conditions as bare identifiers 1`] = `
+"BEGIN
+  RAISE EXCEPTION division_by_zero;
+END"
+`;
+
 exports[`plpgsql-deparser bug fixes Record field qualification (recfield) should handle OLD and NEW record references 1`] = `
 "BEGIN
   IF OLD.status <> NEW.status THEN
@@ -303,6 +315,17 @@ exports[`plpgsql-deparser bug fixes blocks inside control structures should hand
 END"
 `;
 
+exports[`plpgsql-deparser bug fixes bound cursor arguments should emit the parameter list of a bound cursor and not redeclare its args 1`] = `
+"DECLARE
+  c CURSOR (key int, label text) FOR SELECT * FROM users WHERE id = key AND name = label;
+  r RECORD;
+BEGIN
+  OPEN c (42, 'x');
+  FETCH FROM c INTO r;
+  CLOSE c;
+END"
+`;
+
 exports[`plpgsql-deparser bug fixes combined scenarios should handle PERFORM with record fields 1`] = `
 "BEGIN
   PERFORM notify_change(NEW.id, NEW.status);

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

@@ -1238,4 +1238,63 @@ $$`;
       expect(deparsed).not.toContain('"name%TYPE"');
     });
   });
+
+  describe('bound cursor arguments', () => {
+    it('should emit the parameter list of a bound cursor and not redeclare its args', async () => {
+      const sql = `CREATE FUNCTION test_bound_cursor_args() RETURNS void
+LANGUAGE plpgsql AS $$
+DECLARE
+  c CURSOR (key int, label text) FOR SELECT * FROM users WHERE id = key AND name = label;
+  r record;
+BEGIN
+  OPEN c(42, 'x');
+  FETCH c INTO r;
+  CLOSE c;
+END$$`;
+
+      await testUtils.expectAstMatch('bound cursor args', sql);
+
+      const parsed = parsePlPgSQLSync(sql) as unknown as PLpgSQLParseResult;
+      const deparsed = deparseSync(parsed);
+      expect(deparsed).toMatchSnapshot();
+      expect(deparsed).toMatch(/c CURSOR \(key int, label text\) FOR/);
+      // The cursor args must not leak out as standalone DECLARE variables
+      expect(deparsed).not.toMatch(/^\s*key int;/m);
+      expect(deparsed).not.toMatch(/^\s*label text;/m);
+    });
+  });
+
+  describe('RAISE with SQLSTATE condition', () => {
+    it('should emit SQLSTATE codes as SQLSTATE literals, not bare numbers', async () => {
+      const sql = `CREATE FUNCTION test_raise_sqlstate() RETURNS void
+LANGUAGE plpgsql AS $$
+BEGIN
+  RAISE SQLSTATE '22012';
+END$$`;
+
+      await testUtils.expectAstMatch('raise sqlstate', sql);
+
+      const parsed = parsePlPgSQLSync(sql) as unknown as PLpgSQLParseResult;
+      const deparsed = deparseSync(parsed);
+      expect(deparsed).toMatchSnapshot();
+      expect(deparsed).toMatch(/RAISE EXCEPTION SQLSTATE '22012'/i);
+      expect(deparsed).not.toMatch(/RAISE EXCEPTION 22012/);
+    });
+
+    it('should keep named conditions as bare identifiers', async () => {
+      const sql = `CREATE FUNCTION test_raise_named_condition() RETURNS void
+LANGUAGE plpgsql AS $$
+BEGIN
+  RAISE EXCEPTION division_by_zero;
+END$$`;
+
+      await testUtils.expectAstMatch('raise named condition', sql);
+
+      const parsed = parsePlPgSQLSync(sql) as unknown as PLpgSQLParseResult;
+      const deparsed = deparseSync(parsed);
+      expect(deparsed).toMatchSnapshot();
+      expect(deparsed).toMatch(/RAISE EXCEPTION division_by_zero/i);
+      expect(deparsed).not.toMatch(/SQLSTATE/i);
+    });
+  });
 });

packages/plpgsql-deparser/src/plpgsql-deparser.ts

@@ -603,8 +603,25 @@ export class PLpgSQLDeparser {
       return '';
     }
 
+    // Bound-cursor argument variables are emitted inside the cursor's own
+    // declaration (`c CURSOR (key int) FOR ...`), not as standalone DECLAREs.
+    const cursorArgVarnos = new Set<number>();
+    for (const datum of datums) {
+      if ('PLpgSQL_var' in datum && datum.PLpgSQL_var.cursor_explicit_argrow !== undefined) {
+        const argrow = datums[datum.PLpgSQL_var.cursor_explicit_argrow];
+        if (argrow && 'PLpgSQL_row' in argrow) {
+          for (const field of argrow.PLpgSQL_row.fields ?? []) {
+            cursorArgVarnos.add(field.varno);
+          }
+        }
+      }
+    }
+
     // Filter out internal variables (like 'found', parameters, etc.) and loop variables
     const localVars = datums.filter((datum, index) => {
+      if (cursorArgVarnos.has(index)) {
+        return false;
+      }
       // If includedIndices is provided, only include datums at those indices
       if (includedIndices !== undefined && !includedIndices.has(index)) {
         return false;
@@ -701,7 +718,24 @@ export class PLpgSQLDeparser {
           parts.push(kw('NO SCROLL'));
         }
       }
-      parts.push(kw('CURSOR FOR'));
+      parts.push(kw('CURSOR'));
+      if (v.cursor_explicit_argrow !== undefined && context.datums) {
+        const argrow = context.datums[v.cursor_explicit_argrow];
+        if (argrow && 'PLpgSQL_row' in argrow) {
+          const args = (argrow.PLpgSQL_row.fields ?? []).map((field) => {
+            const argDatum = context.datums![field.varno];
+            const argType =
+              argDatum && 'PLpgSQL_var' in argDatum && argDatum.PLpgSQL_var.datatype
+                ? ` ${this.deparseType(argDatum.PLpgSQL_var.datatype)}`
+                : '';
+            return `${field.name}${argType}`;
+          });
+          if (args.length > 0) {
+            parts.push(`(${args.join(', ')})`);
+          }
+        }
+      }
+      parts.push(kw('FOR'));
       parts.push(this.deparseExpr(v.cursor_explicit_expr));
       return parts.join(' ');
     }
@@ -1476,9 +1510,15 @@ export class PLpgSQLDeparser {
       parts.push(level);
     }
 
-    // Condition name (for RAISE without message)
+    // Condition name (for RAISE without message). SQLSTATE conditions are
+    // stored as their raw 5-character code (e.g. '22012') and must be emitted
+    // as `SQLSTATE '22012'`; named conditions are lowercase identifiers.
     if (raise.condname) {
-      parts.push(raise.condname);
+      if (/^[0-9A-Z]{5}$/.test(raise.condname)) {
+        parts.push(`${kw('SQLSTATE')} '${raise.condname}'`);
+      } else {
+        parts.push(raise.condname);
+      }
     }
 
     // Message