From 8b162ff2e2422dcc9ef988e3521f6a5516713273 Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Sun, 19 Jul 2026 22:58:29 +0000 Subject: [PATCH] fix(plpgsql-deparser): re-insert INTO after DML RETURNING and skip implicit final RETURN --- __fixtures__/plpgsql-generated/generated.json | 9 + .../plpgsql/plpgsql_deparser_fixes.sql | 84 +++++++++ .../__snapshots__/deparser-fixes.test.ts.snap | 82 +++++++- .../__snapshots__/hydrate-demo.test.ts.snap | 1 - .../schema-rename-mapped.test.ts.snap | 2 - .../__tests__/deparser-fixes.test.ts | 178 ++++++++++++++++++ .../__snapshots__/plpgsql-pretty.test.ts.snap | 4 - .../plpgsql-deparser/src/plpgsql-deparser.ts | 56 +++++- 8 files changed, 401 insertions(+), 15 deletions(-) diff --git a/__fixtures__/plpgsql-generated/generated.json b/__fixtures__/plpgsql-generated/generated.json index 13f6bf311..c4eb4bf68 100644 --- a/__fixtures__/plpgsql-generated/generated.json +++ b/__fixtures__/plpgsql-generated/generated.json @@ -136,6 +136,15 @@ "plpgsql_deparser_fixes-35.sql": "-- Test 35: CALL statement\nCREATE FUNCTION test_call_statement() RETURNS void\nLANGUAGE plpgsql AS $$\nBEGIN\n CALL my_procedure(1, 'hello');\n RETURN;\nEND$$", "plpgsql_deparser_fixes-36.sql": "-- =============================================================================\n-- Edge Case Tests: Real-World Patterns\n-- =============================================================================\n\n-- Test 36: Permission bitnum trigger pattern (the function that exposed the END; bug)\nCREATE FUNCTION test_permission_bitnum_trigger() RETURNS trigger\nLANGUAGE plpgsql AS $$\nDECLARE\n bitlen int;\n v_len int;\nBEGIN\n v_len := 32;\n BEGIN\n bitlen := bit_length(NEW.bitstr);\n EXCEPTION\n WHEN others THEN\n bitlen := 0;\n END;\n IF bitlen = 0 THEN\n NEW.bitstr := lpad('', v_len, '0');\n END IF;\n RETURN NEW;\nEND$$", "plpgsql_deparser_fixes-37.sql": "-- Test 37: Multi-step sign-in pattern (deeply nested IF chains)\nCREATE FUNCTION test_signin_pattern(v_email text) RETURNS record\nLANGUAGE plpgsql AS $$\nDECLARE\n v_user record;\n v_secret record;\nBEGIN\n SELECT * INTO v_user FROM users WHERE email = v_email;\n IF NOT FOUND THEN\n RAISE EXCEPTION 'USER_NOT_FOUND';\n END IF;\n SELECT * INTO v_secret FROM secrets WHERE user_id = v_user.id;\n IF NOT FOUND THEN\n RAISE EXCEPTION 'NO_CREDENTIALS';\n END IF;\n IF v_secret.locked_at IS NOT NULL THEN\n RAISE EXCEPTION 'ACCOUNT_LOCKED';\n END IF;\n RETURN v_user;\nEND$$", + "plpgsql_deparser_fixes-38.sql": "-- Test 38: INSERT ... RETURNING ... INTO (INTO must be re-inserted after RETURNING)\nCREATE FUNCTION test_insert_returning_into() RETURNS uuid\nLANGUAGE plpgsql AS $$\nDECLARE\n v_id uuid;\nBEGIN\n INSERT INTO s.t (name) VALUES ('x') RETURNING id INTO v_id;\n RETURN v_id;\nEND$$", + "plpgsql_deparser_fixes-39.sql": "-- Test 39: UPDATE ... RETURNING ... INTO\nCREATE FUNCTION test_update_returning_into() RETURNS uuid\nLANGUAGE plpgsql AS $$\nDECLARE\n v_id uuid;\nBEGIN\n UPDATE s.t SET name = 'y' WHERE name = 'x' RETURNING id INTO v_id;\n RETURN v_id;\nEND$$", + "plpgsql_deparser_fixes-40.sql": "-- Test 40: DELETE ... RETURNING ... INTO\nCREATE FUNCTION test_delete_returning_into() RETURNS uuid\nLANGUAGE plpgsql AS $$\nDECLARE\n v_id uuid;\nBEGIN\n DELETE FROM s.t WHERE name = 'x' RETURNING id INTO v_id;\n RETURN v_id;\nEND$$", + "plpgsql_deparser_fixes-41.sql": "-- Test 41: INSERT ... RETURNING ... INTO STRICT\nCREATE FUNCTION test_insert_returning_into_strict() RETURNS uuid\nLANGUAGE plpgsql AS $$\nDECLARE\n v_id uuid;\nBEGIN\n INSERT INTO s.t (name) VALUES ('x') RETURNING id INTO STRICT v_id;\n RETURN v_id;\nEND$$", + "plpgsql_deparser_fixes-42.sql": "-- Test 42: INSERT ... RETURNING multiple columns INTO\nCREATE FUNCTION test_insert_returning_multi_into() RETURNS void\nLANGUAGE plpgsql AS $$\nDECLARE\n v_id uuid;\n v_name text;\nBEGIN\n INSERT INTO s.t (name) VALUES ('x') RETURNING id, name INTO v_id, v_name;\nEND$$", + "plpgsql_deparser_fixes-43.sql": "-- Test 43: INSERT ... RETURNING expression with subquery INTO (INTO must not land inside the subquery)\nCREATE FUNCTION test_insert_returning_subquery_into() RETURNS void\nLANGUAGE plpgsql AS $$\nDECLARE\n v_total bigint;\nBEGIN\n INSERT INTO s.t (name) VALUES ('x') RETURNING (SELECT count(*) FROM s.t WHERE name = 'x') INTO v_total;\nEND$$", + "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$$", + "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$$", + "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$$", "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$$", diff --git a/__fixtures__/plpgsql/plpgsql_deparser_fixes.sql b/__fixtures__/plpgsql/plpgsql_deparser_fixes.sql index 84bf64dfb..30f025440 100644 --- a/__fixtures__/plpgsql/plpgsql_deparser_fixes.sql +++ b/__fixtures__/plpgsql/plpgsql_deparser_fixes.sql @@ -505,3 +505,87 @@ BEGIN END IF; RETURN v_user; END$$; + +-- Test 38: INSERT ... RETURNING ... INTO (INTO must be re-inserted after RETURNING) +CREATE FUNCTION test_insert_returning_into() RETURNS uuid +LANGUAGE plpgsql AS $$ +DECLARE + v_id uuid; +BEGIN + INSERT INTO s.t (name) VALUES ('x') RETURNING id INTO v_id; + RETURN v_id; +END$$; + +-- Test 39: UPDATE ... RETURNING ... INTO +CREATE FUNCTION test_update_returning_into() RETURNS uuid +LANGUAGE plpgsql AS $$ +DECLARE + v_id uuid; +BEGIN + UPDATE s.t SET name = 'y' WHERE name = 'x' RETURNING id INTO v_id; + RETURN v_id; +END$$; + +-- Test 40: DELETE ... RETURNING ... INTO +CREATE FUNCTION test_delete_returning_into() RETURNS uuid +LANGUAGE plpgsql AS $$ +DECLARE + v_id uuid; +BEGIN + DELETE FROM s.t WHERE name = 'x' RETURNING id INTO v_id; + RETURN v_id; +END$$; + +-- Test 41: INSERT ... RETURNING ... INTO STRICT +CREATE FUNCTION test_insert_returning_into_strict() RETURNS uuid +LANGUAGE plpgsql AS $$ +DECLARE + v_id uuid; +BEGIN + INSERT INTO s.t (name) VALUES ('x') RETURNING id INTO STRICT v_id; + RETURN v_id; +END$$; + +-- Test 42: INSERT ... RETURNING multiple columns INTO +CREATE FUNCTION test_insert_returning_multi_into() RETURNS void +LANGUAGE plpgsql AS $$ +DECLARE + v_id uuid; + v_name text; +BEGIN + INSERT INTO s.t (name) VALUES ('x') RETURNING id, name INTO v_id, v_name; +END$$; + +-- Test 43: INSERT ... RETURNING expression with subquery INTO (INTO must not land inside the subquery) +CREATE FUNCTION test_insert_returning_subquery_into() RETURNS void +LANGUAGE plpgsql AS $$ +DECLARE + v_total bigint; +BEGIN + INSERT INTO s.t (name) VALUES ('x') RETURNING (SELECT count(*) FROM s.t WHERE name = 'x') INTO v_total; +END$$; + +-- Test 44: Trigger function with no final return (implicit compiler RETURN must not be emitted) +CREATE FUNCTION test_trigger_no_final_return() RETURNS trigger +LANGUAGE plpgsql AS $$ +BEGIN + IF TG_OP = 'INSERT' THEN + RETURN NEW; + END IF; +END$$; + +-- Test 45: Void function with explicit trailing RETURN (must be preserved) +CREATE FUNCTION test_void_explicit_return() RETURNS void +LANGUAGE plpgsql AS $$ +BEGIN + RAISE NOTICE 'hi'; + RETURN; +END$$; + +-- Test 46: Trigger function ending in RETURN NEW (unchanged) +CREATE FUNCTION test_trigger_return_new() RETURNS trigger +LANGUAGE plpgsql AS $$ +BEGIN + NEW.updated_at := now(); + RETURN NEW; +END$$; diff --git a/packages/plpgsql-deparser/__tests__/__snapshots__/deparser-fixes.test.ts.snap b/packages/plpgsql-deparser/__tests__/__snapshots__/deparser-fixes.test.ts.snap index ac18db54e..a0deb9d6b 100644 --- a/packages/plpgsql-deparser/__tests__/__snapshots__/deparser-fixes.test.ts.snap +++ b/packages/plpgsql-deparser/__tests__/__snapshots__/deparser-fixes.test.ts.snap @@ -1,5 +1,58 @@ // Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing +exports[`plpgsql-deparser bug fixes DML RETURNING ... INTO re-insertion should handle multi-column RETURNING ... INTO 1`] = ` +"DECLARE + v_id uuid; + v_name text; +BEGIN + INSERT INTO s.t (name) VALUES ('x') RETURNING id, name INTO v_id, v_name; +END" +`; + +exports[`plpgsql-deparser bug fixes DML RETURNING ... INTO re-insertion should not insert INTO inside a RETURNING subquery 1`] = ` +"DECLARE + v_total bigint; +BEGIN + INSERT INTO s.t (name) VALUES ('x') RETURNING (SELECT count(*) FROM s.t WHERE name = 'x') INTO v_total; +END" +`; + +exports[`plpgsql-deparser bug fixes DML RETURNING ... INTO re-insertion should preserve STRICT in RETURNING ... INTO STRICT 1`] = ` +"DECLARE + v_id uuid; +BEGIN + INSERT INTO s.t (name) VALUES ('x') RETURNING id INTO STRICT v_id; + RETURN v_id; +END" +`; + +exports[`plpgsql-deparser bug fixes DML RETURNING ... INTO re-insertion should re-insert INTO after RETURNING for DELETE 1`] = ` +"DECLARE + v_id uuid; +BEGIN + DELETE FROM s.t WHERE name = 'x' RETURNING id INTO v_id; + RETURN v_id; +END" +`; + +exports[`plpgsql-deparser bug fixes DML RETURNING ... INTO re-insertion should re-insert INTO after RETURNING for INSERT 1`] = ` +"DECLARE + v_id uuid; +BEGIN + INSERT INTO s.t (name) VALUES ('x') RETURNING id INTO v_id; + RETURN v_id; +END" +`; + +exports[`plpgsql-deparser bug fixes DML RETURNING ... INTO re-insertion should re-insert INTO after RETURNING for UPDATE 1`] = ` +"DECLARE + v_id uuid; +BEGIN + UPDATE s.t SET name = 'y' WHERE name = 'x' RETURNING id INTO v_id; + RETURN v_id; +END" +`; + exports[`plpgsql-deparser bug fixes INTO clause depth-aware scanner should handle INTO STRICT 1`] = ` "DECLARE v_id integer; @@ -74,7 +127,6 @@ exports[`plpgsql-deparser bug fixes OUT parameters with SELECT INTO multiple var "BEGIN SELECT u.name, u.email INTO STRICT name, email FROM users u WHERE u.id = p_id; - RETURN; END" `; @@ -98,21 +150,18 @@ exports[`plpgsql-deparser bug fixes PERFORM SELECT fix should handle PERFORM wit "BEGIN PERFORM set_config('search_path', 'public', true); PERFORM nextval('my_sequence'); - RETURN; END" `; exports[`plpgsql-deparser bug fixes PERFORM SELECT fix should handle PERFORM with subquery 1`] = ` "BEGIN PERFORM 1 FROM users WHERE id = 1; - RETURN; END" `; exports[`plpgsql-deparser bug fixes PERFORM SELECT fix should strip SELECT keyword from PERFORM statements 1`] = ` "BEGIN PERFORM pg_sleep(1); - RETURN; END" `; @@ -139,7 +188,6 @@ BEGIN FOR r IN SELECT id, name FROM users LOOP RAISE NOTICE 'User: % - %', r.id, r.name; END LOOP; - RETURN; END" `; @@ -241,7 +289,6 @@ exports[`plpgsql-deparser bug fixes deep nesting and sequential blocks should ha RAISE NOTICE 'even logging failed'; END; END; - RETURN; END" `; @@ -269,6 +316,28 @@ exports[`plpgsql-deparser bug fixes deep nesting and sequential blocks should ha END" `; +exports[`plpgsql-deparser bug fixes implicit trailing RETURN suppression should leave trigger function ending in RETURN NEW unchanged 1`] = ` +"BEGIN + NEW.updated_at := now(); + RETURN NEW; +END" +`; + +exports[`plpgsql-deparser bug fixes implicit trailing RETURN suppression should not emit implicit compiler-generated RETURN in trigger function 1`] = ` +"BEGIN + IF TG_OP = 'INSERT' THEN + RETURN NEW; + END IF; +END" +`; + +exports[`plpgsql-deparser bug fixes implicit trailing RETURN suppression should preserve explicit trailing RETURN in void function 1`] = ` +"BEGIN + RAISE NOTICE 'hi'; + RETURN; +END" +`; + exports[`plpgsql-deparser bug fixes nested block compositions (END; bug class) should handle labeled nested block 1`] = ` "BEGIN <> @@ -423,7 +492,6 @@ END" exports[`plpgsql-deparser bug fixes untested statement types should handle RETURN QUERY 1`] = ` "BEGIN RETURN QUERY SELECT id, name FROM my_table WHERE active = TRUE; - RETURN; END" `; 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 dd9ac82ee..86583099e 100644 --- a/packages/plpgsql-deparser/__tests__/__snapshots__/hydrate-demo.test.ts.snap +++ b/packages/plpgsql-deparser/__tests__/__snapshots__/hydrate-demo.test.ts.snap @@ -184,6 +184,5 @@ BEGIN END IF; RAISE EXCEPTION; END; - RETURN; END$$" `; diff --git a/packages/plpgsql-deparser/__tests__/__snapshots__/schema-rename-mapped.test.ts.snap b/packages/plpgsql-deparser/__tests__/__snapshots__/schema-rename-mapped.test.ts.snap index 4246b2f37..6a0106138 100644 --- a/packages/plpgsql-deparser/__tests__/__snapshots__/schema-rename-mapped.test.ts.snap +++ b/packages/plpgsql-deparser/__tests__/__snapshots__/schema-rename-mapped.test.ts.snap @@ -227,7 +227,6 @@ CREATE FUNCTION myapp_v2.update_user_status( changed_at ) VALUES (p_user_id, p_status, now()); - RETURN; END$$; CREATE FUNCTION myapp_v2.cleanup_old_sessions( @@ -403,6 +402,5 @@ BEGIN UPDATE myapp_v2.batch_items SET status = 'processed' WHERE id = item.id; END LOOP; UPDATE myapp_v2.batches SET status = 'completed',completed_at = now() WHERE id = p_batch_id; - RETURN; END$$;" `; diff --git a/packages/plpgsql-deparser/__tests__/deparser-fixes.test.ts b/packages/plpgsql-deparser/__tests__/deparser-fixes.test.ts index 4b068ff5e..bedd24a29 100644 --- a/packages/plpgsql-deparser/__tests__/deparser-fixes.test.ts +++ b/packages/plpgsql-deparser/__tests__/deparser-fixes.test.ts @@ -872,4 +872,182 @@ END$$`; expect(deparsed).toMatchSnapshot(); }); }); + + describe('DML RETURNING ... INTO re-insertion', () => { + const countKeyword = (sql: string, keyword: string): number => + (sql.match(new RegExp(`\\b${keyword}\\b`, 'gi')) || []).length; + + const expectKeywordsPreserved = (original: string, deparsed: string) => { + expect(countKeyword(deparsed, 'INTO')).toBeGreaterThanOrEqual(countKeyword(original, 'INTO')); + expect(countKeyword(deparsed, 'RETURNING')).toBeGreaterThanOrEqual(countKeyword(original, 'RETURNING')); + }; + + it('should re-insert INTO after RETURNING for INSERT', async () => { + const sql = `CREATE FUNCTION test_insert_returning_into() RETURNS uuid +LANGUAGE plpgsql AS $$ +DECLARE + v_id uuid; +BEGIN + INSERT INTO s.t (name) VALUES ('x') RETURNING id INTO v_id; + RETURN v_id; +END$$`; + + await testUtils.expectAstMatch('INSERT RETURNING INTO', sql); + + const parsed = parsePlPgSQLSync(sql) as unknown as PLpgSQLParseResult; + const deparsed = deparseSync(parsed); + expect(deparsed).toMatchSnapshot(); + expect(deparsed).toContain('RETURNING id INTO v_id'); + expectKeywordsPreserved(sql, deparsed); + }); + + it('should re-insert INTO after RETURNING for UPDATE', async () => { + const sql = `CREATE FUNCTION test_update_returning_into() RETURNS uuid +LANGUAGE plpgsql AS $$ +DECLARE + v_id uuid; +BEGIN + UPDATE s.t SET name = 'y' WHERE name = 'x' RETURNING id INTO v_id; + RETURN v_id; +END$$`; + + await testUtils.expectAstMatch('UPDATE RETURNING INTO', sql); + + const parsed = parsePlPgSQLSync(sql) as unknown as PLpgSQLParseResult; + const deparsed = deparseSync(parsed); + expect(deparsed).toMatchSnapshot(); + expect(deparsed).toContain('RETURNING id INTO v_id'); + expectKeywordsPreserved(sql, deparsed); + }); + + it('should re-insert INTO after RETURNING for DELETE', async () => { + const sql = `CREATE FUNCTION test_delete_returning_into() RETURNS uuid +LANGUAGE plpgsql AS $$ +DECLARE + v_id uuid; +BEGIN + DELETE FROM s.t WHERE name = 'x' RETURNING id INTO v_id; + RETURN v_id; +END$$`; + + await testUtils.expectAstMatch('DELETE RETURNING INTO', sql); + + const parsed = parsePlPgSQLSync(sql) as unknown as PLpgSQLParseResult; + const deparsed = deparseSync(parsed); + expect(deparsed).toMatchSnapshot(); + expect(deparsed).toContain('RETURNING id INTO v_id'); + expectKeywordsPreserved(sql, deparsed); + }); + + it('should preserve STRICT in RETURNING ... INTO STRICT', async () => { + const sql = `CREATE FUNCTION test_insert_returning_into_strict() RETURNS uuid +LANGUAGE plpgsql AS $$ +DECLARE + v_id uuid; +BEGIN + INSERT INTO s.t (name) VALUES ('x') RETURNING id INTO STRICT v_id; + RETURN v_id; +END$$`; + + await testUtils.expectAstMatch('INSERT RETURNING INTO STRICT', sql); + + const parsed = parsePlPgSQLSync(sql) as unknown as PLpgSQLParseResult; + const deparsed = deparseSync(parsed); + expect(deparsed).toMatchSnapshot(); + expect(deparsed).toContain('RETURNING id INTO STRICT v_id'); + expectKeywordsPreserved(sql, deparsed); + }); + + it('should handle multi-column RETURNING ... INTO', async () => { + const sql = `CREATE FUNCTION test_insert_returning_multi_into() RETURNS void +LANGUAGE plpgsql AS $$ +DECLARE + v_id uuid; + v_name text; +BEGIN + INSERT INTO s.t (name) VALUES ('x') RETURNING id, name INTO v_id, v_name; +END$$`; + + await testUtils.expectAstMatch('INSERT RETURNING multi INTO', sql); + + const parsed = parsePlPgSQLSync(sql) as unknown as PLpgSQLParseResult; + const deparsed = deparseSync(parsed); + expect(deparsed).toMatchSnapshot(); + expect(deparsed).toContain('RETURNING id, name INTO v_id, v_name'); + expectKeywordsPreserved(sql, deparsed); + }); + + it('should not insert INTO inside a RETURNING subquery', async () => { + const sql = `CREATE FUNCTION test_insert_returning_subquery_into() RETURNS void +LANGUAGE plpgsql AS $$ +DECLARE + v_total bigint; +BEGIN + INSERT INTO s.t (name) VALUES ('x') RETURNING (SELECT count(*) FROM s.t WHERE name = 'x') INTO v_total; +END$$`; + + await testUtils.expectAstMatch('INSERT RETURNING subquery INTO', sql); + + const parsed = parsePlPgSQLSync(sql) as unknown as PLpgSQLParseResult; + const deparsed = deparseSync(parsed); + expect(deparsed).toMatchSnapshot(); + // INTO must be appended after the subquery, not inside it + expect(deparsed).toContain(`(SELECT count(*) FROM s.t WHERE name = 'x') INTO v_total`); + expectKeywordsPreserved(sql, deparsed); + }); + }); + + describe('implicit trailing RETURN suppression', () => { + it('should not emit implicit compiler-generated RETURN in trigger function', async () => { + const sql = `CREATE FUNCTION test_trigger_no_final_return() RETURNS trigger +LANGUAGE plpgsql AS $$ +BEGIN + IF TG_OP = 'INSERT' THEN + RETURN NEW; + END IF; +END$$`; + + await testUtils.expectAstMatch('trigger no final return', sql); + + const parsed = parsePlPgSQLSync(sql) as unknown as PLpgSQLParseResult; + const deparsed = deparseSync(parsed); + expect(deparsed).toMatchSnapshot(); + // The compiler-generated implicit final RETURN must not be emitted + // (bare RETURN; is a syntax error inside trigger functions) + expect(deparsed).not.toMatch(/RETURN;/); + }); + + it('should preserve explicit trailing RETURN in void function', async () => { + const sql = `CREATE FUNCTION test_void_explicit_return() RETURNS void +LANGUAGE plpgsql AS $$ +BEGIN + RAISE NOTICE 'hi'; + RETURN; +END$$`; + + await testUtils.expectAstMatch('void explicit return', sql); + + const parsed = parsePlPgSQLSync(sql) as unknown as PLpgSQLParseResult; + const deparsed = deparseSync(parsed); + expect(deparsed).toMatchSnapshot(); + expect(deparsed).toMatch(/RETURN;/); + }); + + it('should leave trigger function ending in RETURN NEW unchanged', async () => { + const sql = `CREATE FUNCTION test_trigger_return_new() RETURNS trigger +LANGUAGE plpgsql AS $$ +BEGIN + NEW.updated_at := now(); + RETURN NEW; +END$$`; + + await testUtils.expectAstMatch('trigger return new', sql); + + const parsed = parsePlPgSQLSync(sql) as unknown as PLpgSQLParseResult; + const deparsed = deparseSync(parsed); + expect(deparsed).toMatchSnapshot(); + expect(deparsed).toContain('RETURN NEW'); + expect(deparsed).not.toMatch(/RETURN;/); + }); + }); }); 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 2f144a35e..2549e936f 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 @@ -173,7 +173,6 @@ begin end if; raise exception; end; - return; end" `; @@ -186,7 +185,6 @@ exports[`lowercase: if-else-function.sql 1`] = ` else return 'small'; end if; - return; end" `; @@ -402,7 +400,6 @@ BEGIN END IF; RAISE EXCEPTION; END; - RETURN; END" `; @@ -415,7 +412,6 @@ exports[`uppercase: if-else-function.sql 1`] = ` ELSE RETURN 'small'; END IF; - RETURN; END" `; diff --git a/packages/plpgsql-deparser/src/plpgsql-deparser.ts b/packages/plpgsql-deparser/src/plpgsql-deparser.ts index 8a88fbbf5..aff10bdce 100644 --- a/packages/plpgsql-deparser/src/plpgsql-deparser.ts +++ b/packages/plpgsql-deparser/src/plpgsql-deparser.ts @@ -213,12 +213,49 @@ export class PLpgSQLDeparser { // Deparse the action block (BEGIN...END) // Pass skipLabel=true since we already output the label if (func.action) { - parts.push(this.deparseStmt(func.action, context, blockLabel ? true : false)); + const action = this.stripImplicitFinalReturn(func.action); + parts.push(this.deparseStmt(action, context, blockLabel ? true : false)); } return parts.join(this.options.newline); } + /** + * Strip the compiler-generated implicit final RETURN from the top-level block. + * + * The PL/pgSQL compiler appends a bare RETURN node (no expr, no lineno) to every + * function body. Emitting it as `RETURN;` is invalid inside trigger functions + * (trigger RETURN requires an expression). An explicit user-written `RETURN;` + * carries a lineno and is preserved. + */ + private stripImplicitFinalReturn(action: PLpgSQLStmtNode): PLpgSQLStmtNode { + if (!('PLpgSQL_stmt_block' in action)) { + return action; + } + const block = action.PLpgSQL_stmt_block; + const body = block.body; + if (!body || body.length < 2) { + return action; + } + const last = body[body.length - 1]; + if (!('PLpgSQL_stmt_return' in last)) { + return action; + } + const ret = (last as any).PLpgSQL_stmt_return; + const isImplicit = ret.expr === undefined && + ret.lineno === undefined && + (ret.retvarno === undefined || ret.retvarno < 0); + if (!isImplicit) { + return action; + } + return { + PLpgSQL_stmt_block: { + ...block, + body: body.slice(0, -1), + }, + }; + } + /** * Collect line numbers of variables introduced by loop constructs. * Only adds a variable's lineno if it matches the loop statement's lineno, @@ -1530,6 +1567,11 @@ export class PLpgSQLDeparser { // Clause keywords that end the SELECT target list at depth 0 const clauseKeywords = ['FROM', 'WHERE', 'GROUP', 'HAVING', 'WINDOW', 'ORDER', 'LIMIT', 'OFFSET', 'FETCH', 'FOR', 'UNION', 'INTERSECT', 'EXCEPT']; + // DML command keywords: for INSERT/UPDATE/DELETE/MERGE ... RETURNING the INTO + // clause goes at the end of the statement, after the RETURNING list. The + // SELECT clause-keyword scan does not apply (and "INSERT INTO" must not be + // mistaken for an existing INTO target clause). + const dmlKeywords = ['INSERT', 'UPDATE', 'DELETE', 'MERGE']; for (let i = 0; i < len; i++) { const char = sql[i]; @@ -1644,6 +1686,18 @@ export class PLpgSQLDeparser { const isWordBoundary = /\s/.test(prevChar) || prevChar === '(' || prevChar === ')' || prevChar === ',' || i === 0; if (isWordBoundary) { + // DML statement: append INTO at the end (after the RETURNING list) + for (const keyword of dmlKeywords) { + const pattern = new RegExp(`^${keyword}\\b`, 'i'); + if (pattern.test(upperSql.slice(i))) { + let insertPos = len; + while (insertPos > 0 && /\s/.test(sql[insertPos - 1])) { + insertPos--; + } + return insertPos; + } + } + // Check if INTO already exists at depth 0 if (/^INTO\s/i.test(upperSql.slice(i))) { return -1;