Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions __fixtures__/plpgsql-generated/generated.json
Original file line number Diff line number Diff line change
Expand Up @@ -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$$",
Expand Down Expand Up @@ -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$$"
}
}
44 changes: 44 additions & 0 deletions __fixtures__/plpgsql/plpgsql_deparser_fixes.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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$$;
104 changes: 104 additions & 0 deletions packages/plpgsql-deparser/AGENTS.md
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
63 changes: 63 additions & 0 deletions packages/plpgsql-deparser/__tests__/deparser-fixes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
19 changes: 17 additions & 2 deletions packages/plpgsql-deparser/src/plpgsql-deparser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 <target>" 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
Expand Down