Skip to content

Conversation

@pyramation
Copy link
Collaborator

@pyramation pyramation commented Jul 1, 2025

Implement Pretty Printing for ALTER TABLE ADD COLUMN Statements

Summary

This PR implements multi-line pretty printing for ALTER TABLE ADD COLUMN statements in the PostgreSQL deparser. When context.isPretty() is enabled, column attributes (COLLATE, REFERENCES, UNIQUE, DEFAULT, GENERATED) are now formatted on separate indented lines for improved readability.

Key formatting improvements:

  • Column name and type remain on the first line
  • Attributes like NOT NULL, COLLATE, REFERENCES, UNIQUE, DEFAULT, GENERATED move to separate indented lines
  • Special handling for REFERENCES + ON DELETE CASCADE (split into separate lines)
  • IDENTITY sequences get proper multi-line indentation
  • Backward compatibility maintained for non-pretty formatting

Review & Testing Checklist for Human

  • Manual verification of pretty printing output - Test the formatted output against the expected examples in the PR description to ensure formatting matches requirements
  • Test complex constraint combinations - Verify edge cases like columns with multiple constraints (UNIQUE + DEFAULT, NOT NULL + REFERENCES + ON DELETE CASCADE)
  • Regression testing for non-pretty mode - Ensure existing non-pretty formatting still works correctly and produces the same output as before
  • End-to-end testing with real ALTER TABLE statements - Test with complex real-world ALTER TABLE ADD COLUMN statements to catch any edge cases
  • Code review of conditional logic - Review the branching logic in the AT_AddColumn case for correctness, especially the constraint handling and UNIQUE+DEFAULT combination logic

Recommended test plan: Run a variety of ALTER TABLE ADD COLUMN statements with different constraint combinations in both pretty and non-pretty modes, comparing outputs to ensure correctness.


Diagram

%%{ init : { "theme" : "default" }}%%
graph TD
    deparser["packages/deparser/src/deparser.ts<br/>AT_AddColumn case implementation"]:::major-edit
    test_file["packages/deparser/__tests__/pretty/<br/>alter-table-column.test.ts"]:::minor-edit
    snapshots["packages/deparser/__tests__/pretty/__snapshots__/<br/>alter-table-column.test.ts.snap"]:::minor-edit
    
    deparser --> test_file
    test_file --> snapshots
    
    subgraph "Key Logic Areas"
        pretty_logic["Pretty formatting logic<br/>context.isPretty() branching"]:::context
        constraint_handling["Constraint processing<br/>REFERENCES, UNIQUE, DEFAULT"]:::context
        indent_formatting["Indentation & newlines<br/>context.indent(), context.newline()"]:::context
    end
    
    deparser --> pretty_logic
    deparser --> constraint_handling  
    deparser --> indent_formatting
    
    subgraph Legend
        L1["Major Edit"]:::major-edit
        L2["Minor Edit"]:::minor-edit
        L3["Context/No Edit"]:::context
    end

classDef major-edit fill:#90EE90,stroke:#333,stroke-width:2px
classDef minor-edit fill:#87CEEB,stroke:#333,stroke-width:2px
classDef context fill:#FFFFFF,stroke:#333,stroke-width:1px
Loading

Notes

  • Session Info: Link to Devin run: https://app.devin.ai/sessions/80f0c5078bd8486490efda4f4f979851, requested by Dan Lynch (@pyramation)
  • Type Safety: Had to use (c: any) type annotation in constraints checking - this may indicate a need for better typing in the future
  • Complexity: The implementation includes intricate logic for handling different constraint combinations, which increases the importance of thorough testing
  • Test Coverage: Added test cases 7-9 to cover additional ALTER TABLE scenarios, bringing total test cases to 9
  • Backward Compatibility: Non-pretty formatting path preserved to ensure no regressions in existing functionality

@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

- Remove duplicate 'GENERATED' and 'AS IDENTITY' text in AT_AddIdentity case
- Fix sequence name formatting to use 'SEQUENCE NAME schema.table' format
- Handle NO MINVALUE/NO MAXVALUE cases when arg is null in CONSTR_IDENTITY
- Fixes __tests__/kitchen-sink/original-alter-alter-table-column.test.ts

Co-Authored-By: Dan Lynch <pyramation@gmail.com>
- Format each ALTER TABLE clause on its own line when pretty: true
- Enhanced CONSTR_IDENTITY formatting with proper indentation for sequence options
- Each sequence option appears on its own line with consistent indentation
- Follows existing pretty formatting patterns used throughout the deparser

Example output:
ALTER TABLE public.table1
  ALTER COLUMN id ADD GENERATED ALWAYS AS IDENTITY (
    SEQUENCE NAME public.table1
    START WITH 1
    INCREMENT BY 1
    NO MINVALUE
    NO MAXVALUE
    CACHE 1
  )

Co-Authored-By: Dan Lynch <pyramation@gmail.com>
- Add collClause handling to AT_AddColumn case in AlterTableCmd function
- Follows same pattern as ColumnDef function for consistency
- Fixes parse/deparse cycle for ALTER TABLE statements with COLLATE clauses

Co-Authored-By: Dan Lynch <pyramation@gmail.com>
Co-Authored-By: Dan Lynch <pyramation@gmail.com>
- Add multi-line indented formatting for column attributes
- Format COLLATE, REFERENCES, UNIQUE, DEFAULT, GENERATED on separate lines
- Maintain backward compatibility with non-pretty formatting
- Update test cases to include all 9 ALTER TABLE column scenarios
- Handle special cases like REFERENCES with ON DELETE CASCADE
- Combine UNIQUE with DEFAULT appropriately

Co-Authored-By: Dan Lynch <pyramation@gmail.com>
@pyramation pyramation merged commit b3e6378 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