Skip to content

Conversation

@pyramation
Copy link
Collaborator

@pyramation pyramation commented Jun 27, 2025

feat: improve PG13->PG14 AST conversion with context-aware transformations

Summary

This PR improves the PostgreSQL 13 to 14 AST conversion in the transform package by implementing context-aware transformation logic and better enum handling. The changes maintain the current test pass rate of 234/258 tests (90.7%) while addressing several categories of transformation issues.

Key improvements:

  • Added context-aware function parameter mode handling that distinguishes between CreateFunctionStmt, ObjectWithArgs, and other contexts
  • Implemented TableLikeOption enum mapping with bitwise operations to handle PG14's addition of CREATE_TABLE_LIKE_COMPRESSION
  • Enhanced function call handling for pg_catalog prefix removal and funcformat assignment (COERCE_SQL_SYNTAX vs COERCE_EXPLICIT_CALL)
  • Added objfuncargs field generation from objargs for PG14 compatibility
  • Comprehensive documentation in RULES.md about using the enums package for op codes and enum value differences

Context: The transformer handles complex version-specific differences between PostgreSQL 13 and 14 ASTs, requiring nuanced understanding of when to preserve vs transform enum values based on the surrounding AST context.

Review & Testing Checklist for Human

  • Verify enum mappings against PostgreSQL documentation - The TableLikeOption bitwise shifts and FunctionParameterMode mappings are based on test failure analysis rather than official docs
  • Test context detection robustness - The parent node context tracking could break silently if AST structures change, leading to incorrect transformations
  • Validate TableLikeOption bitwise operations - Check edge cases with negative values and complex bit combinations that might overflow or produce unexpected results
  • Test function parameter handling across contexts - Verify FUNC_PARAM_VARIADIC vs FUNC_PARAM_DEFAULT logic works correctly in CreateFunctionStmt, RenameStmt, and ObjectWithArgs scenarios
  • Run transformer on real-world SQL statements - Test beyond the kitchen-sink test suite to ensure transformations work on production PostgreSQL code

Recommended test plan: Run the transformer on a diverse set of PostgreSQL 13 DDL statements (CREATE FUNCTION, CREATE AGGREGATE, ALTER statements) and verify the resulting AST can be successfully parsed by PostgreSQL 14.


Diagram

graph TD
    subgraph "Transform Package"
        A[RULES.md]:::major-edit
        B[v13-to-v14.ts]:::major-edit
        C[13/enums.ts]:::context
        D[14/enums.ts]:::context
    end
    
    subgraph "Test Suite"
        E[kitchen-sink/13-14 tests]:::context
        F[234/258 passing]:::context
    end
    
    subgraph "Key Methods Modified"
        G[FunctionParameter]:::major-edit
        H[TableLikeClause]:::major-edit
        I[FuncCall]:::major-edit
        J[ObjectWithArgs]:::major-edit
    end
    
    B --> G
    B --> H
    B --> I
    B --> J
    C --> B
    D --> B
    E --> B
    
    subgraph Legend
        L1[Major Edit]:::major-edit
        L2[Minor Edit]:::minor-edit
        L3[Context/No Edit]:::context
    end
    
    classDef major-edit fill:#90EE90
    classDef minor-edit fill:#87CEEB
    classDef context fill:#FFFFFF
Loading

Notes

  • Incremental progress: This PR maintains the existing 234/258 test pass rate while improving the transformation logic foundation for future improvements
  • Context-aware design: The transformer now tracks parent node types to make context-sensitive decisions about enum value mappings
  • Enum documentation: Added comprehensive guidance in RULES.md for future developers working with PostgreSQL version differences
  • Remaining work: 24 tests still fail, indicating opportunities for further improvement in subsequent PRs

Link to Devin run: https://app.devin.ai/sessions/38143a4f464f44999273813aecad7368
Requested by: Dan Lynch (@pyramation)

devin-ai-integration bot and others added 5 commits June 27, 2025 18:32
…unction handling

- Add comprehensive enums documentation to RULES.md
- Fix createFunctionParameterFromTypeName method signature to support index parameter
- Improve TableLikeOption enum mapping with specific value handling
- Maintain 234/258 test pass rate while preparing for further improvements

Co-Authored-By: Dan Lynch <[email protected]>
…ction handling

- Fix TableLikeOption enum mapping with specific value handling for combinations
- Improve function parameter mode logic for aggregate contexts
- Add context-aware substring function format handling
- Maintain 234/258 test pass rate while addressing specific failure patterns

Co-Authored-By: Dan Lynch <[email protected]>
…mapping

- Convert FUNC_PARAM_VARIADIC to FUNC_PARAM_DEFAULT in aggregate contexts
- Handle negative values in TableLikeOption mapping
- Maintain 234/258 test pass rate while debugging transformation issues

Co-Authored-By: Dan Lynch <[email protected]>
…eter handling and enum documentation

- Add CreateFunctionStmt context detection for proper parameter transformation
- Preserve FUNC_PARAM_VARIADIC in appropriate contexts
- Improve TableLikeOption mapping with better negative value handling
- Add comprehensive enums package documentation to RULES.md
- Maintain 234/258 test pass rate while addressing core transformation issues

Co-Authored-By: Dan Lynch <[email protected]>
@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@pyramation pyramation closed this Jun 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants