Skip to content

Conversation

@pyramation
Copy link
Collaborator

@pyramation pyramation commented Nov 23, 2025

Fix ALTER EXTENSION deparsing and add DRY refactoring for ObjectType

Summary

This PR fixes multiple issues with ALTER EXTENSION statement deparsing and introduces a DRY refactoring for ObjectType keyword mapping:

  1. Fixed ALTER EXTENSION UPDATE TO deparsing: The deparser was checking for defname === 'to' but the AST uses defname === 'new_version', causing it to output NEW_VERSION 2.0 instead of UPDATE TO '2.0'. Also added proper string quoting since DefElem context returns unquoted values.

  2. Fixed ALTER EXTENSION SET SCHEMA deparsing: Changed output from SCHEMA to SET SCHEMA to match PostgreSQL syntax.

  3. Implemented AlterExtensionContentsStmt handler: Added support for ALTER EXTENSION ... ADD/DROP FUNCTION/... statements which were previously unsupported.

  4. DRY refactoring: Created centralized getObjectTypeKeyword() method to eliminate duplicate switch statements in AlterOwnerStmt and AlterObjectSchemaStmt. This reduces ~200 lines of duplicated code.

Test results: All 285 test suites pass (655 tests total), including the new misc-missing-types.test.ts that was previously failing.

Review & Testing Checklist for Human

  • Verify ALTER EXTENSION UPDATE TO '2.0' produces correctly quoted output - The string quoting logic in DefElem is manually added because DefElem context returns unquoted strings. Test that version strings are properly quoted in the output.
  • Spot check the DRY refactoring didn't break edge cases - The centralized getObjectTypeKeyword() is now used by multiple statement types. While CI passes, manually verify a few ALTER ... OWNER TO statements with different object types still work correctly.
  • Test ALTER EXTENSION ADD/DROP with different object types - The new AlterExtensionContentsStmt handler assumes action values of 1 (ADD) and -1 (DROP). Verify this works for both ADD and DROP operations with various object types (FUNCTION, TYPE, etc.).

Notes

  • The action enum values (1 = ADD, -1 = DROP) in AlterExtensionContentsStmt are hardcoded based on the AST structure. If these values are incorrect, the handler will produce wrong output.
  • The getObjectTypeKeyword() method includes a comprehensive mapping of all ObjectType enum values, but some may not be tested by the current fixture set.

Link to Devin run: https://app.devin.ai/sessions/505114984e764debaca3c75b2dab458c
Requested by: Dan Lynch ([email protected]) / @pyramation

pyramation and others added 5 commits November 22, 2025 16:11
- Fix ALTER EXTENSION UPDATE TO deparsing: change defname check from 'to' to 'new_version' and output 'UPDATE TO' instead of just 'TO'
- Fix ALTER EXTENSION SET SCHEMA deparsing: output 'SET SCHEMA' instead of just 'SCHEMA'
- Add AlterExtensionContentsStmt handler for ALTER EXTENSION ADD/DROP statements
- All tests pass (285 test suites, 655 tests)

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 Nov 23, 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