Skip to content

Commit dca74e3

Browse files
Refactor TypeCast deparser to use AST-driven logic
- Add 4 helper methods for AST inspection: * isQualifiedName() - Check if names array matches expected path * isBuiltinPgCatalogType() - Check if type is built-in pg_catalog type * normalizeTypeName() - Extract normalized type name from TypeName node * argumentNeedsCastSyntax() - Determine if argument needs CAST() syntax based on AST node type - Refactor TypeCast method to eliminate string-based heuristics: * Remove arg.includes('(') and arg.startsWith('-') checks * Use AST node types (getNodeType) to determine cast syntax * Check negative numbers by inspecting ival/fval values directly * Preserve round-trip fidelity for pg_catalog.bpchar and negative numbers - All 657 tests passing - No formatting changes, only functional code additions Co-Authored-By: Dan Lynch <[email protected]>
1 parent 125bc36 commit dca74e3

File tree

1 file changed

+169
-23
lines changed

1 file changed

+169
-23
lines changed

packages/deparser/src/deparser.ts

Lines changed: 169 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2346,40 +2346,186 @@ export class Deparser implements DeparserVisitor {
23462346
return `COALESCE(${argStrs.join(', ')})`;
23472347
}
23482348

2349+
/**
2350+
* Helper: Check if a TypeName node's names array matches a specific qualified path.
2351+
* Example: isQualifiedName(node.names, ['pg_catalog', 'bpchar']) checks for pg_catalog.bpchar
2352+
*/
2353+
private isQualifiedName(names: any[] | undefined, expectedPath: string[]): boolean {
2354+
if (!names || names.length !== expectedPath.length) {
2355+
return false;
2356+
}
2357+
2358+
for (let i = 0; i < expectedPath.length; i++) {
2359+
const nameValue = (names[i] as any)?.String?.sval;
2360+
if (nameValue !== expectedPath[i]) {
2361+
return false;
2362+
}
2363+
}
2364+
2365+
return true;
2366+
}
2367+
2368+
/**
2369+
* Helper: Check if a TypeName node represents a built-in pg_catalog type.
2370+
* Uses AST structure, not rendered strings.
2371+
*/
2372+
private isBuiltinPgCatalogType(typeNameNode: t.TypeName): boolean {
2373+
if (!typeNameNode.names) {
2374+
return false;
2375+
}
2376+
2377+
const names = typeNameNode.names.map((name: any) => {
2378+
if (name.String) {
2379+
return name.String.sval || name.String.str;
2380+
}
2381+
return '';
2382+
}).filter(Boolean);
2383+
2384+
if (names.length === 0) {
2385+
return false;
2386+
}
2387+
2388+
// Check if it's a qualified pg_catalog type
2389+
if (names.length === 2 && names[0] === 'pg_catalog') {
2390+
return pgCatalogTypes.includes(names[1]);
2391+
}
2392+
2393+
// Check if it's an unqualified built-in type
2394+
if (names.length === 1) {
2395+
const typeName = names[0];
2396+
if (pgCatalogTypes.includes(typeName)) {
2397+
return true;
2398+
}
2399+
2400+
// Check aliases
2401+
for (const [realType, aliases] of pgCatalogTypeAliases) {
2402+
if (aliases.includes(typeName)) {
2403+
return true;
2404+
}
2405+
}
2406+
}
2407+
2408+
return false;
2409+
}
2410+
2411+
/**
2412+
* Helper: Get normalized type name from TypeName node (strips pg_catalog prefix).
2413+
* Uses AST structure, not rendered strings.
2414+
*/
2415+
private normalizeTypeName(typeNameNode: t.TypeName): string {
2416+
if (!typeNameNode.names) {
2417+
return '';
2418+
}
2419+
2420+
const names = typeNameNode.names.map((name: any) => {
2421+
if (name.String) {
2422+
return name.String.sval || name.String.str;
2423+
}
2424+
return '';
2425+
}).filter(Boolean);
2426+
2427+
if (names.length === 0) {
2428+
return '';
2429+
}
2430+
2431+
// If qualified with pg_catalog, return just the type name
2432+
if (names.length === 2 && names[0] === 'pg_catalog') {
2433+
return names[1];
2434+
}
2435+
2436+
// Otherwise return the first (and typically only) name
2437+
return names[0];
2438+
}
2439+
2440+
/**
2441+
* Helper: Determine if an argument node needs CAST() syntax based on AST structure.
2442+
* Returns true if the argument has complex structure that requires CAST() syntax.
2443+
* Uses AST predicates, not string inspection.
2444+
*/
2445+
private argumentNeedsCastSyntax(argNode: any): boolean {
2446+
const argType = this.getNodeType(argNode);
2447+
2448+
// FuncCall nodes can use :: syntax (TypeCast will add parentheses)
2449+
if (argType === 'FuncCall') {
2450+
return false;
2451+
}
2452+
2453+
// Simple constants and column references can use :: syntax
2454+
if (argType === 'A_Const' || argType === 'ColumnRef') {
2455+
// Check for A_Const with special cases that might need CAST syntax
2456+
if (argType === 'A_Const') {
2457+
// Unwrap the node to get the actual A_Const data
2458+
const nodeAny = (argNode.A_Const || argNode) as any;
2459+
2460+
// Check if this is a negative number (needs parentheses with :: syntax)
2461+
// Negative numbers can be represented as negative ival or as fval starting with '-'
2462+
if (nodeAny.ival !== undefined) {
2463+
const ivalValue = typeof nodeAny.ival === 'object' ? nodeAny.ival.ival : nodeAny.ival;
2464+
if (typeof ivalValue === 'number' && ivalValue < 0) {
2465+
return true; // Negative integer needs CAST() to avoid precedence issues
2466+
}
2467+
}
2468+
2469+
if (nodeAny.fval !== undefined) {
2470+
const fvalValue = typeof nodeAny.fval === 'object' ? nodeAny.fval.fval : nodeAny.fval;
2471+
const fvalStr = String(fvalValue);
2472+
if (fvalStr.startsWith('-')) {
2473+
return true; // Negative float needs CAST() to avoid precedence issues
2474+
}
2475+
}
2476+
2477+
// Check for Integer/Float in val field
2478+
if (nodeAny.val) {
2479+
if (nodeAny.val.Integer?.ival !== undefined && nodeAny.val.Integer.ival < 0) {
2480+
return true;
2481+
}
2482+
if (nodeAny.val.Float?.fval !== undefined) {
2483+
const fvalStr = String(nodeAny.val.Float.fval);
2484+
if (fvalStr.startsWith('-')) {
2485+
return true;
2486+
}
2487+
}
2488+
}
2489+
2490+
// All other A_Const types (positive numbers, strings, booleans, null, bit strings) are simple
2491+
return false;
2492+
}
2493+
2494+
// ColumnRef can always use :: syntax
2495+
return false;
2496+
}
2497+
2498+
// All other node types (A_Expr, SubLink, TypeCast, A_Indirection, RowExpr, etc.)
2499+
// are considered complex and should use CAST() syntax
2500+
return true;
2501+
}
2502+
23492503
TypeCast(node: t.TypeCast, context: DeparserContext): string {
23502504
const arg = this.visit(node.arg, context);
23512505
const typeName = this.TypeName(node.typeName, context);
23522506

2353-
// Check if this is a bpchar typecast that should preserve original syntax for AST consistency
2354-
if (typeName === 'bpchar' || typeName === 'pg_catalog.bpchar') {
2355-
const names = node.typeName?.names;
2356-
const isQualifiedBpchar = names && names.length === 2 &&
2357-
(names[0] as any)?.String?.sval === 'pg_catalog' &&
2358-
(names[1] as any)?.String?.sval === 'bpchar';
2359-
2360-
if (isQualifiedBpchar) {
2361-
return `CAST(${arg} AS ${typeName})`;
2362-
}
2507+
// Special handling for bpchar: preserve pg_catalog.bpchar with CAST() syntax for round-trip fidelity
2508+
if (this.isQualifiedName(node.typeName?.names, ['pg_catalog', 'bpchar'])) {
2509+
return `CAST(${arg} AS ${typeName})`;
23632510
}
23642511

2512+
// Check if this is a built-in pg_catalog type based on the rendered type name
23652513
if (this.isPgCatalogType(typeName)) {
23662514
const argType = this.getNodeType(node.arg);
23672515

2368-
const isSimpleArgument = argType === 'A_Const' || argType === 'ColumnRef';
2369-
const isFunctionCall = argType === 'FuncCall';
2370-
2371-
if (isSimpleArgument || isFunctionCall) {
2372-
// For simple arguments, avoid :: syntax if they have complex structure
2373-
const shouldUseCastSyntax = isSimpleArgument && (arg.includes('(') || arg.startsWith('-'));
2516+
// Determine if we can use :: syntax based on AST structure
2517+
const needsCastSyntax = this.argumentNeedsCastSyntax(node.arg);
2518+
2519+
if (!needsCastSyntax) {
2520+
// Strip pg_catalog prefix from the rendered type name for :: syntax
2521+
const cleanTypeName = typeName.replace(/^pg_catalog\./, '');
23742522

2375-
if (!shouldUseCastSyntax) {
2376-
const cleanTypeName = typeName.replace('pg_catalog.', '');
2377-
// Wrap FuncCall arguments in parentheses to prevent operator precedence issues
2378-
if (isFunctionCall) {
2379-
return `${context.parens(arg)}::${cleanTypeName}`;
2380-
}
2381-
return `${arg}::${cleanTypeName}`;
2523+
// For FuncCall, wrap in parentheses to prevent operator precedence issues
2524+
if (argType === 'FuncCall') {
2525+
return `${context.parens(arg)}::${cleanTypeName}`;
23822526
}
2527+
2528+
return `${arg}::${cleanTypeName}`;
23832529
}
23842530
}
23852531

0 commit comments

Comments
 (0)