Skip to content

Conversation

@pyramation
Copy link
Contributor

@pyramation pyramation commented Jul 18, 2025

Fix: Use RESTRICT for dropping extensions in revert process

Summary

This PR modifies the extension dropping behavior during the revert process to be safer and more predictable. The key change is switching from DROP EXTENSION ... CASCADE to DROP EXTENSION ... RESTRICT with proper error handling for dependency conflicts.

Key Changes:

  • Changed extension dropping from CASCADE to RESTRICT in packages/core/src/projects/revert.ts
  • Added error handling for PostgreSQL error code 2BP01 (dependent_objects_still_exist)
  • Extensions that cannot be dropped due to dependencies are now skipped with a warning instead of failing the entire revert

Motivation:
The CASCADE approach was too aggressive and could accidentally remove important migration functions and other database objects. RESTRICT provides safer behavior by explicitly failing when dependencies exist, allowing us to handle those cases gracefully.

Review & Testing Checklist for Human

  • Test revert with extensions that have dependencies - Verify that extensions with active dependencies are properly skipped with warning messages
  • Verify error handling works correctly - Test that the 2BP01 error code detection works across different PostgreSQL versions and scenarios
  • Ensure migration functions are preserved - Confirm that launchql_migrate schema functions remain intact after revert operations
  • Test backwards compatibility - Verify existing revert workflows continue to work as expected
  • Run the forked-deployment-scenarios test - Ensure the test that was previously failing now passes consistently

Recommended Test Plan:

  1. Set up a test database with extensions that have dependencies
  2. Run revert operations and verify proper handling of dependency conflicts
  3. Test both successful extension drops and cases where dependencies prevent dropping
  4. Verify that essential migration functions remain available after revert

Diagram

%%{ init : { "theme" : "default" }}%%
graph TD
    Test["forked-deployment-scenarios.test.ts"]:::context
    RevertTS["packages/core/src/projects/revert.ts"]:::major-edit
    ClientTS["packages/core/src/migrate/client.ts"]:::context
    
    Test -->|"calls"| RevertTS
    RevertTS -->|"uses migration functions from"| ClientTS
    
    RevertTS -->|"DROP EXTENSION RESTRICT"| DB[("PostgreSQL Database<br/>Extensions & Functions")]:::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:#F5F5F5
Loading

Notes

  • This change addresses the issue where the is_deployed function was missing after revert operations
  • The RESTRICT approach is more conservative but safer for production environments
  • Error code 2BP01 is PostgreSQL-specific for "dependent objects still exist"
  • Session: https://app.devin.ai/sessions/6874d1d0311440f9a2f9a30b4a804af4
  • Requested by: Dan Lynch (@pyramation)

devin-ai-integration bot and others added 2 commits July 18, 2025 10:00
- Add resolveDependencies call to revert method in LaunchQLMigrate client
- Implement cross-module tag resolution (e.g., 'my-first:@v1.0.0') in revert flow
- Add resolvedTags field to DependencyResult interface
- Fix test fixture to use new migration system instead of fast deployment
- Enable revert to specific tags across module boundaries
- Uncomment and fix test case for cross-module tag revert functionality

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 Jul 19, 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