Skip to content

Commit cb1c19d

Browse files
pyramationpyramation
authored
Merge pull request #303 from constructive-io/feat/plpgsql-deparser-cursor-raise-sqlstate-fixes
fix(plpgsql-deparser): cursor SCROLL options, FETCH/MOVE directions, SQLSTATE conditions, bare RAISE, subscripted assignment targets
2 parents f736755 + 4f7de71 commit cb1c19d

7 files changed

Lines changed: 392 additions & 17 deletions

File tree

‎__fixtures__/plpgsql-generated/generated.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,13 @@
145145
"plpgsql_deparser_fixes-44.sql": "-- Test 44: Trigger function with no final return (implicit compiler RETURN must not be emitted)\nCREATE FUNCTION test_trigger_no_final_return() RETURNS trigger\nLANGUAGE plpgsql AS $$\nBEGIN\n IF TG_OP = 'INSERT' THEN\n RETURN NEW;\n END IF;\nEND$$",
146146
"plpgsql_deparser_fixes-45.sql": "-- Test 45: Void function with explicit trailing RETURN (must be preserved)\nCREATE FUNCTION test_void_explicit_return() RETURNS void\nLANGUAGE plpgsql AS $$\nBEGIN\n RAISE NOTICE 'hi';\n RETURN;\nEND$$",
147147
"plpgsql_deparser_fixes-46.sql": "-- Test 46: Trigger function ending in RETURN NEW (unchanged)\nCREATE FUNCTION test_trigger_return_new() RETURNS trigger\nLANGUAGE plpgsql AS $$\nBEGIN\n NEW.updated_at := now();\n RETURN NEW;\nEND$$",
148+
"plpgsql_deparser_fixes-47.sql": "-- Test 47: Bound cursor declared with SCROLL (option must stay on the declaration, not OPEN)\nCREATE FUNCTION test_scroll_cursor_decl() RETURNS int\nLANGUAGE plpgsql AS $$\nDECLARE\n c SCROLL CURSOR FOR SELECT id FROM s.t ORDER BY id;\n v int;\nBEGIN\n OPEN c;\n FETCH PRIOR FROM c INTO v;\n CLOSE c;\n RETURN v;\nEND$$",
149+
"plpgsql_deparser_fixes-48.sql": "-- Test 48: Bound cursor declared with NO SCROLL\nCREATE FUNCTION test_no_scroll_cursor_decl() RETURNS void\nLANGUAGE plpgsql AS $$\nDECLARE\n c NO SCROLL CURSOR FOR SELECT id FROM s.t;\n v int;\nBEGIN\n OPEN c;\n FETCH c INTO v;\n CLOSE c;\nEND$$",
150+
"plpgsql_deparser_fixes-49.sql": "-- Test 49: Plain bound cursor (OPEN must not gain a SCROLL keyword)\nCREATE FUNCTION test_plain_cursor_open() RETURNS void\nLANGUAGE plpgsql AS $$\nDECLARE\n c CURSOR FOR SELECT id FROM s.t;\n v int;\nBEGIN\n OPEN c;\n FETCH c INTO v;\n CLOSE c;\nEND$$",
151+
"plpgsql_deparser_fixes-50.sql": "-- Test 50: MOVE with count/expression directions (counts must be preserved)\nCREATE FUNCTION test_move_directions() RETURNS void\nLANGUAGE plpgsql AS $$\nDECLARE\n c SCROLL CURSOR FOR SELECT id FROM s.t ORDER BY id;\nBEGIN\n OPEN c;\n MOVE FORWARD 3 FROM c;\n MOVE BACKWARD 2 FROM c;\n MOVE FORWARD ALL FROM c;\n MOVE BACKWARD ALL FROM c;\n MOVE LAST IN c;\n CLOSE c;\nEND$$",
152+
"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$$",
153+
"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$$",
154+
"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$$",
148155
"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$$",
149156
"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$$",
150157
"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‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -589,3 +589,90 @@ BEGIN
589589
NEW.updated_at := now();
590590
RETURN NEW;
591591
END$$;
592+
593+
-- Test 47: Bound cursor declared with SCROLL (option must stay on the declaration, not OPEN)
594+
CREATE FUNCTION test_scroll_cursor_decl() RETURNS int
595+
LANGUAGE plpgsql AS $$
596+
DECLARE
597+
c SCROLL CURSOR FOR SELECT id FROM s.t ORDER BY id;
598+
v int;
599+
BEGIN
600+
OPEN c;
601+
FETCH PRIOR FROM c INTO v;
602+
CLOSE c;
603+
RETURN v;
604+
END$$;
605+
606+
-- Test 48: Bound cursor declared with NO SCROLL
607+
CREATE FUNCTION test_no_scroll_cursor_decl() RETURNS void
608+
LANGUAGE plpgsql AS $$
609+
DECLARE
610+
c NO SCROLL CURSOR FOR SELECT id FROM s.t;
611+
v int;
612+
BEGIN
613+
OPEN c;
614+
FETCH c INTO v;
615+
CLOSE c;
616+
END$$;
617+
618+
-- Test 49: Plain bound cursor (OPEN must not gain a SCROLL keyword)
619+
CREATE FUNCTION test_plain_cursor_open() RETURNS void
620+
LANGUAGE plpgsql AS $$
621+
DECLARE
622+
c CURSOR FOR SELECT id FROM s.t;
623+
v int;
624+
BEGIN
625+
OPEN c;
626+
FETCH c INTO v;
627+
CLOSE c;
628+
END$$;
629+
630+
-- Test 50: MOVE with count/expression directions (counts must be preserved)
631+
CREATE FUNCTION test_move_directions() RETURNS void
632+
LANGUAGE plpgsql AS $$
633+
DECLARE
634+
c SCROLL CURSOR FOR SELECT id FROM s.t ORDER BY id;
635+
BEGIN
636+
OPEN c;
637+
MOVE FORWARD 3 FROM c;
638+
MOVE BACKWARD 2 FROM c;
639+
MOVE FORWARD ALL FROM c;
640+
MOVE BACKWARD ALL FROM c;
641+
MOVE LAST IN c;
642+
CLOSE c;
643+
END$$;
644+
645+
-- Test 51: Exception handler with SQLSTATE condition (must emit SQLSTATE 'xxxxx')
646+
CREATE FUNCTION test_sqlstate_condition() RETURNS int
647+
LANGUAGE plpgsql AS $$
648+
BEGIN
649+
RETURN 1;
650+
EXCEPTION
651+
WHEN unique_violation OR SQLSTATE '23503' THEN
652+
RETURN -1;
653+
WHEN SQLSTATE 'P0001' THEN
654+
RETURN -2;
655+
END$$;
656+
657+
-- Test 52: Bare RAISE re-throw inside an exception handler (must stay bare)
658+
CREATE FUNCTION test_bare_raise_rethrow() RETURNS void
659+
LANGUAGE plpgsql AS $$
660+
BEGIN
661+
PERFORM 1;
662+
EXCEPTION
663+
WHEN OTHERS THEN
664+
RAISE;
665+
END$$;
666+
667+
-- Test 53: Array element and slice assignment (target must not be parenthesized)
668+
CREATE FUNCTION test_array_element_assignment() RETURNS int[]
669+
LANGUAGE plpgsql AS $$
670+
DECLARE
671+
a int[] := ARRAY[1, 2, 3, 4, 5];
672+
m int[][] := ARRAY[ARRAY[1, 2], ARRAY[3, 4]];
673+
BEGIN
674+
a[2] := 20;
675+
a[2:3] := ARRAY[9, 9];
676+
m[1][2] := 42;
677+
RETURN a;
678+
END$$;

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

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,21 @@ BEGIN
5353
END"
5454
`;
5555

56+
exports[`plpgsql-deparser bug fixes FETCH/MOVE directions should preserve counts and FIRST/LAST directions 1`] = `
57+
"DECLARE
58+
c SCROLL CURSOR FOR SELECT id FROM s.t ORDER BY id;
59+
BEGIN
60+
OPEN c;
61+
MOVE FORWARD 3 FROM c;
62+
MOVE BACKWARD 2 FROM c;
63+
MOVE ALL FROM c;
64+
MOVE BACKWARD ALL FROM c;
65+
MOVE LAST FROM c;
66+
MOVE FIRST FROM c;
67+
CLOSE c;
68+
END"
69+
`;
70+
5671
exports[`plpgsql-deparser bug fixes INTO clause depth-aware scanner should handle INTO STRICT 1`] = `
5772
"DECLARE
5873
v_id integer;
@@ -209,6 +224,37 @@ exports[`plpgsql-deparser bug fixes Record field qualification (recfield) should
209224
END"
210225
`;
211226

227+
exports[`plpgsql-deparser bug fixes SQLSTATE exception conditions should emit SQLSTATE codes with the SQLSTATE keyword 1`] = `
228+
"BEGIN
229+
BEGIN
230+
RETURN 1;
231+
EXCEPTION
232+
WHEN unique_violation OR SQLSTATE '23503' THEN
233+
RETURN -1;
234+
WHEN SQLSTATE 'P0001' THEN
235+
RETURN -2;
236+
END;
237+
END"
238+
`;
239+
240+
exports[`plpgsql-deparser bug fixes bare RAISE re-throw should keep a bare RAISE bare (not RAISE EXCEPTION;) 1`] = `
241+
"BEGIN
242+
BEGIN
243+
PERFORM 1;
244+
EXCEPTION
245+
WHEN others THEN
246+
RAISE;
247+
END;
248+
END"
249+
`;
250+
251+
exports[`plpgsql-deparser bug fixes bare RAISE re-throw should still emit level and message for normal RAISE 1`] = `
252+
"BEGIN
253+
RAISE EXCEPTION 'boom %', 42 USING HINT = 'h';
254+
RAISE NOTICE 'hi';
255+
END"
256+
`;
257+
212258
exports[`plpgsql-deparser bug fixes blocks inside control structures should handle block inside CASE WHEN 1`] = `
213259
"BEGIN
214260
CASE p_status
@@ -276,6 +322,29 @@ BEGIN
276322
END"
277323
`;
278324

325+
exports[`plpgsql-deparser bug fixes cursor SCROLL options should keep SCROLL on the cursor declaration, not OPEN 1`] = `
326+
"DECLARE
327+
c SCROLL CURSOR FOR SELECT id FROM s.t ORDER BY id;
328+
v int;
329+
BEGIN
330+
OPEN c;
331+
FETCH PRIOR FROM c INTO v;
332+
CLOSE c;
333+
RETURN v;
334+
END"
335+
`;
336+
337+
exports[`plpgsql-deparser bug fixes cursor SCROLL options should not add SCROLL to OPEN of a plain bound cursor 1`] = `
338+
"DECLARE
339+
c CURSOR FOR SELECT id FROM s.t;
340+
v int;
341+
BEGIN
342+
OPEN c;
343+
FETCH FROM c INTO v;
344+
CLOSE c;
345+
END"
346+
`;
347+
279348
exports[`plpgsql-deparser bug fixes deep nesting and sequential blocks should handle block inside exception handler 1`] = `
280349
"BEGIN
281350
BEGIN
@@ -444,6 +513,18 @@ BEGIN
444513
END"
445514
`;
446515
516+
exports[`plpgsql-deparser bug fixes subscripted assignment targets should not parenthesize array element assignment targets 1`] = `
517+
"DECLARE
518+
a int[] := ARRAY[1, 2, 3, 4, 5];
519+
m int[][] := ARRAY[ARRAY[1, 2], ARRAY[3, 4]];
520+
BEGIN
521+
a[2] := 20;
522+
a[2:3] := ARRAY[9, 9];
523+
m[1][2] := 42;
524+
RETURN a;
525+
END"
526+
`;
527+
447528
exports[`plpgsql-deparser bug fixes untested statement types should handle ASSERT statement 1`] = `
448529
"BEGIN
449530
ASSERT p_x > 0, 'x must be positive';

‎packages/plpgsql-deparser/__tests__/__snapshots__/hydrate-demo.test.ts.snap‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,12 +177,12 @@ BEGIN
177177
EXCEPTION
178178
WHEN unique_violation THEN
179179
RAISE NOTICE 'unique_violation: %', sqlerrm;
180-
RAISE EXCEPTION;
180+
RAISE;
181181
WHEN others THEN
182182
IF p_debug THEN
183183
RAISE NOTICE 'error: % (%:%)', sqlerrm, sqlstate, sqlerrm;
184184
END IF;
185-
RAISE EXCEPTION;
185+
RAISE;
186186
END;
187187
END$$"
188188
`;

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

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1050,4 +1050,168 @@ END$$`;
10501050
expect(deparsed).not.toMatch(/RETURN;/);
10511051
});
10521052
});
1053+
1054+
describe('cursor SCROLL options', () => {
1055+
it('should keep SCROLL on the cursor declaration, not OPEN', async () => {
1056+
const sql = `CREATE FUNCTION test_scroll_cursor() RETURNS integer
1057+
LANGUAGE plpgsql AS $$
1058+
DECLARE
1059+
c SCROLL CURSOR FOR SELECT id FROM s.t ORDER BY id;
1060+
v int;
1061+
BEGIN
1062+
OPEN c;
1063+
FETCH PRIOR FROM c INTO v;
1064+
CLOSE c;
1065+
RETURN v;
1066+
END$$`;
1067+
1068+
await testUtils.expectAstMatch('scroll cursor declaration', sql);
1069+
1070+
const parsed = parsePlPgSQLSync(sql) as unknown as PLpgSQLParseResult;
1071+
const deparsed = deparseSync(parsed);
1072+
expect(deparsed).toMatchSnapshot();
1073+
expect(deparsed).toContain('SCROLL CURSOR FOR');
1074+
expect(deparsed).not.toMatch(/OPEN\s+(NO\s+)?SCROLL/i);
1075+
expect(deparsed).not.toMatch(/OPEN\s+c\s+(NO\s+)?SCROLL/i);
1076+
});
1077+
1078+
it('should not add SCROLL to OPEN of a plain bound cursor', async () => {
1079+
const sql = `CREATE FUNCTION test_plain_cursor() RETURNS void
1080+
LANGUAGE plpgsql AS $$
1081+
DECLARE
1082+
c CURSOR FOR SELECT id FROM s.t;
1083+
v int;
1084+
BEGIN
1085+
OPEN c;
1086+
FETCH c INTO v;
1087+
CLOSE c;
1088+
END$$`;
1089+
1090+
await testUtils.expectAstMatch('plain cursor open', sql);
1091+
1092+
const parsed = parsePlPgSQLSync(sql) as unknown as PLpgSQLParseResult;
1093+
const deparsed = deparseSync(parsed);
1094+
expect(deparsed).toMatchSnapshot();
1095+
expect(deparsed).toContain('OPEN c');
1096+
expect(deparsed).not.toMatch(/SCROLL/i);
1097+
});
1098+
});
1099+
1100+
describe('FETCH/MOVE directions', () => {
1101+
it('should preserve counts and FIRST/LAST directions', async () => {
1102+
const sql = `CREATE FUNCTION test_move_directions() RETURNS void
1103+
LANGUAGE plpgsql AS $$
1104+
DECLARE
1105+
c SCROLL CURSOR FOR SELECT id FROM s.t ORDER BY id;
1106+
BEGIN
1107+
OPEN c;
1108+
MOVE FORWARD 3 FROM c;
1109+
MOVE BACKWARD 2 FROM c;
1110+
MOVE FORWARD ALL FROM c;
1111+
MOVE BACKWARD ALL FROM c;
1112+
MOVE LAST IN c;
1113+
MOVE FIRST IN c;
1114+
CLOSE c;
1115+
END$$`;
1116+
1117+
await testUtils.expectAstMatch('move directions', sql);
1118+
1119+
const parsed = parsePlPgSQLSync(sql) as unknown as PLpgSQLParseResult;
1120+
const deparsed = deparseSync(parsed);
1121+
expect(deparsed).toMatchSnapshot();
1122+
expect(deparsed).toContain('FORWARD 3');
1123+
expect(deparsed).toContain('BACKWARD 2');
1124+
expect(deparsed).toContain('BACKWARD ALL');
1125+
expect(deparsed).toContain('LAST');
1126+
expect(deparsed).toContain('FIRST');
1127+
});
1128+
});
1129+
1130+
describe('SQLSTATE exception conditions', () => {
1131+
it('should emit SQLSTATE codes with the SQLSTATE keyword', async () => {
1132+
const sql = `CREATE FUNCTION test_sqlstate_cond() RETURNS integer
1133+
LANGUAGE plpgsql AS $$
1134+
BEGIN
1135+
RETURN 1;
1136+
EXCEPTION
1137+
WHEN unique_violation OR SQLSTATE '23503' THEN
1138+
RETURN -1;
1139+
WHEN SQLSTATE 'P0001' THEN
1140+
RETURN -2;
1141+
END$$`;
1142+
1143+
await testUtils.expectAstMatch('sqlstate condition', sql);
1144+
1145+
const parsed = parsePlPgSQLSync(sql) as unknown as PLpgSQLParseResult;
1146+
const deparsed = deparseSync(parsed);
1147+
expect(deparsed).toMatchSnapshot();
1148+
expect(deparsed).toContain(`SQLSTATE '23503'`);
1149+
expect(deparsed).toContain(`SQLSTATE 'P0001'`);
1150+
expect(deparsed).not.toMatch(/WHEN\s+23503/);
1151+
});
1152+
});
1153+
1154+
describe('bare RAISE re-throw', () => {
1155+
it('should keep a bare RAISE bare (not RAISE EXCEPTION;)', async () => {
1156+
const sql = `CREATE FUNCTION test_bare_raise() RETURNS void
1157+
LANGUAGE plpgsql AS $$
1158+
BEGIN
1159+
PERFORM 1;
1160+
EXCEPTION
1161+
WHEN OTHERS THEN
1162+
RAISE;
1163+
END$$`;
1164+
1165+
await testUtils.expectAstMatch('bare raise', sql);
1166+
1167+
const parsed = parsePlPgSQLSync(sql) as unknown as PLpgSQLParseResult;
1168+
const deparsed = deparseSync(parsed);
1169+
expect(deparsed).toMatchSnapshot();
1170+
expect(deparsed).toMatch(/RAISE;/);
1171+
expect(deparsed).not.toMatch(/RAISE EXCEPTION;/);
1172+
});
1173+
1174+
it('should still emit level and message for normal RAISE', async () => {
1175+
const sql = `CREATE FUNCTION test_raise_msg() RETURNS void
1176+
LANGUAGE plpgsql AS $$
1177+
BEGIN
1178+
RAISE EXCEPTION 'boom %', 42 USING HINT = 'h';
1179+
RAISE NOTICE 'hi';
1180+
END$$`;
1181+
1182+
await testUtils.expectAstMatch('raise with message', sql);
1183+
1184+
const parsed = parsePlPgSQLSync(sql) as unknown as PLpgSQLParseResult;
1185+
const deparsed = deparseSync(parsed);
1186+
expect(deparsed).toMatchSnapshot();
1187+
expect(deparsed).toContain(`RAISE EXCEPTION 'boom %', 42`);
1188+
expect(deparsed).toContain(`RAISE NOTICE 'hi'`);
1189+
});
1190+
});
1191+
1192+
describe('subscripted assignment targets', () => {
1193+
it('should not parenthesize array element assignment targets', async () => {
1194+
const sql = `CREATE FUNCTION test_array_assign() RETURNS int[]
1195+
LANGUAGE plpgsql AS $$
1196+
DECLARE
1197+
a int[] := ARRAY[1, 2, 3, 4, 5];
1198+
m int[][] := ARRAY[ARRAY[1, 2], ARRAY[3, 4]];
1199+
BEGIN
1200+
a[2] := 20;
1201+
a[2:3] := ARRAY[9, 9];
1202+
m[1][2] := 42;
1203+
RETURN a;
1204+
END$$`;
1205+
1206+
await testUtils.expectAstMatch('array element assignment', sql);
1207+
1208+
const parsed = parsePlPgSQLSync(sql) as unknown as PLpgSQLParseResult;
1209+
const deparsed = deparseSync(parsed);
1210+
expect(deparsed).toMatchSnapshot();
1211+
expect(deparsed).toContain('a[2] := 20');
1212+
expect(deparsed).toContain('m[1][2] := 42');
1213+
expect(deparsed).not.toMatch(/\(a\)\[/);
1214+
expect(deparsed).not.toMatch(/\(m\)\[/);
1215+
});
1216+
});
10531217
});

0 commit comments

Comments
 (0)