Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement central db migrations #144

Merged
merged 8 commits into from
Feb 24, 2025
Merged

Conversation

twinguy
Copy link
Contributor

@twinguy twinguy commented Feb 21, 2025

These changes implement the following:

  • Use of a central /migrations directory instead of embedded module-specific migrations.
  • Improved SQL parsing and generation to handle table REFERENCE dependencies and prevent duplicate table creation.
  • SQL files from each module will need to be moved to this central location.

Summary of the changes made across multiple files:

  1. Interface Changes (pkg/application/interface.go):

    • Removed MigrationDirs() method
    • Removed RegisterMigrationDirs() method
  2. Application Implementation (pkg/application/application.go):

    • Modified CollectMigrations() to read from filesystem instead of embedded files
    • Added environment variable support for migration directory path
  3. Module Cleanup:

    • Removed RegisterMigrationDirs() calls from:
      • modules/bichat/module.go
      • modules/core/module.go
      • modules/crm/module.go
      • modules/finance/module.go
      • modules/logging/module.go
      • modules/warehouse/module.go
    • Removed unused migrationFiles variables from modules
  4. Migration Directory (pkg/commands/migrate_command.go):

    • Updated ensureDirectories() to create only the migrations directory
    • Added environment variable support for migration directory path
  5. SQL Parser Enhancements (pkg/schema/ast/parser.go):

    • Added referencesPattern regex to parse REFERENCES clauses
    • Enhanced ParseColumnDefinition to extract reference information:
      • Added "references", "referenced_table", "referenced_cols" metadata
    • Improved handling of column constraints and foreign key relationships
  6. Migration Generator Improvements (pkg/schema/diff/generator.go):

    • Added dependency tracking for table creation order:
    • Enhanced Generate method to:
      • Process CREATE TABLE statements first
      • Handle table dependencies correctly
      • Prevent duplicate table creation
    • Added sortChangesByDependencies method for topological sorting
    • Added isTableInList helper for duplicate detection
    • Implemented case-insensitive table name handling

Output from migration targets

iota-sdk on  main [!?⇡] via  v23.5.0
❯ make migrate-up
go run cmd/migrate/main.go up
2025/02/20 21:05:33 Applied 2 migrations

❯ psql -h localhost -d iota_erp -U postgres -W -S -c "select * from gorp_migrations;"
Password:
             id              |          applied_at
-----------------------------+-------------------------------
 changes-1740105949.down.sql | 2025-02-21 03:43:48.764501+00
 changes-1740105949.sql      | 2025-02-21 03:43:48.916475+00
(2 rows)

iota-sdk on  main [!?⇡] via  v23.5.0
❯ make migrate-down
go run cmd/migrate/main.go down
2025/02/20 21:05:37 Rolled back 2 migrations

@twinguy
Copy link
Contributor Author

twinguy commented Feb 21, 2025

@diyor28 Can you please take a look at this PR and see if I am on the right track. The core logic that applies the migrations from a central location seems to be working as expected but I am unsure of the history of how the prior implementations were expected to be applied. I also included an update to the parser and generator code that handles SQL references.

Happy to discuss/revise anything that is needed but definitely needing a second/third set of eyes on these changes.

@diyor28 diyor28 changed the base branch from main to staging February 22, 2025 11:43
@twinguy
Copy link
Contributor Author

twinguy commented Feb 24, 2025

@diyor28 Could you please review, I have reverted the module registration and CollectMigrations logic back to their former state. I also added support for "DROP TABLE" command. Feedback appreciated.

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