-
-
Notifications
You must be signed in to change notification settings - Fork 557
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
[PIP] Schema compatibility check #1122
Comments
Hi! I was thinking recently about a similar thing. For a while now Django has a feature: whenever you change your model, you can simply run its CLI
All this is done without any connection to a database. Django does not run the migrations within a transaction to see what happens. It does not try to compare the actual tables with anything. Just an example (self-explanatory, I hope). Let's say I have a simple model class MyModel(models.Model):
name = models.CharField(max_length=10) After running class Migration(migrations.Migration):
initial = True
dependencies = [
]
operations = [
migrations.CreateModel(
name='MyModel',
fields=[
('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('name', models.CharField(max_length=10)),
],
),
] Now, if I change the model by adding a new field, like so: class MyModel(models.Model):
name = models.CharField(max_length=10)
age = models.IntegerField(default=1) and launch class Migration(migrations.Migration):
dependencies = [
('<app name here>', '0001_initial'),
]
operations = [
migrations.AddField(
model_name='mymodel',
name='age',
field=models.IntegerField(default=1),
),
] This is not only an incredibly useful feature, saving both time and keeping developer from making mistakes. It can also be used to validate the migrations against the existing entities (models). Exactly what you described earlier. I believe SeaORM can perform the same trick with some changes. Currently all migrations use Instead, Statements themselves (I mean things like Doing that will both make it possible to perform the compatibility check and later to automatically generate new migrations according to the changes made to the entities. All this could even be done as a separate library, but too low-level |
Thank you for you input, but it looks like a separate proposal indeed. |
Let me add an other perspective to this. |
I believe the first migration cannot fail in this case, since SeaORM maintains a special table called But you've raised a good point.
The migrations should not mutate. This is why I find a bit weird all current examples of migrations both in the docs and this repo, which look like these: manager
.create_table(
sea_query::Table::create()
.table(Post::Table)
.if_not_exists()
.col(
ColumnDef::new(Post::Id)
.integer()
.not_null()
.auto_increment()
.primary_key()
)
.col(ColumnDef::new(Post::Title).string().not_null())
.col(ColumnDef::new(Post::Text).string().not_null())
.to_owned()
) or manager
.drop_table(
sea_query::Table::drop()
.table(Post::Table)
.to_owned()
) All such examples always rely on using the entity structs directly (as in I would have gone as far as to require explicitly to have all column and table names within migrations as strings, perhaps even manager
.create_table(
sea_query::Table::create().table(TableRef::Table(Alias::new("my_table_name").into_iden())) It would be great to have something like this instead: manager
.create_table(
sea_query::Table::create(("schema", "my_table_name"))
.if_not_exists()
.col(
ColumnDef::new("id")
.integer()
.not_null()
.auto_increment()
.primary_key()
) |
Maybe I didn't make that case clear enough. Yes the migration will run fine on a system where the first migration was already executed, but if that was not the case before, it will fail. Currently I help myself with checks in the migration steps to look up whether something already exists and not do the change if that is the case. But this does only step around some issues. |
No pun intended, PIP stands for "Proposal & Implementation Plan".
Motivation
To allow an application to check that all Entities are compatible with the current database schema, on CI and/or on application startup.
Architecture
We need to introduce a standardized way to define all entities, and I imagine:
And as such, we can have the following API:
Mode of operation
Under the hood, it will discover the entire schema from the database (just like codegen) and compare it against EntitySchema.
The tricky part is that these two schema will not be exactly the same, so we should be somewhat tolerant to permutations.
The live schema can be a superset of Entity schema, i.e. there can be more tables and columns. So we should start from EntitySchema and check that 1) column exist and 2) the types are inter-operable (i.e. DateTime can work with time::DateTime or chrono::DateTime, to complicate things a bit, numeric can also work with both Decimal and BigDecimal)
The MVP don't have to check absolutely everything, but just some sanity checks in CI would be better than nothing.
Since this depends on SeaSchema, we should place this implementation in
sea-orm-migration
.And we need the support of the
DeriveEntityCollection
macro as well as codegen to generate the Entities enum in the first place.A bonus would be that we can return all errors instead of failing immediately.
The text was updated successfully, but these errors were encountered: