Skip to content

Conversation

@pyramation
Copy link
Collaborator

@pyramation pyramation commented Jul 1, 2025

Fix: preserve num field in AlterTableCmd 14->15 transformation

Summary

Fixed an AST transformation mismatch where the num field was being dropped during PostgreSQL 14 to 15 AST transformation for ALTER INDEX statements. The issue occurred in the AlterTableCmd transformation method which was explicitly preserving only certain fields but missing the num field.

Root cause: The AlterTableCmd method in v14-to-v15.ts had conditional field preservation logic but was missing handling for the num field, causing it to be dropped during transformation even though both PG14 and PG15 should have identical AST structures for this statement type.

Changes made:

  • Added num field preservation in AlterTableCmd transformation method
  • Enabled the previously skipped test case latest/postgres/create_index-345.sql

Review & Testing Checklist for Human

  • Run the specific test case - Verify yarn test __tests__/kitchen-sink/14-15/latest-postgres-create_index.test.ts passes and specifically tests the SQL ALTER INDEX concur_exprs_index_expr ALTER COLUMN 1 SET STATISTICS 100
  • Check for other missing fields - Review the AlterTableCmd method to ensure no other fields are missing from the transformation logic (compare against PG14/PG15 type definitions)
  • Run full test suite - Execute yarn test to ensure no regressions were introduced by preserving this field
  • Verify pattern consistency - Confirm the added num field handling follows the same conditional preservation pattern as other fields in the method

Diagram

%%{ init : { "theme" : "default" }}%%
graph TD
    TestFile["__tests__/kitchen-sink/14-15/latest-postgres-create_index.test.ts"]:::context
    SQL["ALTER INDEX ... ALTER COLUMN 1 SET STATISTICS 100"]:::context
    SkipList["test-utils/skip-tests/transformer-errors.ts"]:::minor-edit
    Transformer["src/transformers/v14-to-v15.ts:AlterTableCmd()"]:::major-edit
    
    TestFile --> SQL
    SQL --> Transformer
    SkipList -.-> TestFile
    
    Transformer --> |"Before: drops 'num' field"| BadAST["AST missing 'num': 1"]:::context
    Transformer --> |"After: preserves 'num' field"| GoodAST["AST with 'num': 1"]:::context
    
    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

@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

- Add missing num field handling in AlterTableCmd transformation method
- Enable create_index-345.sql test that was previously skipped
- Fixes AST transformation mismatch for ALTER INDEX ... ALTER COLUMN ... SET STATISTICS

Co-Authored-By: Dan Lynch <[email protected]>
@pyramation pyramation force-pushed the pg/create-index-345 branch from 4b2a0c9 to f52083c Compare July 1, 2025 06:36
@pyramation pyramation merged commit 96fc5e4 into main Jul 1, 2025
1 check passed
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