Electron100 / butane

An ORM for Rust with a focus on simplicity and on writing Rust, not SQL
Apache License 2.0
94 stars 13 forks source link

collapse deletes custom sql #202

Open jayvdb opened 6 months ago

jayvdb commented 6 months ago

butane collapse runs clear_migrations which deletes all migrations on disk, and then it creates a new migration from None (empty db) to the db as of last_applied_migration.

For any unmigrated migrations, they are lost, however it is possible to run a butane make-migration to create a new migration that will include them all.

The result is any edited <foo>_up.sql and <foo>_down.sql are lost.

I think we agreed at https://github.com/Electron100/butane/issues/91 or an issue of that era that butane would allow users to edit the generated SQL.

As a result, collapse should either

  1. be renamed to regenerate-migrations or something that makes it more clear it is destructive.
  2. have a default mode that merges the on-disk SQL files instead of generating new SQL.
jayvdb commented 6 months ago

django command squashmigrations has a flag --no-optimize, which avoids the optimizer which is enabled by default.

However django has two types of migration operations - declarative schema changes (which are able to be safely optimised), and SQL changes (not optimisable), whereas butane only has SQL changes.

jayvdb commented 6 months ago

... whereas butane only has SQL changes.

Strictly speaking, butane also has the declarative schema changes, but those have been converted to SQL. It would not be difficult to have butane also generate the SQL for those changes at runtime during the migrate / rollback actions being performed, then it is more like django with a separation between declarative & SQL changes. As there could be bugs in the generated SQL (like https://github.com/Electron100/butane/issues/201), it would still be useful for make-migration to have an option to move the descriptive changes into SQL changes, so they can be edited manually.

RobertoMaurizzi commented 6 months ago

Django's approach where you have generic migration descriptions from which SQL is created for each database backend type allows for very nice features, database independence in migrations for one. However, it's "easy" to do that in a dynamic language like Python... I can't see how to do that in Rust other than creating some specialized description "language" (maybe commented SQL?)

jayvdb commented 6 months ago

Ya the Django migration system is great. There are some parts which are not going to be easy (or desirable) to replicate, such as the re-usable apps which each have their own migration series.

But butane can do the same thing with collapsing/squashing migrations. In each migration it stores a .table file for every table, so it can create a "diff" between each migration and create SQL at runtime instead of using the .sql files. e.g. https://github.com/Electron100/butane/blob/master/examples/getting_started/.butane/migrations/20240115_023841384_dbconstraints/Blog.table vs previous https://github.com/Electron100/butane/blob/master/examples/getting_started/.butane/migrations/20201229_171630604_likes/Blog.table

The logic already exists to generate SQL from these differences. So the question is: do we want to do that?

I cant see any downside, as long as it is still possible to also store the per-backend SQL, for those cases where the butane dynamic SQL is buggy.

Note that butane already has a lot of the plumbing for database independence. https://github.com/Electron100/butane/pull/198 adds a few of the missing bits to expose that plumbing to the CLI.

jayvdb commented 6 months ago

Actually, there is a downside. If a user relies on dynamic SQL to create tables/columns, etc, their migration become reliant on butane being stable into the future. This can be quite tricky especially when thinking about rolling back. If butane provides this feature, all future work on butane migration generation needs to behave correctly for all previous versions of butane - i.e. even replicate the same bugs because the users may be depending on the SQL that was generated. By generating only SQL, the user can fix those SQL scripts, check them in, and never need to worry about what future butane might do. tl;dr - even though we could dynamically generate the SQL, providing that feature could slow down the development of other features.

Personally I am not interested in supporting dynamically generating migrations, and also not particularly keen on getting the migration generator to emit perfect SQL, as any serious software development is going to take the time to customise their SQL for the migrations. I am also more interested in using a dedicated piece of software for doing migrations, as I think the butane team doesnt have the capacity to do this. e.g. https://github.com/Electron100/butane/issues/67