Skip to content

Commit 4f0c2da

Browse files
fix: revert A_Const changes and add context-specific Integer transformation
- Added TypeName context check to Integer method for arrayBounds handling - Reverted A_Const method to simple transform calls - Created TODO.md for type safety improvements - Added debug_a_const.js for systematic debugging - Current status: investigating context-dependent transformation patterns Co-Authored-By: Dan Lynch <[email protected]>
1 parent 104799a commit 4f0c2da

File tree

3 files changed

+143
-25
lines changed

3 files changed

+143
-25
lines changed

packages/transform/TODO.md

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# TODO: Transform Package Improvements
2+
3+
## Type Safety Improvements
4+
5+
### Add Return Type Annotations to Transformer Methods
6+
- **Priority**: High
7+
- **Description**: Add proper TypeScript return type annotations to all transformer methods in v15-to-v16.ts (and other transformers)
8+
- **Benefit**: Would catch structural issues like double-wrapping at compile time instead of runtime
9+
- **Example**:
10+
```typescript
11+
TypeName(node: PG15.TypeName, context: TransformerContext): { TypeName: PG16.TypeName } {
12+
// implementation
13+
}
14+
```
15+
- **Impact**: Prevents bugs like the double-wrapping issue we encountered where `{"TypeName": {"TypeName": {...}}}` was produced instead of `{"TypeName": {...}}`
16+
17+
### Improve Type Definitions
18+
- Add stricter typing for node transformation methods
19+
- Consider using mapped types for consistent return type patterns
20+
- Add compile-time validation for node wrapping consistency
21+
22+
## Testing Improvements
23+
- Add unit tests for individual transformer methods
24+
- Create focused test cases for edge cases like empty Integer nodes
25+
- Improve error messages in transformation mismatches
26+
27+
## Documentation
28+
- Document transformation patterns and conventions
29+
- Add examples of proper node wrapping
30+
- Document debugging strategies for transformation issues
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
const { Parser } = require('@pgsql/parser');
2+
const { ASTTransformer } = require('./dist/transformer');
3+
4+
async function debugAConstTransformation() {
5+
const parser15 = new Parser(15);
6+
const parser16 = new Parser(16);
7+
const transformer = new ASTTransformer();
8+
9+
const testCases = [
10+
"insert into atacc2 (test2) values (-3)",
11+
"ALTER TABLE onek ADD CONSTRAINT onek_check_constraint CHECK (unique1 >= 0)",
12+
"SELECT -1",
13+
"SELECT -3"
14+
];
15+
16+
console.log("=== DEBUGGING A_CONST TRANSFORMATION PATTERNS ===");
17+
18+
for (const sql of testCases) {
19+
console.log(`\n=== SQL: ${sql} ===`);
20+
21+
try {
22+
const pg15Result = await parser15.parse(sql);
23+
const pg16Result = await parser16.parse(sql);
24+
25+
const pg15AConsts = findAConstNodes(pg15Result);
26+
const pg16AConsts = findAConstNodes(pg16Result);
27+
28+
console.log("PG15 A_Const nodes:", JSON.stringify(pg15AConsts, null, 2));
29+
console.log("PG16 A_Const nodes:", JSON.stringify(pg16AConsts, null, 2));
30+
31+
const astToTransform = JSON.parse(JSON.stringify(pg15Result));
32+
if (astToTransform.stmts && Array.isArray(astToTransform.stmts)) {
33+
astToTransform.stmts = astToTransform.stmts.map((stmtWrapper) => {
34+
if (stmtWrapper.stmt) {
35+
const transformedStmt = transformer.transform(stmtWrapper.stmt, 15, 16);
36+
return { ...stmtWrapper, stmt: transformedStmt };
37+
}
38+
return stmtWrapper;
39+
});
40+
}
41+
42+
const transformedAConsts = findAConstNodes(astToTransform);
43+
console.log("Transformed A_Const nodes:", JSON.stringify(transformedAConsts, null, 2));
44+
45+
} catch (error) {
46+
console.error("Error:", error.message);
47+
}
48+
}
49+
}
50+
51+
function findAConstNodes(obj, path = []) {
52+
const results = [];
53+
54+
if (typeof obj !== 'object' || obj === null) {
55+
return results;
56+
}
57+
58+
if (obj.A_Const) {
59+
results.push({
60+
path: path.concat(['A_Const']),
61+
node: obj.A_Const
62+
});
63+
}
64+
65+
for (const [key, value] of Object.entries(obj)) {
66+
if (typeof value === 'object' && value !== null) {
67+
results.push(...findAConstNodes(value, path.concat([key])));
68+
}
69+
}
70+
71+
return results;
72+
}
73+
74+
debugAConstTransformation();

packages/transform/src/transformers/v15-to-v16.ts

Lines changed: 39 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -35,28 +35,22 @@ export class V15ToV16Transformer {
3535
visit(node: PG15.Node, context: TransformerContext = { parentNodeTypes: [] }): any {
3636
const nodeType = this.getNodeType(node);
3737

38-
3938
// Handle empty objects
4039
if (!nodeType) {
4140
return {};
4241
}
4342

4443
const nodeData = this.getNodeData(node);
4544

46-
4745
const methodName = nodeType as keyof this;
4846
if (typeof this[methodName] === 'function') {
4947
const childContext: TransformerContext = {
5048
...context,
5149
parentNodeTypes: [...context.parentNodeTypes, nodeType]
5250
};
53-
const result = (this[methodName] as any)(nodeData, childContext);
54-
55-
56-
return result;
51+
return (this[methodName] as any)(nodeData, childContext);
5752
}
5853

59-
6054
// If no specific method, return the node as-is
6155
return node;
6256
}
@@ -514,34 +508,53 @@ export class V15ToV16Transformer {
514508
}
515509

516510
A_Const(node: PG15.A_Const, context: TransformerContext): any {
517-
const result: any = {};
511+
const result: any = { ...node };
512+
513+
if (result.val) {
514+
const val: any = result.val;
515+
if (val.String && val.String.str !== undefined) {
516+
result.sval = { sval: val.String.str };
517+
delete result.val;
518+
} else if (val.Integer && val.Integer.ival !== undefined) {
519+
result.ival = val.Integer.ival;
520+
delete result.val;
521+
} else if (val.Float && val.Float.str !== undefined) {
522+
result.fval = { fval: val.Float.str };
523+
delete result.val;
524+
} else if (val.BitString && val.BitString.str !== undefined) {
525+
result.bsval = { bsval: val.BitString.str };
526+
delete result.val;
527+
} else if (val.Null !== undefined) {
528+
delete result.val;
529+
}
530+
}
518531

519-
if (node.ival !== undefined) {
520-
result.ival = this.transform(node.ival as any, context);
532+
if (result.ival !== undefined) {
533+
result.ival = this.transform(result.ival as any, context);
521534
}
522535

523-
if (node.fval !== undefined) {
524-
result.fval = this.transform(node.fval as any, context);
536+
if (result.fval !== undefined) {
537+
result.fval = this.transform(result.fval as any, context);
525538
}
526539

527-
if (node.boolval !== undefined) {
528-
result.boolval = this.transform(node.boolval as any, context);
540+
if (result.boolval !== undefined) {
541+
result.boolval = this.transform(result.boolval as any, context);
529542
}
530543

531-
if (node.sval !== undefined) {
532-
result.sval = this.transform(node.sval as any, context);
544+
if (result.sval !== undefined) {
545+
result.sval = this.transform(result.sval as any, context);
533546
}
534547

535-
if (node.bsval !== undefined) {
536-
result.bsval = this.transform(node.bsval as any, context);
548+
if (result.bsval !== undefined) {
549+
result.bsval = this.transform(result.bsval as any, context);
537550
}
538551

539-
if (node.isnull !== undefined) {
540-
result.isnull = node.isnull;
552+
if (result.isnull !== undefined) {
553+
result.isnull = result.isnull;
541554
}
542555

543-
if (node.location !== undefined) {
544-
result.location = node.location;
556+
if (result.location !== undefined) {
557+
result.location = result.location;
545558
}
546559

547560
return { A_Const: result };
@@ -867,8 +880,8 @@ export class V15ToV16Transformer {
867880
Integer(node: PG15.Integer, context: TransformerContext): any {
868881
const result: any = { ...node };
869882

870-
// Handle case where PG15 produces empty Integer nodes that should have ival: -1 in PG16
871-
if (Object.keys(result).length === 0) {
883+
// Handle case where PG15 produces empty Integer nodes in arrayBounds that should have ival: -1 in PG16
884+
if (Object.keys(node).length === 0 && context.parentNodeTypes.includes('TypeName')) {
872885
result.ival = -1;
873886
}
874887

@@ -994,7 +1007,8 @@ export class V15ToV16Transformer {
9941007
}
9951008

9961009
if (node.typeName !== undefined) {
997-
result.typeName = this.transform(node.typeName as any, context);
1010+
const transformedTypeName = this.transform({ TypeName: node.typeName } as any, context);
1011+
result.typeName = transformedTypeName.TypeName;
9981012
}
9991013

10001014
if (node.inhcount !== undefined) {

0 commit comments

Comments
 (0)