Opened 8 hours ago

Last modified 8 hours ago

#36161 new Bug

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

Reported by: Enrico Zini Owned by:
Component: Migrations Version: 4.2
Severity: Normal Keywords:
Cc: Colin Watson Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Colin Watson)

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:

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 (see https://salsa.debian.org/freexian-team/debusine/-/merge_requests/1588#note_577725) and is extremely difficult to identify and handle when it does.

Change History (4)

comment:1 by Colin Watson, 8 hours ago

Cc: Colin Watson added

comment:2 by Enrico Zini, 8 hours ago

Description: modified (diff)

comment:3 by Colin Watson, 8 hours ago

Description: modified (diff)

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.

comment:4 by Colin Watson, 8 hours ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.
Back to Top