Opened 20 months ago

Last modified 3 months ago

#26488 new Bug

migrate crashes when renaming a multi-table inheritance base model

Reported by: Christopher Neugebauer Owned by: nobody
Component: Migrations Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I needed to rename the base class that several models depend on due to a change in naming conventions in my product. It appears as though this is not possible.

I was able to reproduce this in Django 1.9.2 with the following simple case:

Steps to reproduce:

Create a fresh django project, and populate models.py like so:

from django.db import models


class Base(models.Model):
    foo = models.BooleanField()


class SubA(Base):
    bar = models.BooleanField()

Run makemigrations to get an initial migration file.

Replace the contents of models.py with the following:

from django.db import models


class BrokenBase(models.Model):
    foo = models.BooleanField()


class SubA(BrokenBase):
    bar = models.BooleanField()

Run makemigrations, and accept all of the y/N questions. It correctly detects the renames, like so:

Did you rename the oops.Base model to BrokenBase? [y/N] y
Did you rename suba.base_ptr to suba.brokenbase_ptr (a OneToOneField)? [y/N] y

Now run migrate:

django.db.migrations.exceptions.InvalidBasesError: Cannot resolve bases for [<ModelState: 'oops.SubA'>]
This can happen if you are inheriting models from an app with migrations (e.g. contrib.auth)
 in an app with no migrations; see https://docs.djangoproject.com/en/1.9/topics/migrations/#dependencies for more

Change History (12)

comment:1 Changed 20 months ago by Tim Graham

Triage Stage: UnreviewedAccepted

May be related or a duplicate of #23521. Created #26490 for a regression in master which hides the InvalidBasesError behind a RecursionError: maximum recursion depth exceeded

comment:2 Changed 20 months ago by Tim Graham

Summary: migration raises django.db.migrations.exceptions.InvalidBasesError if you rename a base modelmigrate raises InvalidBasesError if you rename a multitable inheritance base model

comment:3 Changed 19 months ago by Simon Philips

I ran into the same issue. It seems that the bases of all child classes are not updated in the state_forwards method of the RenameModel operation.

This custom operation solves it, but there may be a more elegant solution:

class RenameBaseModel(migrations.RenameModel):
    def state_forwards(self, app_label, state):
        full_old_name = ("%s.%s" % (app_label, self.old_name)).lower()
        full_new_name = ("%s.%s" % (app_label, self.new_name)).lower()

        for model in state.models.values():
            if full_old_name in model.bases:
                model.bases = tuple(full_new_name if b == full_old_name else b
                                    for b in model.bases)

        super(RenameBaseModel, self).state_forwards(app_label, state)
Last edited 19 months ago by Simon Philips (previous) (diff)

comment:4 Changed 19 months ago by Simon Philips

It seems the above solution is broken as the bases should be updated after renaming the model and before reloading any other models. Naively calling super().state_forwards() doesn't work.

class RenameBaseModel(migrations.RenameModel):
    def state_forwards(self, app_label, state):
        apps = state.apps
        model = apps.get_model(app_label, self.old_name)
        model._meta.apps = apps
        # Get all of the related objects we need to repoint
        all_related_objects = (
            f for f in model._meta.get_fields(include_hidden=True)
            if f.auto_created and not f.concrete and (not f.hidden or f.many_to_many)
        )
        # Rename the model
        state.models[app_label, self.new_name_lower] = state.models[app_label, self.old_name_lower]
        state.models[app_label, self.new_name_lower].name = self.new_name
        state.remove_model(app_label, self.old_name_lower)
        # Repoint bases, this needs to be done before reloading any model
        full_old_name = "%s.%s" % (app_label, self.old_name_lower)
        full_new_name = "%s.%s" % (app_label, self.new_name_lower)
        for state_model in state.models.values():
            if full_old_name in state_model.bases:
                state_model.bases = tuple(
                    full_new_name if b == full_old_name else b
                    for b in state_model.bases
                )
        # Repoint the FKs and M2Ms pointing to us
        for related_object in all_related_objects:
            if related_object.model is not model:
                # The model being renamed does not participate in this relation
                # directly. Rather, a superclass does.
                continue
            # Use the new related key for self referential related objects.
            if related_object.related_model == model:
                related_key = (app_label, self.new_name_lower)
            else:
                related_key = (
                    related_object.related_model._meta.app_label,
                    related_object.related_model._meta.model_name,
                )
            new_fields = []
            for name, field in state.models[related_key].fields:
                if name == related_object.field.name:
                    field = field.clone()
                    field.remote_field.model = "%s.%s" % (app_label, self.new_name)
                new_fields.append((name, field))
            state.models[related_key].fields = new_fields
            state.reload_model(*related_key)
        state.reload_model(app_label, self.new_name_lower)

Note that the migration that initially creates SubA must be run before this migration. If SubA is in a different app, that means you'll have to update that app's migration and set its run_before attribute.

Last edited 19 months ago by Simon Philips (previous) (diff)

comment:5 Changed 6 months ago by Tim Graham

Summary: migrate raises InvalidBasesError if you rename a multitable inheritance base modelmigrate crashes when renaming a multi-table inheritance base model

As reported in #28243 (closed as duplicate), after ecd625e830ef765d5e60d5db7bc6e3b7ffc76d06, the error is now something like "FieldError: Auto-generated field 'a_ptr' in class 'B' for parent_link to base class 'A' clashes with declared field of the same name."

comment:6 in reply to:  5 Changed 5 months ago by kesterlester

Replying to Tim Graham:

As reported in #28243 (closed as duplicate), after ecd625e830ef765d5e60d5db7bc6e3b7ffc76d06, the error is now something like "FieldError: Auto-generated field 'a_ptr' in class 'B' for parent_link to base class 'A' clashes with declared field of the same name."

Ah - such a relief to find this bug report! ~24 hours ago I hit this problem in my first django project. Being a newbie I presumed I was doing something wrong and have been trying all sorts of things (without success) to allow me to rename a base class without losing all the data that's now in my database. I'll leave the renaming issue alone now and await a patch. I don't feel anywhere near experienced enough yet to try to patch things myself, but maybe later?

In agreement with Tim's comment I see:

jango.core.exceptions.FieldError: Auto-generated field 'content_ptr' in class 'Answer' for parent_link to base class 'Content' clashes with declared field of the same name.

as a result of me trying to rename a multi-table inheritance base class (having no non-automatic fields) from name "Content" to something else, when class Answer derives from Content.

comment:7 Changed 5 months ago by Piranha Phish

Just wanted to confirm that this is affecting me as well. I'm receiving the new error message "… clashes with declared field of the same name" when trying to apply a migration in which a base class of MTI is renamed.

The two versions of the workaround class proposed by Simon don't seem to work for me. Is there any other known workaround?

comment:8 Changed 4 months ago by Christopher Neugebauer

So here's something fun. Here's how you *can* create a model with a base class whose name you can change, and Migrations will be happy.

EXAMPLE DELETED BECAUSE THE NEXT COMMENT SUPERSEDES IT IN A MUCH NICER WAY

You can see that RenamedBaseModel works as expected. It appears as though everything here works, as long as you aren't tracking the base class when migrations first creates the model.

Last edited 4 months ago by Christopher Neugebauer (previous) (diff)

comment:9 Changed 4 months ago by Christopher Neugebauer

So the previous example is a bit evil. Here's a reduced example that *works* in Django 1.11:

Step 0

    class BaseModel(models.Model):
        pass


    class SubModel(BaseModel):
        basemodel_ptr = models.OneToOneField(BaseModel, null=True)

Run makemigrations

Step 1

    class BaseModel(models.Model):
        pass


    class SubModel(BaseModel):
        pass

Run makemigrations

Step 2

    class RenamedBaseModel(models.Model):
        pass


    class SubModel(RenamedBaseModel):
        pass

Run makemigrations

If you start with Step 0, you can successfully migrate at the end of step 2.
If you start with Step 1, migrate breaks, but with a new error:

django.core.exceptions.FieldError: Auto-generated field 'basemodel_ptr' in class 'SubModel' for parent_link to base class 'BaseModel' clashes with declared field of the same name.

comment:10 Changed 3 months ago by Markus Holtermann

When I try to run makemigrations after *step 0* with the code you provided I get the following system check failure:

$ ./manage.py makemigrations         
SystemCheckError: System check identified some issues:

ERRORS:
ticket26488.SubModel.basemodel_ptr: (fields.E007) Primary keys must not have null=True.
        HINT: Set null=False on the field, or remove primary_key=True argument.

Further, neither steps 0-2 nor steps 1-2 work for me. It comes down to django.core.exceptions.FieldError: Auto-generated field 'basemodel_ptr' in class 'SubModel' for parent_link to base class 'BaseModel' clashes with declared field of the same name. both times.

comment:11 Changed 3 months ago by Markus Holtermann

Needs tests: set
Version: 1.9master

So, this is due to the fact that Django doesn't have a way to alter model bases. For whatever reason. I don't have a solution at hand right now. But there are a few other tickets (one being #23521) that suffer from the same issue.

Idea for a solution:

  • add AlterModelBases migration operation
  • extend autodetector to detect model bases changes

comment:12 Changed 3 months ago by Markus Holtermann

Looks like using a AlterModelBases operation wouldn't cut it in the given case as the bases alteration needs to happen as part of the RenameModel operation.

Here is a very early patch including debugging output and some other cruft that wouldn't be part of a proper patch that does not work on SQLite, yet.

Note: See TracTickets for help on using tickets.
Back to Top