[Django] #32787: ContentTypes are created instead of renamed when using SeparateDatabaseAndState
Django
SeparateDatabaseAndState
------------------------------------------------+------------------------
Reporter: David Seddon | Owner: nobody
Type: Bug | Status: new
Component: contrib.contenttypes | Version: 3.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
When renaming a model using `SeparateDatabaseAndState`, rather than
renaming the existing `ContentType`, a new one is created. This breaks any
existing generic foreign keys pointing to the model.
**Cause**
When a model is renamed in the standard way, the content types framework
uses the pre_migrate signal to rename the `ContentType` model, ensuring
that existing generic foreign keys continue to work. Code here:
https://github.com/django/django/blob/main/django/contrib/contenttypes/management/__init__.py#L79
If `SeparateDatabaseAndState` is used to control the migration, the
`isinstance` check will miss the fact that there is a `RenameModel`
operation, as it just checks the containing `SeparateDatabaseAndState`
operation. It therefore fails to inject a `RenameContentType` operation.
Later in the migration process, in
[https://github.com/django/django/blob/main/django/contrib/contenttypes/management/__init__.py#L105
create_contenttypes] will be triggered by the `post_migrate` signal. Since
there is now a missing ContentType for the new model name, it will create
it.
**Discussion**
While `SeparateDatabaseAndState` is of course meant to be a low level
operation, it's difficult to anticipate this behaviour. Since the
ContentTypes table is being changed by in any event, it would seem to me
preferable to do it in a backward-compatible way.
Alternatively, emitting a warning might be worthwhile: the developer could
then manually add a `RenameContentType` to the migration file once they're
aware of the issue.
I think this could be solved by adjusting
[https://github.com/django/django/blob/main/django/contrib/contenttypes/management/__init__.py#L105
the check] so that it drills down to the `database_operations` if the
operation is a `SeparateDatabaseAndState` instance.
--
Ticket URL: <https://code.djangoproject.com/ticket/32787>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
SeparateDatabaseAndState
Reporter: David Seddon | Owner: nobody
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* status: new => closed
* resolution: => wontfix
* component: contrib.contenttypes => Database layer (models, ORM)
* type: Bug => New feature
Comment:
`SeparateDatabaseAndState` is ''"a highly specialized operation that lets
you mix and match the database (schema-changing) and state (autodetector-
powering) aspects of operations."'' (see
[https://docs.djangoproject.com/en/3.2/ref/migration-
operations/#django.db.migrations.operations.SeparateDatabaseAndState
docs]). If you want to use it to rename models you need to also update
content types in the `database_operations`. Django will not analyze raw
SQL queries, there is also now way to provide a sensible warning.
--
Ticket URL: <https://code.djangoproject.com/ticket/32787#comment:1>
Django
SeparateDatabaseAndState
Reporter: David Seddon | Owner: nobody
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution: wontfix
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by David Seddon):
Thanks for the reply!
> If you want to use it to rename models you need to also update content
types in the database_operations.
That's what we recommend to do now at my workplace, but it was hard-won
knowledge involving some forensic work to get to the bottom of what was
going on.
I would argue is that as it stands, the content types hooks are a bit of a
footgun even for experienced developers who need to use
`SeparateDatabaseAndState`. My expectation would have been that using
`SeparateDatabaseAndState` suppressed all database actions, but in fact it
does perform a database action, just the wrong one.
I wonder if an alternative mitigation would be to output to the console
what contenttypes is doing in the `pre_migrate`/`post_migrate` steps, just
so developers are aware this is happening.
Anyway I do appreciate this is probably a pretty niche issue. Let me know
if you change your mind, I'd be happy to put together a pull request.
--
Ticket URL: <https://code.djangoproject.com/ticket/32787#comment:2>