Skip to content

Commit 0192055

Browse files
pyramationpyramation
authored
Merge pull request #352 from constructive-io/fix/deparser-issues-292-346-348-351
fix: deparser partition/FK fixes, enforced constraint builder default, walkSql body-error abort (#292 #346 #348 #349 #350 #351)
2 parents 0a734eb + c887751 commit 0192055

15 files changed

Lines changed: 339 additions & 34 deletions

File tree

‎__fixtures__/generated/generated.json‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21374,6 +21374,24 @@
2137421374
"misc/issues-20.sql": "CREATE TABLE test_exclude_where (\n id uuid PRIMARY KEY,\n database_id uuid NOT NULL,\n status text NOT NULL DEFAULT 'pending',\n EXCLUDE USING btree (database_id WITH =)\n WHERE (status = 'pending')\n)",
2137521375
"misc/issues-21.sql": "CREATE TABLE test_named_exclude (\n id uuid PRIMARY KEY,\n database_id uuid NOT NULL,\n status text NOT NULL DEFAULT 'pending',\n CONSTRAINT one_pending_per_database\n EXCLUDE USING btree (database_id WITH =)\n WHERE (status = 'pending')\n)",
2137621376
"misc/issues-22.sql": "ALTER TABLE test_named_exclude ADD CONSTRAINT no_overlap EXCLUDE USING gist (room WITH =, during WITH &&)",
21377+
"misc/issue-349-fk-set-cols-1.sql": "ALTER TABLE ONLY child ADD CONSTRAINT child_fk FOREIGN KEY (a, b) REFERENCES parent(a, b) ON DELETE SET NULL (b)",
21378+
"misc/issue-349-fk-set-cols-2.sql": "ALTER TABLE ONLY child ADD CONSTRAINT child_fk FOREIGN KEY (a, b) REFERENCES parent(a, b) ON DELETE SET DEFAULT (b)",
21379+
"misc/issue-349-fk-set-cols-3.sql": "ALTER TABLE ONLY child ADD CONSTRAINT child_fk FOREIGN KEY (a, b) REFERENCES parent(a, b) ON UPDATE CASCADE ON DELETE SET NULL (a, b)",
21380+
"misc/issue-349-fk-set-cols-4.sql": "CREATE TABLE child (a int, b int, FOREIGN KEY (a, b) REFERENCES parent (a, b) ON DELETE SET NULL (b))",
21381+
"misc/issue-348-350-partition-cmd-1.sql": "ALTER TABLE ONLY public.measurement ATTACH PARTITION public.measurement_y2024 FOR VALUES FROM ('2024-01-01') TO ('2025-01-01')",
21382+
"misc/issue-348-350-partition-cmd-2.sql": "ALTER TABLE ONLY public.measurement DETACH PARTITION public.measurement_y2024",
21383+
"misc/issue-348-350-partition-cmd-3.sql": "ALTER TABLE public.measurement DETACH PARTITION public.measurement_y2024 CONCURRENTLY",
21384+
"misc/issue-348-350-partition-cmd-4.sql": "ALTER TABLE ONLY measurement ATTACH PARTITION measurement_y2024 FOR VALUES FROM ('2024-01-01') TO ('2025-01-01')",
21385+
"misc/issue-348-350-partition-cmd-5.sql": "ALTER TABLE ONLY o ATTACH PARTITION o_p FOR VALUES WITH (MODULUS 4, REMAINDER 0)",
21386+
"misc/issue-348-350-partition-cmd-6.sql": "ALTER TABLE ONLY o ATTACH PARTITION o_p FOR VALUES WITH (MODULUS 4, REMAINDER 1)",
21387+
"misc/issue-348-350-partition-cmd-7.sql": "ALTER TABLE ONLY s.o ATTACH PARTITION s.o_p DEFAULT",
21388+
"misc/issue-348-350-partition-cmd-8.sql": "ALTER TABLE ONLY o ATTACH PARTITION o_p FOR VALUES IN (1, 2)",
21389+
"misc/issue-348-350-partition-cmd-9.sql": "CREATE TABLE o_p0 PARTITION OF o FOR VALUES WITH (MODULUS 4, REMAINDER 0)",
21390+
"misc/issue-292-merge-when-1.sql": "MERGE INTO t AS target USING (SELECT 1 AS id) AS source ON target.id = source.id WHEN MATCHED THEN UPDATE SET name = 'x' WHEN NOT MATCHED THEN INSERT (id, name) VALUES (source.id, 'x')",
21391+
"misc/issue-292-merge-when-2.sql": "MERGE INTO t AS target USING (SELECT 1 AS id) AS source ON target.id = source.id WHEN MATCHED AND cond THEN DELETE",
21392+
"misc/issue-292-merge-when-3.sql": "MERGE INTO t AS target USING (SELECT 1 AS id) AS source ON target.id = source.id WHEN NOT MATCHED BY SOURCE THEN UPDATE SET name = 'x'",
21393+
"misc/issue-292-merge-when-4.sql": "MERGE INTO t AS target USING (SELECT 1 AS id) AS source ON target.id = source.id WHEN MATCHED THEN DO NOTHING",
21394+
"misc/issue-292-merge-when-5.sql": "MERGE INTO t AS target USING (SELECT 1 AS id) AS source ON target.id = source.id WHEN NOT MATCHED THEN INSERT DEFAULT VALUES",
2137721395
"misc/inflection-1.sql": "CREATE SCHEMA inflection",
2137821396
"misc/inflection-2.sql": "GRANT USAGE ON SCHEMA inflection TO PUBLIC",
2137921397
"misc/inflection-3.sql": "ALTER DEFAULT PRIVILEGES IN SCHEMA inflection \n GRANT EXECUTE ON FUNCTIONS TO PUBLIC",

‎__fixtures__/kitchen-sink/misc/issue-292-merge-when.sql‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
-- Ref: constructive-io/pgsql-parser#292
2+
MERGE INTO t AS target USING (SELECT 1 AS id) AS source ON target.id = source.id WHEN MATCHED THEN UPDATE SET name = 'x' WHEN NOT MATCHED THEN INSERT (id, name) VALUES (source.id, 'x');
3+
MERGE INTO t AS target USING (SELECT 1 AS id) AS source ON target.id = source.id WHEN MATCHED AND cond THEN DELETE;
4+
MERGE INTO t AS target USING (SELECT 1 AS id) AS source ON target.id = source.id WHEN NOT MATCHED BY SOURCE THEN UPDATE SET name = 'x';
5+
MERGE INTO t AS target USING (SELECT 1 AS id) AS source ON target.id = source.id WHEN MATCHED THEN DO NOTHING;
6+
MERGE INTO t AS target USING (SELECT 1 AS id) AS source ON target.id = source.id WHEN NOT MATCHED THEN INSERT DEFAULT VALUES;

‎__fixtures__/kitchen-sink/misc/issue-348-350-partition-cmd.sql‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
-- Ref: constructive-io/pgsql-parser#348
2+
-- Ref: constructive-io/pgsql-parser#350
3+
ALTER TABLE ONLY public.measurement ATTACH PARTITION public.measurement_y2024 FOR VALUES FROM ('2024-01-01') TO ('2025-01-01');
4+
ALTER TABLE ONLY public.measurement DETACH PARTITION public.measurement_y2024;
5+
ALTER TABLE public.measurement DETACH PARTITION public.measurement_y2024 CONCURRENTLY;
6+
ALTER TABLE ONLY measurement ATTACH PARTITION measurement_y2024 FOR VALUES FROM ('2024-01-01') TO ('2025-01-01');
7+
ALTER TABLE ONLY o ATTACH PARTITION o_p FOR VALUES WITH (MODULUS 4, REMAINDER 0);
8+
ALTER TABLE ONLY o ATTACH PARTITION o_p FOR VALUES WITH (MODULUS 4, REMAINDER 1);
9+
ALTER TABLE ONLY s.o ATTACH PARTITION s.o_p DEFAULT;
10+
ALTER TABLE ONLY o ATTACH PARTITION o_p FOR VALUES IN (1, 2);
11+
CREATE TABLE o_p0 PARTITION OF o FOR VALUES WITH (MODULUS 4, REMAINDER 0);

‎__fixtures__/kitchen-sink/misc/issue-349-fk-set-cols.sql‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
-- Ref: constructive-io/pgsql-parser#349
2+
ALTER TABLE ONLY child ADD CONSTRAINT child_fk FOREIGN KEY (a, b) REFERENCES parent(a, b) ON DELETE SET NULL (b);
3+
ALTER TABLE ONLY child ADD CONSTRAINT child_fk FOREIGN KEY (a, b) REFERENCES parent(a, b) ON DELETE SET DEFAULT (b);
4+
ALTER TABLE ONLY child ADD CONSTRAINT child_fk FOREIGN KEY (a, b) REFERENCES parent(a, b) ON UPDATE CASCADE ON DELETE SET NULL (a, b);
5+
CREATE TABLE child (a int, b int, FOREIGN KEY (a, b) REFERENCES parent (a, b) ON DELETE SET NULL (b));

‎packages/deparser/__tests__/kitchen-sink/misc-issue-292-merge-when.test.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
2+
import { FixtureTestUtils } from '../../test-utils';
3+
const fixtures = new FixtureTestUtils();
4+
5+
it('misc-issue-292-merge-when', async () => {
6+
await fixtures.runFixtureTests([
7+
"misc/issue-292-merge-when-1.sql",
8+
"misc/issue-292-merge-when-2.sql",
9+
"misc/issue-292-merge-when-3.sql",
10+
"misc/issue-292-merge-when-4.sql",
11+
"misc/issue-292-merge-when-5.sql"
12+
]);
13+
});

‎packages/deparser/__tests__/kitchen-sink/misc-issue-348-350-partition-cmd.test.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
2+
import { FixtureTestUtils } from '../../test-utils';
3+
const fixtures = new FixtureTestUtils();
4+
5+
it('misc-issue-348-350-partition-cmd', async () => {
6+
await fixtures.runFixtureTests([
7+
"misc/issue-348-350-partition-cmd-1.sql",
8+
"misc/issue-348-350-partition-cmd-2.sql",
9+
"misc/issue-348-350-partition-cmd-3.sql",
10+
"misc/issue-348-350-partition-cmd-4.sql",
11+
"misc/issue-348-350-partition-cmd-5.sql",
12+
"misc/issue-348-350-partition-cmd-6.sql",
13+
"misc/issue-348-350-partition-cmd-7.sql",
14+
"misc/issue-348-350-partition-cmd-8.sql",
15+
"misc/issue-348-350-partition-cmd-9.sql"
16+
]);
17+
});

‎packages/deparser/__tests__/kitchen-sink/misc-issue-349-fk-set-cols.test.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
2+
import { FixtureTestUtils } from '../../test-utils';
3+
const fixtures = new FixtureTestUtils();
4+
5+
it('misc-issue-349-fk-set-cols', async () => {
6+
await fixtures.runFixtureTests([
7+
"misc/issue-349-fk-set-cols-1.sql",
8+
"misc/issue-349-fk-set-cols-2.sql",
9+
"misc/issue-349-fk-set-cols-3.sql",
10+
"misc/issue-349-fk-set-cols-4.sql"
11+
]);
12+
});

‎packages/deparser/src/deparser.ts‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3079,6 +3079,15 @@ export class Deparser implements DeparserVisitor {
30793079
deleteClause += 'SET DEFAULT';
30803080
break;
30813081
}
3082+
if ((node.fk_del_action === 'n' || node.fk_del_action === 'd') && node.fk_del_set_cols && node.fk_del_set_cols.length > 0) {
3083+
const setColumns = ListUtils.unwrapList(node.fk_del_set_cols)
3084+
.map(column => {
3085+
const stringNode = (column as { String: { sval?: string; str?: string } }).String;
3086+
return QuoteUtils.quoteIdentifier(stringNode.sval || stringNode.str || '');
3087+
})
3088+
.join(', ');
3089+
deleteClause += ` (${setColumns})`;
3090+
}
30823091
if (context.isPretty()) {
30833092
output.push('\n' + context.indent(deleteClause));
30843093
} else {
@@ -3917,12 +3926,12 @@ export class Deparser implements DeparserVisitor {
39173926
PartitionCmd(node: t.PartitionCmd, context: DeparserContext): string {
39183927
const output: string[] = [];
39193928

3920-
if (node.concurrent) {
3921-
output.push('CONCURRENTLY');
3929+
if (node.name) {
3930+
output.push(this.RangeVar(node.name, context));
39223931
}
39233932

3924-
if (node.name) {
3925-
output.push(this.visit(node.name as any, context));
3933+
if (node.concurrent) {
3934+
output.push('CONCURRENTLY');
39263935
}
39273936

39283937
if (node.bound) {
@@ -3947,9 +3956,9 @@ export class Deparser implements DeparserVisitor {
39473956
.join(', ');
39483957
output.push(`(${upperValues})`);
39493958
}
3950-
} else if (node.bound.strategy === 'h' && node.bound.modulus !== undefined && node.bound.remainder !== undefined) {
3959+
} else if (node.bound.strategy === 'h' && node.bound.modulus !== undefined) {
39513960
output.push('FOR VALUES WITH');
3952-
output.push(`(modulus ${node.bound.modulus}, remainder ${node.bound.remainder})`);
3961+
output.push(`(MODULUS ${node.bound.modulus}, REMAINDER ${node.bound.remainder ?? 0})`);
39533962
} else if (node.bound.is_default) {
39543963
output.push('DEFAULT');
39553964
}
@@ -11644,4 +11653,3 @@ export class Deparser implements DeparserVisitor {
1164411653
return stringLiteralRegex.test(content);
1164511654
}
1164611655
}
11647-

‎packages/plpgsql-parser/__tests__/walk-sql-body-errors.test.ts‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
import { loadModule, parseSync, walkSql } from '../src';
2+
3+
beforeAll(async () => {
4+
await loadModule();
5+
});
6+
7+
const BROKEN_FUNCTION_SQL = `
8+
CREATE FUNCTION broken_connectors() RETURNS void
9+
LANGUAGE plpgsql AS $$
10+
BEGIN
11+
UPDATE connectors SET instance_id = ;
12+
END;
13+
$$;
14+
`;
15+
16+
describe('walkSql PL/pgSQL body errors', () => {
17+
it('reports a broken PL/pgSQL body as an abort and records its statement index', () => {
18+
const parsed = parseSync(BROKEN_FUNCTION_SQL);
19+
expect(parsed.errors).toHaveLength(1);
20+
expect(parsed.errors[0].stmtIndex).toBe(0);
21+
22+
const result = walkSql(BROKEN_FUNCTION_SQL, {});
23+
expect(result.aborted).toBe(true);
24+
expect(result.reason).toBeTruthy();
25+
expect(result.reasons).toEqual([parsed.errors[0].message]);
26+
});
27+
28+
it('walks valid PL/pgSQL bodies', () => {
29+
const visited: string[] = [];
30+
const result = walkSql(
31+
`
32+
CREATE FUNCTION valid_connectors() RETURNS void
33+
LANGUAGE plpgsql AS $$
34+
BEGIN
35+
UPDATE connectors SET instance_id = 1;
36+
END;
37+
$$;
38+
`,
39+
(path) => {
40+
if (path.tag.startsWith('PLpgSQL_')) {
41+
visited.push(path.tag);
42+
}
43+
}
44+
);
45+
46+
expect(result.aborted).toBe(false);
47+
expect(visited).toContain('PLpgSQL_function');
48+
expect(visited).toContain('PLpgSQL_stmt_execsql');
49+
});
50+
51+
it('does not parse broken bodies when walkFunctionBodies is false', () => {
52+
const result = walkSql(BROKEN_FUNCTION_SQL, {}, { walkFunctionBodies: false });
53+
expect(result.aborted).toBe(false);
54+
});
55+
56+
it('does not inspect non-PL/pgSQL function bodies', () => {
57+
const result = walkSql(
58+
`
59+
CREATE FUNCTION sql_function() RETURNS integer
60+
LANGUAGE sql AS $$ THIS IS NOT VALID SQL BODY TEXT $$;
61+
`,
62+
{},
63+
);
64+
65+
expect(result.aborted).toBe(false);
66+
});
67+
});

‎packages/plpgsql-parser/src/parse.ts‎

Lines changed: 35 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import type {
1212
ParsedFunction,
1313
ParsedItem,
1414
ParsedScript,
15+
ParsedScriptError,
1516
ParsedStatement,
1617
ParseOptions
1718
} from './types';
@@ -68,15 +69,19 @@ function getStatementSql(sqlBuffer: Buffer, rawStmt: any): string {
6869
return sqlBuffer.slice(start, end).toString('utf8');
6970
}
7071

71-
function extractFunctionInfo(stmt: any, stmtIndex: number, stmtSql: string): ParsedFunction | null {
72+
function extractFunctionInfo(
73+
stmt: any,
74+
stmtIndex: number,
75+
stmtSql: string
76+
): { fn: ParsedFunction | null; error?: string } {
7277
const createFunctionStmt = stmt?.CreateFunctionStmt;
73-
if (!createFunctionStmt) return null;
78+
if (!createFunctionStmt) return { fn: null };
7479

7580
const language = getLanguageFromOptions(createFunctionStmt.options);
76-
if (language !== 'plpgsql') return null;
81+
if (language !== 'plpgsql') return { fn: null };
7782

7883
const body = getBodyFromOptions(createFunctionStmt.options);
79-
if (!body) return null;
84+
if (!body) return { fn: null };
8085

8186
try {
8287
// Parse only this statement's SQL. Parsing the full script would return
@@ -86,20 +91,25 @@ function extractFunctionInfo(stmt: any, stmtIndex: number, stmtSql: string): Par
8691
const { ast: hydrated, stats, errors } = hydratePlpgsqlAst(plpgsqlRaw);
8792

8893
return {
89-
kind: 'plpgsql-function',
90-
stmt: createFunctionStmt,
91-
stmtIndex,
92-
language: language || 'plpgsql',
93-
body,
94-
plpgsql: {
95-
raw: plpgsqlRaw,
96-
hydrated,
97-
stats,
98-
errors
94+
fn: {
95+
kind: 'plpgsql-function',
96+
stmt: createFunctionStmt,
97+
stmtIndex,
98+
language: language || 'plpgsql',
99+
body,
100+
plpgsql: {
101+
raw: plpgsqlRaw,
102+
hydrated,
103+
stats,
104+
errors
105+
}
99106
}
100107
};
101108
} catch (err) {
102-
return null;
109+
return {
110+
fn: null,
111+
error: err instanceof Error ? err.message : String(err)
112+
};
103113
}
104114
}
105115

@@ -109,6 +119,7 @@ export function parse(sql: string, options: ParseOptions = {}): ParsedScript {
109119
const sqlResult: ParseResult = parseSqlSync(sql);
110120
const items: ParsedItem[] = [];
111121
const functions: ParsedFunction[] = [];
122+
const errors: ParsedScriptError[] = [];
112123
const sqlBuffer = Buffer.from(sql, 'utf8');
113124

114125
if (sqlResult.stmts) {
@@ -117,12 +128,15 @@ export function parse(sql: string, options: ParseOptions = {}): ParsedScript {
117128
const stmt = rawStmt?.stmt;
118129

119130
if (stmt && isPlpgsqlFunction(stmt) && hydrate) {
120-
const fnInfo = extractFunctionInfo(stmt, i, getStatementSql(sqlBuffer, rawStmt));
121-
if (fnInfo) {
122-
items.push(fnInfo);
123-
functions.push(fnInfo);
131+
const result = extractFunctionInfo(stmt, i, getStatementSql(sqlBuffer, rawStmt));
132+
if (result.fn) {
133+
items.push(result.fn);
134+
functions.push(result.fn);
124135
continue;
125136
}
137+
if (result.error) {
138+
errors.push({ stmtIndex: i, message: result.error });
139+
}
126140
}
127141

128142
const stmtItem: ParsedStatement = {
@@ -137,7 +151,8 @@ export function parse(sql: string, options: ParseOptions = {}): ParsedScript {
137151
return {
138152
sql: sqlResult,
139153
items,
140-
functions
154+
functions,
155+
errors
141156
};
142157
}
143158

0 commit comments

Comments
 (0)