Skip to content

Commit 2844a9b

Browse files
fix: revert overly broad A_Const ival transformation
- Reverted targeted fix that transformed all empty ival objects to {ival: -1} - Caused test pass rate to drop from 184/258 to 144/258 (too aggressive) - Restored stable baseline of 184/258 tests passing (71.3% success rate) - Need more sophisticated approach to distinguish zero vs negative integer contexts Co-Authored-By: Dan Lynch <[email protected]>
1 parent c4c40ad commit 2844a9b

File tree

6 files changed

+295
-17
lines changed

6 files changed

+295
-17
lines changed
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
const fs = require('fs');
2+
3+
function analyzeCIFailures() {
4+
const logFile = '/home/ubuntu/full_outputs/gh_run_view_log_fail_1751134169.160914.txt';
5+
6+
try {
7+
const content = fs.readFileSync(logFile, 'utf8');
8+
9+
const lines = content.split('\n');
10+
const failures = [];
11+
let currentFailure = null;
12+
13+
for (let i = 0; i < lines.length; i++) {
14+
const line = lines[i];
15+
16+
if (line.includes('FAIL __tests__/kitchen-sink/')) {
17+
const match = line.match(/FAIL __tests__\/kitchen-sink\/(\d+-\d+)\//);
18+
if (match) {
19+
if (currentFailure) {
20+
failures.push(currentFailure);
21+
}
22+
currentFailure = {
23+
transformer: match[1],
24+
testFile: line.split('FAIL ')[1],
25+
errors: []
26+
};
27+
}
28+
}
29+
30+
if (currentFailure && (line.includes('- Expected') || line.includes('+ Received'))) {
31+
const errorContext = lines.slice(Math.max(0, i-5), i+10).join('\n');
32+
currentFailure.errors.push(errorContext);
33+
}
34+
}
35+
36+
if (currentFailure) {
37+
failures.push(currentFailure);
38+
}
39+
40+
console.log('=== CI FAILURE ANALYSIS ===');
41+
console.log(`Total failures found: ${failures.length}`);
42+
43+
const transformerCounts = {};
44+
failures.forEach(f => {
45+
transformerCounts[f.transformer] = (transformerCounts[f.transformer] || 0) + 1;
46+
});
47+
48+
console.log('\n=== FAILURES BY TRANSFORMER ===');
49+
Object.entries(transformerCounts).forEach(([transformer, count]) => {
50+
console.log(`${transformer}: ${count} failures`);
51+
});
52+
53+
const errorPatterns = {
54+
'str_to_sval': 0,
55+
'missing_wrapping': 0,
56+
'ival_issues': 0
57+
};
58+
59+
failures.forEach(f => {
60+
f.errors.forEach(error => {
61+
if (error.includes('"str":') && error.includes('"sval":')) {
62+
errorPatterns.str_to_sval++;
63+
}
64+
if (error.includes('+ ') && error.includes('Object {')) {
65+
errorPatterns.missing_wrapping++;
66+
}
67+
if (error.includes('ival')) {
68+
errorPatterns.ival_issues++;
69+
}
70+
});
71+
});
72+
73+
console.log('\n=== ERROR PATTERNS ===');
74+
Object.entries(errorPatterns).forEach(([pattern, count]) => {
75+
console.log(`${pattern}: ${count} occurrences`);
76+
});
77+
78+
const my_failures = failures.filter(f => f.transformer === '15-16').slice(0, 3);
79+
console.log('\n=== SAMPLE 15-16 FAILURES ===');
80+
my_failures.forEach((f, i) => {
81+
console.log(`\n--- Failure ${i+1}: ${f.testFile} ---`);
82+
if (f.errors.length > 0) {
83+
console.log(f.errors[0].substring(0, 500) + '...');
84+
}
85+
});
86+
87+
} catch (error) {
88+
console.error('Error analyzing CI failures:', error.message);
89+
}
90+
}
91+
92+
analyzeCIFailures();
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
const { Parser } = require('@pgsql/parser');
2+
const { V15ToV16Transformer } = require('./dist/transformers/v15-to-v16');
3+
4+
class DebugTransformer extends V15ToV16Transformer {
5+
Integer(node, context) {
6+
console.log('=== Integer method called ===');
7+
console.log('Node:', JSON.stringify(node, null, 2));
8+
console.log('Context parentNodeTypes:', context.parentNodeTypes);
9+
console.log('Context path:', context.path);
10+
console.log('Object.keys(node).length:', Object.keys(node).length);
11+
console.log('Is empty?', Object.keys(node).length === 0);
12+
console.log('Has TypeName parent?', context.parentNodeTypes.includes('TypeName'));
13+
console.log('Has A_Const parent?', context.parentNodeTypes.includes('A_Const'));
14+
15+
const result = super.Integer(node, context);
16+
console.log('Result:', JSON.stringify(result, null, 2));
17+
console.log('================================');
18+
19+
return result;
20+
}
21+
22+
A_Const(node, context) {
23+
console.log('=== A_Const method called ===');
24+
console.log('Node:', JSON.stringify(node, null, 2));
25+
console.log('Context parentNodeTypes:', context.parentNodeTypes);
26+
console.log('Context path:', context.path);
27+
28+
const result = super.A_Const(node, context);
29+
console.log('A_Const result:', JSON.stringify(result, null, 2));
30+
console.log('================================');
31+
32+
return result;
33+
}
34+
}
35+
36+
async function debugContextDetailed() {
37+
const parser15 = new Parser(15);
38+
const transformer = new DebugTransformer();
39+
40+
const sql = "insert into atacc2 (test2) values (-3)";
41+
42+
console.log("=== DEBUGGING DETAILED CONTEXT ===");
43+
console.log("SQL:", sql);
44+
45+
try {
46+
const pg15Result = await parser15.parse(sql);
47+
48+
console.log("\n=== STARTING TRANSFORMATION ===");
49+
const astToTransform = JSON.parse(JSON.stringify(pg15Result));
50+
if (astToTransform.stmts && Array.isArray(astToTransform.stmts)) {
51+
astToTransform.stmts = astToTransform.stmts.map((stmtWrapper) => {
52+
if (stmtWrapper.stmt) {
53+
const transformedStmt = transformer.transform(stmtWrapper.stmt, { parentNodeTypes: [] });
54+
return { ...stmtWrapper, stmt: transformedStmt };
55+
}
56+
return stmtWrapper;
57+
});
58+
}
59+
60+
console.log("\n=== TRANSFORMATION COMPLETE ===");
61+
62+
} catch (error) {
63+
console.error("Error:", error.message);
64+
}
65+
}
66+
67+
debugContextDetailed();

packages/transform/debug_insert_negative.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
const { Parser } = require('@pgsql/parser');
2-
const { ASTTransformer } = require('./dist/transformer');
2+
const { V15ToV16Transformer } = require('./dist/transformers/v15-to-v16');
33

44
async function debugInsertNegative() {
55
const parser15 = new Parser(15);
66
const parser16 = new Parser(16);
7-
const transformer = new ASTTransformer();
7+
const transformer = new V15ToV16Transformer();
88

99
const sql = "insert into atacc2 (test2) values (-3)";
1010

@@ -33,7 +33,7 @@ async function debugInsertNegative() {
3333
if (astToTransform.stmts && Array.isArray(astToTransform.stmts)) {
3434
astToTransform.stmts = astToTransform.stmts.map((stmtWrapper) => {
3535
if (stmtWrapper.stmt) {
36-
const transformedStmt = transformer.transform(stmtWrapper.stmt, 15, 16);
36+
const transformedStmt = transformer.transform(stmtWrapper.stmt, { parentNodeTypes: [] });
3737
return { ...stmtWrapper, stmt: transformedStmt };
3838
}
3939
return stmtWrapper;
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
const { Parser } = require('@pgsql/parser');
2+
const { ASTTransformer } = require('./dist/transformer');
3+
4+
class DebugTransformer extends ASTTransformer {
5+
transform(node, fromVersion, toVersion) {
6+
console.log('=== Transform called ===');
7+
console.log('Node type:', this.getNodeType(node));
8+
console.log('Node data:', JSON.stringify(this.getNodeData(node), null, 2));
9+
console.log('========================');
10+
11+
return super.transform(node, fromVersion, toVersion);
12+
}
13+
14+
A_Const(node, context) {
15+
console.log('=== A_Const called ===');
16+
console.log('Node:', JSON.stringify(node, null, 2));
17+
console.log('Context path:', context.path);
18+
console.log('Parent types:', context.parentNodeTypes);
19+
console.log('========================');
20+
21+
return super.A_Const(node, context);
22+
}
23+
24+
Integer(node, context) {
25+
console.log('=== Integer called ===');
26+
console.log('Node:', JSON.stringify(node, null, 2));
27+
console.log('Context path:', context.path);
28+
console.log('Parent types:', context.parentNodeTypes);
29+
console.log('========================');
30+
31+
return super.Integer(node, context);
32+
}
33+
}
34+
35+
async function debugTransformationFlow() {
36+
const parser15 = new Parser(15);
37+
const transformer = new DebugTransformer();
38+
39+
const sql = "insert into atacc2 (test2) values (-3)";
40+
41+
console.log("=== DEBUGGING TRANSFORMATION FLOW ===");
42+
console.log("SQL:", sql);
43+
44+
try {
45+
const pg15Result = await parser15.parse(sql);
46+
47+
console.log("\n=== STARTING TRANSFORMATION ===");
48+
const astToTransform = JSON.parse(JSON.stringify(pg15Result));
49+
if (astToTransform.stmts && Array.isArray(astToTransform.stmts)) {
50+
astToTransform.stmts = astToTransform.stmts.map((stmtWrapper) => {
51+
if (stmtWrapper.stmt) {
52+
const transformedStmt = transformer.transform(stmtWrapper.stmt, 15, 16);
53+
return { ...stmtWrapper, stmt: transformedStmt };
54+
}
55+
return stmtWrapper;
56+
});
57+
}
58+
59+
console.log("\n=== TRANSFORMATION COMPLETE ===");
60+
61+
} catch (error) {
62+
console.error("Error:", error.message);
63+
}
64+
}
65+
66+
debugTransformationFlow();
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
const { Parser } = require('@pgsql/parser');
2+
const { V15ToV16Transformer } = require('./dist/transformers/v15-to-v16');
3+
4+
async function debugZeroVsNegative() {
5+
const parser15 = new Parser(15);
6+
const parser16 = new Parser(16);
7+
const transformer = new V15ToV16Transformer();
8+
9+
const testCases = [
10+
"insert into atacc2 (test2) values (-3)", // negative integer
11+
"ALTER TABLE onek ADD CONSTRAINT onek_check_constraint CHECK (unique1 >= 0)" // zero
12+
];
13+
14+
console.log("=== DEBUGGING ZERO VS NEGATIVE INTEGER ===");
15+
16+
for (const sql of testCases) {
17+
console.log(`\n=== SQL: ${sql} ===`);
18+
19+
try {
20+
const pg15Result = await parser15.parse(sql);
21+
const pg16Result = await parser16.parse(sql);
22+
23+
const pg15AConst = findAConstInAST(pg15Result);
24+
const pg16AConst = findAConstInAST(pg16Result);
25+
26+
console.log("PG15 A_Const ival:", JSON.stringify(pg15AConst?.ival));
27+
console.log("PG16 A_Const ival:", JSON.stringify(pg16AConst?.ival));
28+
29+
const astToTransform = JSON.parse(JSON.stringify(pg15Result));
30+
if (astToTransform.stmts && Array.isArray(astToTransform.stmts)) {
31+
astToTransform.stmts = astToTransform.stmts.map((stmtWrapper) => {
32+
if (stmtWrapper.stmt) {
33+
const transformedStmt = transformer.transform(stmtWrapper.stmt, { parentNodeTypes: [] });
34+
return { ...stmtWrapper, stmt: transformedStmt };
35+
}
36+
return stmtWrapper;
37+
});
38+
}
39+
40+
const transformedAConst = findAConstInAST(astToTransform);
41+
console.log("Transformed ival:", JSON.stringify(transformedAConst?.ival));
42+
43+
console.log("Should transform?", pg16AConst?.ival && Object.keys(pg16AConst.ival).length > 0 ? "YES" : "NO");
44+
45+
} catch (error) {
46+
console.error("Error:", error.message);
47+
}
48+
}
49+
}
50+
51+
function findAConstInAST(obj) {
52+
if (!obj || typeof obj !== 'object') return null;
53+
54+
if (obj.A_Const) return obj.A_Const;
55+
56+
for (const key in obj) {
57+
if (typeof obj[key] === 'object') {
58+
const result = findAConstInAST(obj[key]);
59+
if (result) return result;
60+
}
61+
}
62+
63+
return null;
64+
}
65+
66+
debugZeroVsNegative();

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

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -530,20 +530,7 @@ export class V15ToV16Transformer {
530530
}
531531

532532
if (result.ival !== undefined) {
533-
// Handle case where PG15 produces empty ival objects for negative integers
534-
if (typeof result.ival === 'object' && Object.keys(result.ival).length === 0) {
535-
// Check if we're in an INSERT context which typically has negative integers
536-
const hasInsertContext = context.parentNodeTypes && context.parentNodeTypes.includes('InsertStmt');
537-
const hasSelectContext = context.parentNodeTypes && context.parentNodeTypes.includes('SelectStmt');
538-
539-
if (hasInsertContext || hasSelectContext) {
540-
result.ival = { ival: -1 }; // Use -1 as default since PG15 loses the original value
541-
} else {
542-
result.ival = result.ival;
543-
}
544-
} else {
545-
result.ival = this.transform(result.ival as any, context);
546-
}
533+
result.ival = this.transform(result.ival as any, context);
547534
}
548535

549536
if (result.fval !== undefined) {

0 commit comments

Comments
 (0)