Merge branch 'pg13-pg14-base' into pg14-pg15-transformer

Author: pyramation

packages/transform/STATUS-15-16.md

@@ -1 +1,57 @@
-not started — see previous status docs for format style.
+# PostgreSQL v15-to-v16 AST Transformer Status
+
+## Current Status: **IN PROGRESS** 🟡
+- **Test Pass Rate**: 184/258 tests passing (71.3% success rate)
+- **Branch**: `pg15-pg16-transformer` 
+- **PR**: [#175](https://github.com/launchql/pgsql-parser/pull/175)
+
+## Progress Summary
+Started from a basic skeleton transformer and systematically implemented node wrapping and transformation logic across all node types. Made significant progress improving test pass rate from initial ~30% to current 71.3%.
+
+## Key Achievements
+- ✅ Implemented comprehensive node transformation methods for 100+ node types
+- ✅ Fixed node wrapping issues across SelectStmt, InsertStmt, UpdateStmt, DeleteStmt
+- ✅ Resolved PartitionSpec strategy mapping in CreateStmt method
+- ✅ Added proper Array handling to transform method for nested node processing
+- ✅ Established stable baseline of 184/258 tests passing locally
+
+## Current Challenge: Negative Integer Transformation
+**Root Issue**: PG15 produces `"ival": {}` (empty objects) where PG16 expects `"ival": {"ival": -3}` for negative integers in A_Const nodes.
+
+**Analysis Completed**:
+- Created detailed debug scripts to analyze transformation flow
+- Identified that A_Const method calls `this.transform()` on empty ival objects
+- Empty objects `{}` don't get routed to Integer method due to missing node wrapper structure
+- Need targeted fix that distinguishes between zero values (should remain empty) and negative values (need nested structure)
+
+**Attempted Solutions**:
+- ❌ Broad A_Const fix (transforms all empty ival objects) - caused test pass rate to drop to 144/258
+- ❌ Context-based detection (constraint/ALTER TABLE contexts) - caused test pass rate to drop to 174/258
+- ✅ Successfully reverted to stable 184/258 baseline after testing approaches
+- 🔄 Dual-parse approach explored but @pgsql/parser returns empty objects for all SQL queries
+
+## Debug Tools Created
+- `debug_transformation_flow_detailed.js` - Analyzes exact transformation flow for negative integers
+- `debug_sql_value_analysis.js` - Compares PG15 vs PG16 behavior across test cases
+- `debug_ival_contexts.js` - Analyzes empty ival contexts across different SQL scenarios
+- `debug_alternative_approach.js` - Explores alternative detection methods beyond context analysis
+- `debug_insert_negative.js` - Tests specific INSERT statement with negative value
+- `debug_zero_vs_negative.js` - Compares zero vs negative value handling
+- `debug_context_analysis.js` - Analyzes context-dependent transformation patterns
+
+## Next Steps
+1. Investigate alternative approaches beyond context-based and dual-parse methods
+2. Consider advanced pattern matching or heuristic detection for negative integer cases
+3. Explore selective transformation targeting only high-confidence scenarios
+4. Verify specific failing test cases like `alter_table-234.sql`
+5. Continue systematic improvement of remaining 74 failing tests
+
+## Test Categories
+- **Passing (184)**: Basic node transformations, most SQL constructs
+- **Failing (74)**: Primarily negative integer transformations, some complex nested structures
+
+## Technical Notes
+- Following patterns from v13-to-v14 transformer as reference
+- Focus only on v15-to-v16 transformer per user instructions
+- Ignoring CI failures per user directive, focusing on local test improvements
+- Maintaining systematic approach to avoid regressions

packages/transform/STATUS-16-17.md

@@ -1 +1,47 @@
-not started — see previous status docs for format style.
+# PostgreSQL 16-to-17 AST Transformer Status
+
+## Current Status: 98.8% Complete (255/258 tests passing)
+
+### Test Results (Confirmed: June 28, 2025)
+- **Total tests**: 258
+- **Passing**: 255 (98.8%)
+- **Failing**: 3 (1.2%)
+
+### Progress Summary
+- ✅ **Core transformer implementation**: Complete
+- ✅ **Basic AST node transformations**: Complete  
+- ✅ **Domain creation contexts**: Fixed
+- ✅ **SELECT statement contexts**: Fixed
+- ⚠️ **Complex nested contexts**: 3 remaining failures
+
+### Remaining Test Failures
+1. **pretty-misc.test.ts**: JSON TypeCast prefix handling
+   - Issue: Transformer adding pg_catalog prefix when expected output has none
+   - Context: WITH clauses containing A_Expr with JSON TypeCasts
+   - Status: Logic needs to be reversed - remove prefixes instead of adding them
+
+2. **misc-quotes_etc.test.ts**: String escaping issue
+   - Issue: \v character handling difference between PG16/PG17
+   - Expected: `\v` → Received: `v`
+   - Status: Likely parser-level difference, not transformer issue
+
+3. **latest-postgres-create_am.test.ts**: CREATE ACCESS METHOD syntax
+   - Issue: `syntax error at or near "DEFAULT"`
+   - Status: PG16 parser limitation - syntax not supported in PG16
+
+### Key Insights
+- **JSON prefix logic**: Test failures show expected output does NOT want pg_catalog prefixes
+- **String escaping**: PG16/PG17 parser difference in escape sequence handling
+- **CREATE ACCESS METHOD**: PG16 parser doesn't support this PG17 syntax feature
+
+### Technical Details
+- **Branch**: pg16-pg17-transformer
+- **PR**: #177 (https://github.com/launchql/pgsql-parser/pull/177)
+- **Last test run**: June 28, 2025 20:47 UTC
+- **Current approach**: Context-aware transformation based on parent node types
+
+### Next Steps to Achieve 100%
+1. **Remove JSON pg_catalog prefix logic entirely** from TypeCast method
+2. **Investigate String method** for \v character handling
+3. **Document CREATE ACCESS METHOD limitation** as expected PG16 parser constraint
+4. **Final test run** to confirm 258/258 success rate

packages/transform/TODO.md

@@ -0,0 +1,30 @@
+# TODO: Transform Package Improvements
+
+## Type Safety Improvements
+
+### Add Return Type Annotations to Transformer Methods
+- **Priority**: High
+- **Description**: Add proper TypeScript return type annotations to all transformer methods in v15-to-v16.ts (and other transformers)
+- **Benefit**: Would catch structural issues like double-wrapping at compile time instead of runtime
+- **Example**: 
+  ```typescript
+  TypeName(node: PG15.TypeName, context: TransformerContext): { TypeName: PG16.TypeName } {
+    // implementation
+  }
+  ```
+- **Impact**: Prevents bugs like the double-wrapping issue we encountered where `{"TypeName": {"TypeName": {...}}}` was produced instead of `{"TypeName": {...}}`
+
+### Improve Type Definitions
+- Add stricter typing for node transformation methods
+- Consider using mapped types for consistent return type patterns
+- Add compile-time validation for node wrapping consistency
+
+## Testing Improvements
+- Add unit tests for individual transformer methods
+- Create focused test cases for edge cases like empty Integer nodes
+- Improve error messages in transformation mismatches
+
+## Documentation
+- Document transformation patterns and conventions
+- Add examples of proper node wrapping
+- Document debugging strategies for transformation issues

packages/transform/analyze_15_16_ci_failures.js

@@ -0,0 +1,104 @@
+const fs = require('fs');
+
+function analyze15To16CIFailures() {
+  const logFile = '/home/ubuntu/full_outputs/gh_run_view_log_fail_1751134577.6148772.txt';
+  
+  try {
+    const content = fs.readFileSync(logFile, 'utf8');
+    
+    const lines = content.split('\n');
+    const failures15to16 = [];
+    let currentFailure = null;
+    let isIn15to16 = false;
+    
+    for (let i = 0; i < lines.length; i++) {
+      const line = lines[i];
+      
+      if (line.includes('FAIL __tests__/kitchen-sink/15-16/')) {
+        isIn15to16 = true;
+        if (currentFailure) {
+          failures15to16.push(currentFailure);
+        }
+        currentFailure = {
+          testFile: line.split('FAIL ')[1],
+          errors: [],
+          fullContext: []
+        };
+      } else if (line.includes('FAIL __tests__/kitchen-sink/') && !line.includes('15-16')) {
+        isIn15to16 = false;
+        if (currentFailure && isIn15to16) {
+          failures15to16.push(currentFailure);
+          currentFailure = null;
+        }
+      }
+      
+      if (isIn15to16 && currentFailure) {
+        currentFailure.fullContext.push(line);
+        
+        if (line.includes('- Expected') || line.includes('+ Received') || 
+            line.includes('ival') || line.includes('Integer')) {
+          const errorContext = lines.slice(Math.max(0, i-3), i+7).join('\n');
+          currentFailure.errors.push(errorContext);
+        }
+      }
+    }
+    
+    if (currentFailure && isIn15to16) {
+      failures15to16.push(currentFailure);
+    }
+    
+    console.log('=== 15-16 CI FAILURE ANALYSIS ===');
+    console.log(`Total 15-16 failures found: ${failures15to16.length}`);
+    
+    const errorPatterns = {
+      'ival_empty_to_nested': 0,
+      'integer_wrapping': 0,
+      'negative_integer': 0,
+      'missing_node_wrapping': 0
+    };
+    
+    failures15to16.forEach(f => {
+      f.errors.forEach(error => {
+        if (error.includes('ival') && error.includes('{}') && error.includes('ival":')) {
+          errorPatterns.ival_empty_to_nested++;
+        }
+        if (error.includes('Integer') && error.includes('+') && error.includes('-')) {
+          errorPatterns.integer_wrapping++;
+        }
+        if (error.includes('-3') || error.includes('negative')) {
+          errorPatterns.negative_integer++;
+        }
+        if (error.includes('+ ') && error.includes('Object {')) {
+          errorPatterns.missing_node_wrapping++;
+        }
+      });
+    });
+    
+    console.log('\n=== 15-16 ERROR PATTERNS ===');
+    Object.entries(errorPatterns).forEach(([pattern, count]) => {
+      console.log(`${pattern}: ${count} occurrences`);
+    });
+    
+    console.log('\n=== DETAILED 15-16 FAILURES ===');
+    failures15to16.slice(0, 3).forEach((f, i) => {
+      console.log(`\n--- Failure ${i+1}: ${f.testFile} ---`);
+      if (f.errors.length > 0) {
+        console.log(f.errors[0].substring(0, 800));
+      } else {
+        console.log('No specific error patterns found, showing context:');
+        console.log(f.fullContext.slice(-20).join('\n').substring(0, 800));
+      }
+      console.log('...\n');
+    });
+    
+    console.log('\n=== COMPARISON WITH LOCAL FAILURES ===');
+    console.log('CI shows 148 failures in 15-16 transformer');
+    console.log('Local shows 74 failures in 15-16 transformer');
+    console.log('Difference suggests CI may be testing additional scenarios or environment differences');
+    
+  } catch (error) {
+    console.error('Error analyzing CI failures:', error.message);
+  }
+}
+
+analyze15To16CIFailures();

packages/transform/analyze_ci_failures.js

@@ -0,0 +1,92 @@
+const fs = require('fs');
+
+function analyzeCIFailures() {
+  const logFile = '/home/ubuntu/full_outputs/gh_run_view_log_fail_1751134169.160914.txt';
+  
+  try {
+    const content = fs.readFileSync(logFile, 'utf8');
+    
+    const lines = content.split('\n');
+    const failures = [];
+    let currentFailure = null;
+    
+    for (let i = 0; i < lines.length; i++) {
+      const line = lines[i];
+      
+      if (line.includes('FAIL __tests__/kitchen-sink/')) {
+        const match = line.match(/FAIL __tests__\/kitchen-sink\/(\d+-\d+)\//);
+        if (match) {
+          if (currentFailure) {
+            failures.push(currentFailure);
+          }
+          currentFailure = {
+            transformer: match[1],
+            testFile: line.split('FAIL ')[1],
+            errors: []
+          };
+        }
+      }
+      
+      if (currentFailure && (line.includes('- Expected') || line.includes('+ Received'))) {
+        const errorContext = lines.slice(Math.max(0, i-5), i+10).join('\n');
+        currentFailure.errors.push(errorContext);
+      }
+    }
+    
+    if (currentFailure) {
+      failures.push(currentFailure);
+    }
+    
+    console.log('=== CI FAILURE ANALYSIS ===');
+    console.log(`Total failures found: ${failures.length}`);
+    
+    const transformerCounts = {};
+    failures.forEach(f => {
+      transformerCounts[f.transformer] = (transformerCounts[f.transformer] || 0) + 1;
+    });
+    
+    console.log('\n=== FAILURES BY TRANSFORMER ===');
+    Object.entries(transformerCounts).forEach(([transformer, count]) => {
+      console.log(`${transformer}: ${count} failures`);
+    });
+    
+    const errorPatterns = {
+      'str_to_sval': 0,
+      'missing_wrapping': 0,
+      'ival_issues': 0
+    };
+    
+    failures.forEach(f => {
+      f.errors.forEach(error => {
+        if (error.includes('"str":') && error.includes('"sval":')) {
+          errorPatterns.str_to_sval++;
+        }
+        if (error.includes('+ ') && error.includes('Object {')) {
+          errorPatterns.missing_wrapping++;
+        }
+        if (error.includes('ival')) {
+          errorPatterns.ival_issues++;
+        }
+      });
+    });
+    
+    console.log('\n=== ERROR PATTERNS ===');
+    Object.entries(errorPatterns).forEach(([pattern, count]) => {
+      console.log(`${pattern}: ${count} occurrences`);
+    });
+    
+    const my_failures = failures.filter(f => f.transformer === '15-16').slice(0, 3);
+    console.log('\n=== SAMPLE 15-16 FAILURES ===');
+    my_failures.forEach((f, i) => {
+      console.log(`\n--- Failure ${i+1}: ${f.testFile} ---`);
+      if (f.errors.length > 0) {
+        console.log(f.errors[0].substring(0, 500) + '...');
+      }
+    });
+    
+  } catch (error) {
+    console.error('Error analyzing CI failures:', error.message);
+  }
+}
+
+analyzeCIFailures();