[Django] #36161: Deletion in reverse data migration fails with a chain of at least 3 foreign keys

[Django]

#36161: Deletion in reverse data migration fails with a chain of at least 3 foreign
keys
-----------------------------+--------------------------------------
Reporter: Enrico Zini | Type: Bug
Status: new | Component: Migrations
Version: 4.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
-----------------------------+--------------------------------------
We had a data migration that created an object, and deleted it on its
reverse side. Reversing the migration failed with an error like:
{{{
ValueError: Cannot query "Modelname object (1)": Must be "Modelname"
instance.
}}}

Similar code worked on a migration earlier in the migration history, and
bisecting which of the steps between them introduced the issue we
pinpointed it to something that looked completely unrelated.

This was a nasty issue to debug.

I put together a reproducer at https://github.com/spanezz/django-reverse-
migration-issue and Colin Watson came up with a workaround. We are not in
a position to be able to go as far as to propose a fix.

This looks very much like the problems encountered in these two
stackoverflow issues, still without a solution:

* https://stackoverflow.com/questions/69836002/historical-model-does-not-
allow-deletion-in-reverting-django-data-migration
* https://stackoverflow.com/questions/75249322/django-cant-delete-rows-
when-migrating-backwards

It could also be related to https://code.djangoproject.com/ticket/27737

When building the migration state, various methods in `ProjectState`
sometimes choose to delay rendering of relationships for non-relational
fields (apparently to improve migration performance).  This causes
`ProjectState._find_reload_model` to use `get_related_models_tuples`
rather than `get_related_models_recursive` to determine which other models
need to be reloaded, which returns only models that are one relation step
away from the model being changed.  `_find_reload_model` then follows one
more step (under the comment "For all direct related models recursively
get all related models"; note that "recursively" is not true if `delay` is
set), but stops there.  This means that for models two relation steps
away, the migration state ends up with a type object as returned by
`apps.get_model`, while the foreign key information of a related model
(three relation steps from the model changed in the migration) contains an
equivalent, but different type object, which was instantiated in a
previous migration step.

While the two type objects point to the same model, they are different
objects and their `id()` values differ. The problem surfaces a lot lower
in the execution stack in
`django.db.models.query_utils.check_rel_lookup_compatibility`, invoked
indirectly by model deletion code, which tries to enforce that a model is
the same as the target of a foreign key:

{{{
  def check_rel_lookup_compatibility(model, target_opts, field):
      """
      Check that self.model is compatible with target_opts. Compatibility
      is OK if:
        1) model and opts match (where proxy inheritance is removed)
        2) model is parent of opts' model or the other way around
      """

      def check(opts):
          # Uncomment this to get a breakpoint when the mismatch happens:
          # if model._meta.label == opts.label and id(
          #     model._meta.concrete_model
          # ) != id(opts.concrete_model):
          #     breakpoint()
          return (
              # This would be a work-around, but not a fix:
              # (model._meta.app_label, model._meta.object_name)
              # == (opts.app_label, opts.object_name)
              model._meta.concrete_model == opts.concrete_model
              or opts.concrete_model in model._meta.get_parent_list()
              or model in opts.get_parent_list()
          )
}}}

The workaround that we are currently using (see https://github.com/spanezz
/django-reverse-migration-issue/blob/main/db/workaround.py and
https://github.com/spanezz/django-reverse-migration-
issue/blob/main/db/migrations/0002_alter_models.py#L34 ) involves
introducing a migration operation that forces a reload of the affected
model.

This situation hints at some caching issue in the migration infrastructure
which we've been unable to follow up, but which surfaces in real world
scenarios and is extremely difficult to identify and handle when it does.
--
Ticket URL: <https://code.djangoproject.com/ticket/36161>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36161: Deletion in reverse data migration fails with a chain of at least 3 foreign
keys
-----------------------------+--------------------------------------

Reporter: Enrico Zini | Owner: (none)

Type: Bug | Status: new
Component: Migrations | Version: 4.2

Severity: Normal | Resolution:

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 Colin Watson):

* cc: Colin Watson (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/36161#comment:1>

[Django]

#36161: Deletion in reverse data migration fails with a chain of at least 3 foreign
keys
-----------------------------+--------------------------------------
Reporter: Enrico Zini | Owner: (none)
Type: Bug | Status: new
Component: Migrations | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+--------------------------------------

Description changed by Enrico Zini:

Old description:

New description:

scenarios (see https://salsa.debian.org/freexian-
team/debusine/-/merge_requests/1588#note_577725 ) and is extremely

difficult to identify and handle when it does.

--

--
Ticket URL: <https://code.djangoproject.com/ticket/36161#comment:2>

[Django]

#36161: Deletion in reverse data migration fails with a chain of at least 3 foreign
keys
-----------------------------+--------------------------------------
Reporter: Enrico Zini | Owner: (none)
Type: Bug | Status: new
Component: Migrations | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+--------------------------------------

scenarios and is extremely difficult to identify and handle when it does.

--
Comment (by cjwatson):

While we encountered this on 4.2, I also checked changes to the migration
logic from 4.2 to current, and nothing seemed at all relevant.
--
Ticket URL: <https://code.djangoproject.com/ticket/36161#comment:3>

[Django]

#36161: Deletion in reverse data migration fails with a chain of at least 3 foreign
keys
-----------------------------+--------------------------------------
Reporter: Enrico Zini | Owner: (none)
Type: Bug | Status: new
Component: Migrations | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+--------------------------------------

Description changed by cjwatson:

> in real world scenarios and is extremely difficult to identify and handle

scenarios (see https://salsa.debian.org/freexian-
team/debusine/-/merge_requests/1588#note_577725) and is extremely

difficult to identify and handle when it does.

--

--
Ticket URL: <https://code.djangoproject.com/ticket/36161#comment:4>

[Django]

#36161: Deletion in reverse data migration fails with a chain of at least 3 foreign
keys
-----------------------------+------------------------------------

Reporter: Enrico Zini | Owner: (none)
Type: Bug | Status: new
Component: Migrations | Version: 4.2
Severity: Normal | Resolution:

Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+------------------------------------

Changes (by Sarah Boyce):

* stage: Unreviewed => Accepted

Comment:

Thank you for the sample project! Replicated the error on main
--
Ticket URL: <https://code.djangoproject.com/ticket/36161#comment:5>

[Django]

#36161: Deletion in reverse data migration fails with a chain of at least 3 foreign
keys

-----------------------------+--------------------------------------
Reporter: Enrico Zini | Owner: zaryab ali
Type: Bug | Status: assigned

Component: Migrations | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-----------------------------+--------------------------------------
Changes (by zaryab ali):

* owner: (none) => zaryab ali
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36161#comment:6>

[Django]

#36161: Deletion in reverse data migration fails with a chain of at least 3 foreign
keys

-----------------------------+------------------------------------
Reporter: Enrico Zini | Owner: (none)
Type: Bug | Status: new

Component: Migrations | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+------------------------------------

Changes (by zaryab ali):

* owner: zaryab ali => (none)
* status: assigned => new

--
Ticket URL: <https://code.djangoproject.com/ticket/36161#comment:7>

[Django]

#36161: Deletion in reverse data migration fails with a chain of at least 3 foreign
keys

-----------------------------+-------------------------------------

Reporter: Enrico Zini | Owner: (none)

Type: Bug | Status: closed
Component: Migrations | Version: 4.2
Severity: Normal | Resolution: duplicate

Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-----------------------------+-------------------------------------
Changes (by Simon Charette):

* resolution: => duplicate
* status: new => closed

Comment:

I'm pretty confident this is a duplicate of #33586 (reverse migration,
cascade deletion, corrupted model cache) which will unfortunately be very
hard to solve the current model invalidation optimization strategy didn't
account for backward migration.

I guess a strategy could be to systematically clear the model cache when
applying migration in reverse order but that will cost at a significant
cost in performance unless #29898 is implemented.

I'm going to close this ticket as a duplicate to consolidate efforts and
discussions on this front given #33586 predates this ticket and has a bit
more discussions on the topic.
--
Ticket URL: <https://code.djangoproject.com/ticket/36161#comment:8>