-
Notifications
You must be signed in to change notification settings - Fork 13
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
CompositeType.Operation will create current type definition, not historical definition #8
Comments
I've had a look at the django migration code, it seems pretty inflexible. The only way I can think to do this right now without abusing undocumented API's is by extending the Thoughts? (Bonus: this would also prevent the need for manually adding the operation to migrations.) |
That is the same conclusion I came to. That does come with its own set of problems though, as you would have to effectively maintain a fork of the Django migration code, possibly against multiple versions of Django, for this to work. The best approach would be to make Django migrations more flexible, allowing other apps to hook in to the autodetection step, and allowing things other than models to participate in the migration workflow. This is a big task though. |
I think the best outcome would be to add a new command that output operations just for types (even requiring people to copy them into their migrations manually). And then remove the magic-generated operations for a more conventional CreateType, AlterType, DropType set of operations. |
After taking a crack at this, I can see this could be done, but it's a bit too much work for me right now. I think you'd necessarily have to end up re-implementing a lot of existing django migrations code. |
I'm experimenting with a different approach that may or may not yield results: https://github.com/financica/django-postgres-composite-types/tree/dev/compat-with-model My theory is that a composite type is similar to a model; so, it should be possible to make Django think that it is a Model, and hack into Django so that it generates the types and selects them correctly. Success 1: By subclassing CreateModel migration operation into a CreateType operation, I've been able to create composite types without issues. However, the migration generated by makemigrations needs hand editing as there is no way to tell it to use that particular operation instead of the CreateModel operation, which is hardcoded. This is easily fixed in Django. Success 2: I've renamed Fail 1: Models absolutely require a primary key in Django. I have however worked around this by generating a dummy Current blocker: Tests are failing due to the following:
I'm not quite sure how to fix this yet, will keep investigating later. |
|
Looking at all these test failures now. |
The current implementation of
CompositeType.Operation
will create the type as it is when the migration is run, not as the type was when the migration was created. This can lead to problems when changing the type later, and making a migration for it. Consider the following scenario:This will work. However, for someone making a new database from scratch, this will fail. The type will be created with all three fields in the first migration, and the second migration will fail to add the duplicate field.
The creation migration should save the state of the type at the time the migration is created, rather than the time the migration is run.
The text was updated successfully, but these errors were encountered: