sqlx
sqlx copied to clipboard
feat: add migration_table name option
Add migration_table option to enable sharing a database among multiple apps/clients. It is the responsibility of the user to ensure database permissions for the database role performing the migration is not able affect other co-hosted apps/clients done in separate migration "partitions".
- fixes #1835, fixes #1698, fixes #1356, fixes #946
Tested locally with sqlite by cargo install the cli with --git flag for this branch.
Does this seem like a reasonable approach to quickly satisfy the needs outlined in the use cases before I do more things like add tests?
If more migration options, or some more refactoring is needed later, it may be warranted to look at refactoring the relationship between the migration and the database connection. They seem too tightly bound when the database connection is required to know about the migration table, _sqlx_migrations, when it may be more reasonable to just have a function to ask if a table exists, select from it, etc. -- currently the migration table name is hard-coded in every database connection implementation, e.g., postgres:
https://github.com/launchbadge/sqlx/blob/d0fbe7feff4b3a200cf0453417d5e53bd011643a/sqlx-postgres/src/migrate.rs#L90-L111
Hey @kriswuollett any updates on this? I would like to see this merged too.
@abonander, are you willing to work with an externally contributed PR to implement a migration table name option? I assume I'd need to add in schema/table name validation for the new option. Is there some other approach that would be preferred to the rather direct way I implemented the PR?
+1 @abonander is there any interest on getting @kriswuollett's change reviewed and merged, or potentially helping define what you'd prefer to see? Or is this a non-priority at the moment for launchbadge (which is fine too)?
@kriswuollett I'm so sorry. Once a PR gets pushed onto the second page I tend to forget about it.
I'm inclined to say that this should be a property that's specified at the crate level since that seems to be the most common use-case. --migration-table/.set_migration_table() seems like it'd be too easy to forget.
I'm thinking maybe we have a configuration file that lives in the migrations directory itself which both sqlx-cli and migrate!() can read automatically.
I'm thinking one of a few options (both dotfiles so they're sorted separately from migrations):
migrations/.table_name- Simple text file containing just the table name
- Fast to read in
migrate!()(important since proc macros compile in debug mode by default)
migrations/.config.toml- Extensible
- More complex to parse
sqlx.toml(at the crate root)- A single, global configuration file per crate
- Already planning on adding this to configure the query macros
I think I'm kinda leaning towards sqlx.toml but migrations/.table_name would be really quick to add.
What would work best for you?
I'd be in favour of sqlx.toml
Closing to clear out the PR queue. If you have time to come back to this, please address my previous comment.