migrate crashes when renaming a multi-table inheritance base model

[Christopher Neugebauer]

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

[Tim Graham]

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

[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 (app, model_name), model in state.models.items():
            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)

[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.

[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."

[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.

[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?

[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.

[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.

[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.

[Markus Holtermann]

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

[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.

[Matthijs Kooijman]

This bug seems to also apply to proxy subclasses. E.g. if you start with:

from django.db import models

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

class SubA(BrokenBase):
    class Meta:
        proxy = True

And then go to the same steps as described in the initial post, you will get the same error as shown in the initial posted (tested with Django 2.0.1). I also tested with an abstract superclass, but that works as expected (probably because an abstract class is not involved in any migrations).

[Aaron Riedener]

@Markus Holtermann
I managed to fix the issue for my very specific case using a modified version of your draft patch.
I did not need to modify the base.py and state.py nor did i need to add the new class "AlterModelBases"

Can you tell me what needs to be improved in your opinion to make it ready to be implemented on the django master?

What is the restirction that it does not work with sqlite?

I know its currently an ugly hack - but here you find a version of a BaseModel name change from SalesContract to SalesDocument
​https://github.com/scaphilo/koalixcrm/blob/modifications_20171110/koalixcrm/crm/migrations/0015_auto_20180111_2043.py

Id really like to support you to fix that issue as soon as possible.
I think the automated database-migration is one of the key features of django and it is important to
fix such issues as soon as possible

[Nuwan Goonasekera]

Based on the patch proposed by Markus Holtermann, I've created a migration operation that extends the existing RenameModel migration, and it seems to be working as expected. Sharing in case someone else runs into the same issue. I've only tested this with Django 2.1.7.

from django.db import migrations, models

class RenameModelAndBaseOperation(migrations.RenameModel):

    def __init__(self, old_name, new_name):
        super(RenameModelAndBaseOperation, self).__init__(old_name, new_name)

    def state_forwards(self, app_label, state):
        old_remote_model = '%s.%s' % (app_label, self.old_name_lower)
        new_remote_model = '%s.%s' % (app_label, self.new_name_lower)
        to_reload = []
        # change all bases affected by rename
        for (model_app_label, model_name), model_state in state.models.items():
            if old_remote_model in model_state.bases:
                new_bases_tuple = tuple(
                    new_remote_model if base == old_remote_model
                    else base
                    for base in model_state.bases)
                state.models[model_app_label, model_name].bases = new_bases_tuple
                to_reload.append((model_app_label, model_name))
        super(RenameModelAndBaseOperation, self).state_forwards(app_label, state)
        state.reload_models(to_reload, delay=True)

class Migration(migrations.Migration):

    operations = [
        RenameModelAndBaseOperation(
            old_name='Cloud',
            new_name='CloudRenamed',
        ),
    ]

[Mariusz Felisiak]

Adding migrations.AlterModelBases() to a migration operations from ​patch proposed for #23521 fix this issue for me, e.g.

    operations = [
        migrations.AlterModelBases('suba', (models.Model,)),
        migrations.RenameModel(
            old_name='Base',
            new_name='BrokenBase',
        ),
        migrations.RenameField(
            model_name='suba',
            old_name='base_ptr',
            new_name='brokenbase_ptr',
        ),
    ]

Marking as a duplicate of #23521.