diff --git a/__fixtures__/plpgsql-generated/generated.json b/__fixtures__/plpgsql-generated/generated.json index d617cb68..f14cbc70 100644 --- a/__fixtures__/plpgsql-generated/generated.json +++ b/__fixtures__/plpgsql-generated/generated.json @@ -100,8 +100,10 @@ "plpgsql_deparser_fixes-11.sql": "-- Test 11: OUT parameter function with bare RETURN\nCREATE FUNCTION test_out_params(OUT ok boolean, OUT message text)\nLANGUAGE plpgsql AS $$\nBEGIN\n ok := true;\n message := 'success';\n RETURN;\nEND$$", "plpgsql_deparser_fixes-12.sql": "-- Test 12: RETURNS TABLE function with RETURN QUERY\nCREATE FUNCTION test_returns_table(p_prefix text)\nRETURNS TABLE(id int, name text)\nLANGUAGE plpgsql AS $$\nBEGIN\n RETURN QUERY SELECT 1, p_prefix || '_one';\n RETURN QUERY SELECT 2, p_prefix || '_two';\n RETURN;\nEND$$", "plpgsql_deparser_fixes-13.sql": "-- Test 13: Trigger function with complex logic\nCREATE FUNCTION test_trigger_complex() RETURNS trigger\nLANGUAGE plpgsql AS $$\nDECLARE\n defaults_record record;\n bit_len int;\nBEGIN\n bit_len := bit_length(NEW.permissions);\n \n SELECT * INTO defaults_record\n FROM permission_defaults AS t\n LIMIT 1;\n \n IF found THEN\n NEW.is_approved := defaults_record.is_approved;\n NEW.is_verified := defaults_record.is_verified;\n END IF;\n \n IF NEW.is_owner IS TRUE THEN\n NEW.is_admin := true;\n NEW.is_approved := true;\n NEW.is_verified := true;\n END IF;\n \n SELECT\n NEW.is_approved IS TRUE\n AND NEW.is_verified IS TRUE\n AND NEW.is_disabled IS FALSE INTO NEW.is_active;\n \n RETURN NEW;\nEND$$", - "plpgsql_deparser_fixes-14.sql": "-- Test 14: Procedure (implicit void return)\nCREATE PROCEDURE test_procedure(p_message text)\nLANGUAGE plpgsql AS $$\nBEGIN\n RAISE NOTICE '%', p_message;\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_deparser_fixes-14.sql": "-- Test 14: Procedure (implicit void return)\nCREATE PROCEDURE test_procedure(p_message text)\nLANGUAGE plpgsql AS $$\nBEGIN\n RAISE NOTICE '%', p_message;\nEND$$", + "plpgsql_deparser_fixes-15.sql": "-- Test 15: OUT parameters with SELECT INTO multiple variables\n-- This pattern is used in auth functions (sign_in, sign_up) where we need to\n-- populate multiple OUT parameters from a single SELECT statement\nCREATE FUNCTION test_out_params_select_into(\n p_user_id uuid,\n OUT id uuid,\n OUT user_id uuid,\n OUT access_token text,\n OUT access_token_expires_at timestamptz,\n OUT is_verified boolean,\n OUT totp_enabled boolean\n)\nLANGUAGE plpgsql AS $$\nDECLARE\n v_token_id uuid;\n v_plaintext_token text;\nBEGIN\n v_plaintext_token := encode(gen_random_bytes(48), 'hex');\n v_token_id := uuid_generate_v5(uuid_ns_url(), v_plaintext_token);\n \n INSERT INTO tokens (id, user_id, access_token_hash)\n VALUES (v_token_id, p_user_id, digest(v_plaintext_token, 'sha256'));\n \n SELECT tkn.id, tkn.user_id, v_plaintext_token, tkn.access_token_expires_at, tkn.is_verified, tkn.totp_enabled\n INTO id, user_id, access_token, access_token_expires_at, is_verified, totp_enabled\n FROM tokens AS tkn\n WHERE tkn.id = v_token_id;\n \n RETURN;\nEND$$", + "plpgsql_deparser_fixes-16.sql": "-- Test 16: OUT parameters with SELECT INTO and STRICT\nCREATE FUNCTION test_out_params_strict(\n p_id uuid,\n OUT name text,\n OUT email text\n)\nLANGUAGE plpgsql AS $$\nBEGIN\n SELECT u.name, u.email INTO STRICT name, email\n FROM users u\n WHERE u.id = p_id;\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$$", "plpgsql_control-4.sql": "do $$\nbegin\n for i in reverse 1..3 by -1 loop\n raise notice 'reverse 1..3 by -1: i = %', i;\n end loop;\nend$$", @@ -189,4 +191,4 @@ "plpgsql_array-21.sql": "-- some types don't support arrays\ndo $$\ndeclare\n v pg_node_tree;\n v1 v%type[];\nbegin\nend;\n$$", "plpgsql_array-22.sql": "-- check functionality\ndo $$\ndeclare\n v1 int;\n v2 varchar;\n a1 v1%type[];\n a2 v2%type[];\nbegin\n v1 := 10;\n v2 := 'Hi';\n a1 := array[v1,v1];\n a2 := array[v2,v2];\n raise notice '% %', a1, a2;\nend;\n$$", "plpgsql_array-23.sql": "do $$\ndeclare tg array_test_table%rowtype[];\nbegin\n tg := array(select array_test_table from array_test_table);\n raise notice '%', tg;\n tg := array(select row(a,b) from array_test_table);\n raise notice '%', tg;\nend;\n$$" -} \ No newline at end of file +} diff --git a/__fixtures__/plpgsql/plpgsql_deparser_fixes.sql b/__fixtures__/plpgsql/plpgsql_deparser_fixes.sql index 24698cd1..3c80cc59 100644 --- a/__fixtures__/plpgsql/plpgsql_deparser_fixes.sql +++ b/__fixtures__/plpgsql/plpgsql_deparser_fixes.sql @@ -162,3 +162,47 @@ LANGUAGE plpgsql AS $$ BEGIN RAISE NOTICE '%', p_message; END$$; + +-- Test 15: OUT parameters with SELECT INTO multiple variables +-- This pattern is used in auth functions (sign_in, sign_up) where we need to +-- populate multiple OUT parameters from a single SELECT statement +CREATE FUNCTION test_out_params_select_into( + p_user_id uuid, + OUT id uuid, + OUT user_id uuid, + OUT access_token text, + OUT access_token_expires_at timestamptz, + OUT is_verified boolean, + OUT totp_enabled boolean +) +LANGUAGE plpgsql AS $$ +DECLARE + v_token_id uuid; + v_plaintext_token text; +BEGIN + v_plaintext_token := encode(gen_random_bytes(48), 'hex'); + v_token_id := uuid_generate_v5(uuid_ns_url(), v_plaintext_token); + + INSERT INTO tokens (id, user_id, access_token_hash) + VALUES (v_token_id, p_user_id, digest(v_plaintext_token, 'sha256')); + + SELECT tkn.id, tkn.user_id, v_plaintext_token, tkn.access_token_expires_at, tkn.is_verified, tkn.totp_enabled + INTO id, user_id, access_token, access_token_expires_at, is_verified, totp_enabled + FROM tokens AS tkn + WHERE tkn.id = v_token_id; + + RETURN; +END$$; + +-- Test 16: OUT parameters with SELECT INTO and STRICT +CREATE FUNCTION test_out_params_strict( + p_id uuid, + OUT name text, + OUT email text +) +LANGUAGE plpgsql AS $$ +BEGIN + SELECT u.name, u.email INTO STRICT name, email + FROM users u + WHERE u.id = p_id; +END$$; diff --git a/packages/plpgsql-deparser/AGENTS.md b/packages/plpgsql-deparser/AGENTS.md new file mode 100644 index 00000000..6b22c839 --- /dev/null +++ b/packages/plpgsql-deparser/AGENTS.md @@ -0,0 +1,104 @@ +# PL/pgSQL Deparser - Agent Instructions + +## Adding Test Fixtures + +When adding new test fixtures for the PL/pgSQL deparser, follow this workflow: + +### Step 1: Add SQL Fixtures + +Add your PL/pgSQL function/procedure definitions to the appropriate fixture file in `__fixtures__/plpgsql/`. For deparser-specific fixes, use `plpgsql_deparser_fixes.sql`. + +Example fixture: +```sql +-- Test N: Description of what this tests +CREATE FUNCTION test_example(p_input text, OUT result text) +LANGUAGE plpgsql AS $$ +BEGIN + result := p_input; + RETURN; +END$$; +``` + +### Step 2: Generate Test Fixtures + +Run the fixture generation script from the plpgsql-deparser package: + +```bash +cd packages/plpgsql-deparser +pnpm fixtures +``` + +This script (`scripts/make-fixtures.ts`): +1. Reads all `.sql` files from `__fixtures__/plpgsql/` +2. Parses each file to extract PL/pgSQL statements (CREATE FUNCTION, CREATE PROCEDURE, DO blocks) +3. Validates each statement can be parsed by the PL/pgSQL parser +4. Outputs valid fixtures to `__fixtures__/plpgsql-generated/generated.json` + +### Step 3: Run Tests + +Run the test suite to verify your fixtures round-trip correctly: + +```bash +cd packages/plpgsql-deparser +pnpm test +``` + +The round-trip test (`__tests__/plpgsql-deparser.test.ts`): +1. Loads all fixtures from `generated.json` +2. For each fixture: parse -> deparse -> reparse +3. Compares the AST from original parse with the AST from reparsed output +4. Reports any failures (AST mismatches or reparse failures) + +### Step 4: Add Snapshot Tests (Optional but Recommended) + +For important deparser fixes, add explicit test cases with snapshots to `__tests__/deparser-fixes.test.ts`: + +```typescript +it('should handle [description]', async () => { + const sql = `CREATE FUNCTION test_example(...) +LANGUAGE plpgsql AS $$ +BEGIN + -- your test case +END$$`; + + await testUtils.expectAstMatch('description', sql); + + const parsed = parsePlPgSQLSync(sql) as unknown as PLpgSQLParseResult; + const deparsed = deparseSync(parsed); + expect(deparsed).toMatchSnapshot(); + // Add specific assertions + expect(deparsed).toContain('expected output'); +}); +``` + +Then run tests with snapshot update: + +```bash +pnpm test --updateSnapshot +``` + +### Step 5: Commit All Files + +Always commit the fixture file, generated.json, test file, AND snapshots together: + +```bash +git add __fixtures__/plpgsql/plpgsql_deparser_fixes.sql +git add __fixtures__/plpgsql-generated/generated.json +git add packages/plpgsql-deparser/__tests__/deparser-fixes.test.ts +git add packages/plpgsql-deparser/__tests__/__snapshots__/deparser-fixes.test.ts.snap +git commit -m "test: add fixtures for [description]" +``` + +## Important Notes + +- The `generated.json` file is the source of truth for tests - it must be regenerated when fixtures change +- Fixtures that fail PL/pgSQL parsing are skipped (logged as warnings during generation) +- The test suite has a `KNOWN_FAILING_FIXTURES` set for fixtures with known issues - avoid adding to this unless necessary +- When adding fixtures for new deparser features, ensure the fixture exercises the specific AST pattern you're testing + +## Fixture File Conventions + +- `plpgsql_deparser_fixes.sql` - Fixtures for deparser bug fixes and edge cases +- `plpgsql_*.sql` - PostgreSQL regression test fixtures (from upstream) +- Each fixture should have a comment describing what it tests +- Number fixtures sequentially (Test 1, Test 2, etc.) within each file 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 65553e4f..dc0e93d4 100644 --- a/packages/plpgsql-deparser/__tests__/__snapshots__/deparser-fixes.test.ts.snap +++ b/packages/plpgsql-deparser/__tests__/__snapshots__/deparser-fixes.test.ts.snap @@ -70,6 +70,30 @@ BEGIN END" `; +exports[`plpgsql-deparser bug fixes OUT parameters with SELECT INTO multiple variables should handle SELECT INTO STRICT with multiple OUT parameters 1`] = ` +"BEGIN + SELECT u.name, u.email INTO STRICT name, email FROM users u + WHERE u.id = p_id; + RETURN; +END" +`; + +exports[`plpgsql-deparser bug fixes OUT parameters with SELECT INTO multiple variables should handle SELECT INTO multiple OUT parameters 1`] = ` +"DECLARE + v_token_id uuid; + v_plaintext_token text; +BEGIN + v_plaintext_token := encode(gen_random_bytes(48), 'hex'); + v_token_id := uuid_generate_v5(uuid_ns_url(), v_plaintext_token); + INSERT INTO tokens (id, user_id, access_token_hash) + VALUES (v_token_id, p_user_id, digest(v_plaintext_token, 'sha256')); + SELECT tkn.id, tkn.user_id, v_plaintext_token, tkn.access_token_expires_at, tkn.is_verified, tkn.totp_enabled INTO id, user_id, access_token, access_token_expires_at, is_verified, totp_enabled + FROM tokens AS tkn + WHERE tkn.id = v_token_id; + RETURN; +END" +`; + exports[`plpgsql-deparser bug fixes PERFORM SELECT fix should handle PERFORM with complex expressions 1`] = ` "BEGIN PERFORM set_config('search_path', 'public', true); 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 16ed9d7f..daed06eb 100644 --- a/packages/plpgsql-deparser/__tests__/__snapshots__/hydrate-demo.test.ts.snap +++ b/packages/plpgsql-deparser/__tests__/__snapshots__/hydrate-demo.test.ts.snap @@ -98,7 +98,7 @@ BEGIN t.orders_scanned, t.gross_total, t.avg_total INTO v_orders_scanned, v_gross, v_avg - FROM totals AS t; + FROM totals AS t; IF p_apply_discount THEN v_rebate := round(v_gross * GREATEST(LEAST(v_discount_rate + v_jitter, 0.50), 0), p_round_to); ELSE @@ -109,7 +109,7 @@ BEGIN SELECT oi.sku, CAST(sum(oi.quantity) AS bigint) AS qty INTO v_top_sku, v_top_sku_qty - FROM app_public.order_item AS oi + FROM app_public.order_item AS oi JOIN app_public.app_order AS o ON o.id = oi.order_id WHERE o.org_id = p_org_id 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 b1ca3bc8..8347f1e8 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 @@ -194,7 +194,7 @@ exports[`schema rename mapped should transform schema names and snapshot schema total_count int; BEGIN SELECT count(*) INTO total_count - FROM myapp_v2.users AS u + FROM myapp_v2.users AS u JOIN myapp_v2.orders AS o ON o.user_id = u.id WHERE u.id = p_user_id; @@ -270,7 +270,7 @@ CREATE FUNCTION myapp_v2.calculate_order_total( discount numeric; BEGIN SELECT sum(quantity * price) INTO subtotal - FROM myapp_v2.order_items + FROM myapp_v2.order_items WHERE order_id = p_order_id; tax_amount := myapp_v2.get_tax_rate() * subtotal; diff --git a/packages/plpgsql-deparser/__tests__/deparser-fixes.test.ts b/packages/plpgsql-deparser/__tests__/deparser-fixes.test.ts index 0f4329f4..6d0ad1b8 100644 --- a/packages/plpgsql-deparser/__tests__/deparser-fixes.test.ts +++ b/packages/plpgsql-deparser/__tests__/deparser-fixes.test.ts @@ -307,6 +307,69 @@ $$`; }); }); + describe('OUT parameters with SELECT INTO multiple variables', () => { + it('should handle SELECT INTO multiple OUT parameters', async () => { + const sql = `CREATE FUNCTION test_out_params_select_into( + p_user_id uuid, + OUT id uuid, + OUT user_id uuid, + OUT access_token text, + OUT access_token_expires_at timestamptz, + OUT is_verified boolean, + OUT totp_enabled boolean +) +LANGUAGE plpgsql AS $$ +DECLARE + v_token_id uuid; + v_plaintext_token text; +BEGIN + v_plaintext_token := encode(gen_random_bytes(48), 'hex'); + v_token_id := uuid_generate_v5(uuid_ns_url(), v_plaintext_token); + + INSERT INTO tokens (id, user_id, access_token_hash) + VALUES (v_token_id, p_user_id, digest(v_plaintext_token, 'sha256')); + + SELECT tkn.id, tkn.user_id, v_plaintext_token, tkn.access_token_expires_at, tkn.is_verified, tkn.totp_enabled + INTO id, user_id, access_token, access_token_expires_at, is_verified, totp_enabled + FROM tokens AS tkn + WHERE tkn.id = v_token_id; + + RETURN; +END$$`; + + await testUtils.expectAstMatch('OUT params SELECT INTO', sql); + + const parsed = parsePlPgSQLSync(sql) as unknown as PLpgSQLParseResult; + const deparsed = deparseSync(parsed); + expect(deparsed).toMatchSnapshot(); + // Verify multiple INTO targets are present + expect(deparsed).toMatch(/INTO\s+id\s*,\s*user_id\s*,\s*access_token/i); + }); + + it('should handle SELECT INTO STRICT with multiple OUT parameters', async () => { + const sql = `CREATE FUNCTION test_out_params_strict( + p_id uuid, + OUT name text, + OUT email text +) +LANGUAGE plpgsql AS $$ +BEGIN + SELECT u.name, u.email INTO STRICT name, email + FROM users u + WHERE u.id = p_id; +END$$`; + + await testUtils.expectAstMatch('OUT params STRICT', sql); + + const parsed = parsePlPgSQLSync(sql) as unknown as PLpgSQLParseResult; + const deparsed = deparseSync(parsed); + expect(deparsed).toMatchSnapshot(); + expect(deparsed).toContain('STRICT'); + // Verify multiple INTO targets are present + expect(deparsed).toMatch(/INTO\s+STRICT\s+name\s*,\s*email/i); + }); + }); + describe('combined scenarios', () => { it('should handle PERFORM with record fields', async () => { const sql = `CREATE FUNCTION test_perform_record() RETURNS trigger 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 23713970..fa462a99 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 @@ -66,7 +66,7 @@ begin t.orders_scanned, t.gross_total, t.avg_total into v_orders_scanned, v_gross, v_avg - FROM totals t; + FROM totals t; if p_apply_discount then v_discount := round(v_gross * GREATEST(LEAST(v_discount_rate + v_jitter, 0.50), 0), p_round_to); else @@ -77,7 +77,7 @@ begin SELECT oi.sku, sum(oi.quantity)::bigint AS qty into v_top_sku, v_top_sku_qty - FROM app_public.order_item oi + FROM app_public.order_item oi JOIN app_public.app_order o ON o.id = oi.order_id WHERE o.org_id = p_org_id AND o.user_id = p_user_id @@ -295,7 +295,7 @@ BEGIN t.orders_scanned, t.gross_total, t.avg_total INTO v_orders_scanned, v_gross, v_avg - FROM totals t; + FROM totals t; IF p_apply_discount THEN v_discount := round(v_gross * GREATEST(LEAST(v_discount_rate + v_jitter, 0.50), 0), p_round_to); ELSE @@ -306,7 +306,7 @@ BEGIN SELECT oi.sku, sum(oi.quantity)::bigint AS qty INTO v_top_sku, v_top_sku_qty - FROM app_public.order_item oi + FROM app_public.order_item oi JOIN app_public.app_order o ON o.id = oi.order_id WHERE o.org_id = p_org_id AND o.user_id = p_user_id diff --git a/packages/plpgsql-deparser/src/plpgsql-deparser.ts b/packages/plpgsql-deparser/src/plpgsql-deparser.ts index 177dea6e..13afe071 100644 --- a/packages/plpgsql-deparser/src/plpgsql-deparser.ts +++ b/packages/plpgsql-deparser/src/plpgsql-deparser.ts @@ -1485,8 +1485,23 @@ export class PLpgSQLDeparser { // large gaps like "SELECT x INTO y FROM z" const before = sql.slice(0, insertPos); let after = sql.slice(insertPos); - // Collapse leading whitespace (but preserve a single space before the next keyword) - after = after.replace(/^[ \t]+/, ' '); + // Normalize whitespace after INTO insertion + // The parser strips "INTO " but leaves whitespace behind, which can cause + // weird formatting like "SELECT x INTO y FROM z" + // We collapse all whitespace to either a single space or a newline with standard indent + const leadingWsMatch = after.match(/^(\s+)/); + if (leadingWsMatch) { + const ws = leadingWsMatch[1]; + const hasNewline = /\n/.test(ws); + if (hasNewline) { + // If original had newlines, use a newline with standard 4-space indent + // This normalizes any weird indentation left by the parser + after = after.replace(/^\s+/, '\n '); + } else { + // If original was just spaces/tabs, collapse to single space + after = after.replace(/^[ \t]+/, ' '); + } + } sql = before + intoClause + after; } else { // -1 means INTO already exists at depth 0, don't add another one