Skip to content

Commit 9656922

Browse files
committed
fix: resolve 7 plpgsql_schema_rename fixture failures
Two root causes fixed: 1. cleanPlpgsqlTree query normalization bug: The transform function applied normalizeQueryWhitespace to ALL keys named 'query', but PLpgSQL_stmt_return_query.query is an object (wrapping PLpgSQL_expr), not a string. normalizeQueryWhitespace returned non-strings unchanged WITHOUT recursing, so the inner PLpgSQL_expr.query string was never normalized. Fixed by making the query handler recurse into objects. 2. GET DIAGNOSTICS kind mapping: The parser returns diag_item.kind as a string (e.g. 'ROW_COUNT') but getDiagItemKindName() only handled numeric enum values. Added string-based mapping so both forms work. Also removed KNOWN_FAILING_FIXTURES allowlist — all 225 fixtures now pass round-trip testing.
1 parent df3f813 commit 9656922

3 files changed

Lines changed: 48 additions & 19 deletions

File tree

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,7 @@ describe('PLpgSQLDeparser', () => {
4040
// - Labeled blocks with EXIT statements
4141
// - Nested DECLARE inside FOR loops (lineno-based scope tracking)
4242
const KNOWN_FAILING_FIXTURES = new Set<string>([
43-
// Pre-existing schema rename deparser issues (whitespace in query expressions)
44-
'plpgsql_schema_rename-4.sql',
45-
'plpgsql_schema_rename-5.sql',
46-
'plpgsql_schema_rename-7.sql',
47-
'plpgsql_schema_rename-9.sql',
48-
'plpgsql_schema_rename-10.sql',
49-
'plpgsql_schema_rename-11.sql',
50-
'plpgsql_schema_rename-12.sql',
43+
// No known failures - all fixtures pass!
5144
]);
5245

5346
it('should round-trip ALL generated fixtures (excluding known failures)', async () => {

packages/plpgsql-deparser/src/plpgsql-deparser.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2034,9 +2034,31 @@ export class PLpgSQLDeparser {
20342034
/**
20352035
* Get the diagnostic item kind name
20362036
*/
2037-
private getDiagItemKindName(kind: number | undefined): string {
2037+
private getDiagItemKindName(kind: number | string | undefined): string {
20382038
if (kind === undefined) return '';
20392039

2040+
// The parser may return kind as a string name (e.g. "ROW_COUNT") or as a
2041+
// numeric enum value. Handle both forms so the deparser works regardless.
2042+
if (typeof kind === 'string') {
2043+
// Map string names to their SQL keyword equivalents
2044+
const stringMap: Record<string, string> = {
2045+
'ROW_COUNT': 'ROW_COUNT',
2046+
'PG_CONTEXT': 'PG_CONTEXT',
2047+
'PG_EXCEPTION_CONTEXT': 'PG_EXCEPTION_CONTEXT',
2048+
'PG_EXCEPTION_DETAIL': 'PG_EXCEPTION_DETAIL',
2049+
'PG_EXCEPTION_HINT': 'PG_EXCEPTION_HINT',
2050+
'RETURNED_SQLSTATE': 'RETURNED_SQLSTATE',
2051+
'COLUMN_NAME': 'COLUMN_NAME',
2052+
'CONSTRAINT_NAME': 'CONSTRAINT_NAME',
2053+
'PG_DATATYPE_NAME': 'PG_DATATYPE_NAME',
2054+
'MESSAGE_TEXT': 'MESSAGE_TEXT',
2055+
'TABLE_NAME': 'TABLE_NAME',
2056+
'SCHEMA_NAME': 'SCHEMA_NAME',
2057+
};
2058+
const mapped = stringMap[kind];
2059+
return mapped ? this.keyword(mapped) : '';
2060+
}
2061+
20402062
switch (kind) {
20412063
case DiagItemKind.PLPGSQL_GETDIAG_ROW_COUNT:
20422064
return this.keyword('ROW_COUNT');

packages/plpgsql-deparser/test-utils/index.ts

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -207,17 +207,31 @@ export const transform = (obj: any, props: any): any => {
207207
throw new Error("Unable to copy obj! Its type isn't supported.");
208208
};
209209

210+
// Props object for cleanPlpgsqlTree, defined as a variable so the query handler
211+
// can reference it for recursive descent when 'query' is an object (e.g.
212+
// PLpgSQL_stmt_return_query.query wraps a PLpgSQL_expr, while PLpgSQL_expr.query
213+
// is the actual SQL string).
214+
const cleanProps: Record<string, any> = {
215+
lineno: noop,
216+
location: noop,
217+
stmt_len: noop,
218+
stmt_location: noop,
219+
// varno values are assigned based on position in datums array and can change
220+
// when implicit variables (like sqlstate/sqlerrm) are filtered out during deparse
221+
varno: noop,
222+
query: (val: any): any => {
223+
if (typeof val === 'string') {
224+
return normalizeQueryWhitespace(val);
225+
}
226+
// Not a string (e.g. PLpgSQL_stmt_return_query.query is an object wrapping
227+
// PLpgSQL_expr) — continue recursing so the inner PLpgSQL_expr.query string
228+
// gets normalized.
229+
return transform(val, cleanProps);
230+
},
231+
};
232+
210233
export const cleanPlpgsqlTree = (tree: any) => {
211-
return transform(tree, {
212-
lineno: noop,
213-
location: noop,
214-
stmt_len: noop,
215-
stmt_location: noop,
216-
// varno values are assigned based on position in datums array and can change
217-
// when implicit variables (like sqlstate/sqlerrm) are filtered out during deparse
218-
varno: noop,
219-
query: normalizeQueryWhitespace,
220-
});
234+
return transform(tree, cleanProps);
221235
};
222236

223237
type ParseErrorType =

0 commit comments

Comments
 (0)