search

Electron100 / butane

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

Migration Metadata Cannot See the Type System (Migrations v2) #60

Open Electron100 opened 1 year ago

Electron100 commented 1 year ago

One of the marquee features of Butane is the ability to have fully automatic migrations, but I unfortunately made a key architectural error in the design and implementation of this. All of the metadata to support the migration is written out by a proc-macro run during compilation. This has two major downsides:

  1. Since proc macros see only the AST and not the type system, all of the ToSql/FromSql implementations are worthless here, we're essentially maintaining two separate systems of knowledge for how to map between Rust types and SQL types -- one in the type system and the other in the proc macro. This makes butane_type a necessary kludge when also implementing the to/from conversions and requires adding (error-prone) type name recognition to the proc macro each time we add a new core type.
  2. The proc macro runs independently on each #[model], and Cargo does not appear to provide any environment variables with a unique identifier for the build. Thus each .table file is generated independently and there isn't a great way to reap dead ones (that I've identified).

I'd like to solve the first problem and part of the second by changing when the migration metadata happens, at the cost of slightly more work for the user (developer using Butane). Rather than creating the migration metadata (ADB structure) at build time from the #[model] proc macro, the proc macro would just create model implementations capable of generating migration metadata (each model could generate its corresponding ATable). The proc macro would also generate a program: butane_migration_helper.rs designed to be invoked by butane cli. We'd probably need the the user to add this as a [[bin]] to their Cargo.toml. We could automate that with butane cli using toml_edit, but would probably still want to prompt the user before making automatic changes to their project file.

We'd probably also need to introduce a new proc macro where the user has to explicitly list all of their models to fully solve the dead-model problem and make generation of butane_migration_helpers.rs easier otherwise we'd have no canonical point for creating it.

I've begun experimenting with this in https://github.com/Electron100/butane/commits/migration_v2

jayvdb commented 3 months ago

We'd probably also need to introduce a new proc macro where the user has to explicitly list all of their models to fully solve the dead-model problem ..

Yes, I see in your branch you have a butane_database proc-macro which I guess intends to do this. afaics, that could also be done in the current system. That would likely eliminate the need for types.json, as the resolve step would occur in this proc-macro where it knows the set of types that are needed, and it could delete any .table files which it knows are no longer needed. That might still break "IDE+running tests", but only if the running tests are using tables/types which have been removed in the IDE, in which case the developer is expecting the running tests to fail.

Very likely a butane_database proc-macro added to the current system also removes the need for .table files to be written in each proc-macro, and the binaries build wouldnt need to depend on the .table files at all - they could be written out only to keep the existing CLI logic as-is.