Removing Model bases from migration state

Recently, I had to clean up some poor modeling decision-making I made about a decade ago. I chose to use multi-table inheritance for some models and now wanted to remove it. However, when trying to run python manage.py migrate or python manage.py sqlmigrate I faced the error:

django.core.exceptions.FieldError: Local field 'enabled' in class 'ContributorCriteria' clashes with field of the same name from base class 'Criteria'.

What follows is the setup of the project, what needs to change, the above error and how to solve it.

Project setup

Let’s assume our project has the following models:

from django.db import models

class Criteria(models.Model):
    """
    Base Criteria model to identify users who need to be
    alerted based on specific data
    """
    tenant = models.ForeignKey("other_app.Tenant", on_delete=models.CASCADE)
    enabled = models.BooleanField()

class MaintainerCriteria(Criteria):
    """Criteria specifically for maintainers"""
    criteria = models.OneToOneField(Criteria, related_name="maintainer_criteria", parent_link=True, on_delete=models.CASCADE)
    reviews_greater_than = models.IntegerField()
    reviews_less_than = models.IntegerField()

class ContributorCriteria(Criteria):
    """Criteria specifically for contributors"""
    criteria = models.OneToOneField(Criteria, related_name="contributor_criteria", parent_link=True, on_delete=models.CASCADE)
    commits_greater_than = models.IntegerField()
    commits_less_than = models.IntegerField()

Across the organizations using the application, there are two types of criteria that operate the same, but on different sets of data. The above solution can work nicely in that a Criteria queryset can be created and operated on regardless of the other relations. It’s also nice that we have a clean ForeignKey from other models to Criteria. This can make tracking when all criteria are used pretty easy.

Now what if we wanted to add a name to each of these criteria and make it unique for the tenant.

Unfortunately, we can’t.


class ContributorCriteria(Criteria):
    """Criteria specifically for contributors"""
    class Meta:
        constraints = [models.UniqueConstraint(fields=("tenant", "name"))]
        
    criteria = models.OneToOneField(Criteria, related_name="contributor_criteria", parent_link=True, on_delete=models.CASCADE)
    name = models.CharField()

The above model isn’t valid for the migration system. You’ll get an error about how tenant isn’t a field in the ContributorCriteria model.

The choice I decided to make was to remove this multi-table inheritance1.

This would mean using GenericForeignKey’s (GFKs) wherever I had ForeignKey(Criteria) before. That doesn’t excite me, as I prefer to avoid GFKs and likely chose the original model design to avoid them all those years ago. But frankly, I got a lot of mileage out of this model design. So perhaps I shouldn’t be too hard on myself.

Removing multi-table inheritance

Let’s start with where this all should end up. Below are the models that I would like to use.

from django.db import models

class Criteria(models.Model):
    """
    Base Criteria model to identify users who need to be
    alerted based on specific data
    """
    class Meta:
        abstract = True
        
    tenant = models.ForeignKey("other_app.Tenant", on_delete=models.CASCADE)
    enabled = models.BooleanField()
    name = models.CharField()

class MaintainerCriteria(Criteria):
    """Criteria specifically for maintainers"""
    class Meta:
        # The unique constraints need unique names, so I defined them here
        constraints = [
            models.UniqueConstraint(fields=("tenant", "name"), name="unq_maintainer_criteria"),
        ]
        
    reviews_greater_than = models.IntegerField()
    reviews_less_than = models.IntegerField()

class ContributorCriteria(Criteria):
    """Criteria specifically for contributors"""
    class Meta:
        constraints = [
            models.UniqueConstraint(fields=("tenant", "name"), name="unq_contributor_criteria"),
        ]
        
    commits_greater_than = models.IntegerField()
    commits_less_than = models.IntegerField()

Implementation plan

I would also like all the data from the original Criteria table to be copied into the new fields on ContributorCriteria and MaintainerCriteria. I would also like this to minimize downtime on the product application as much as possible. To achieve that, the implementation plan would be something like:

  1. Create a new abstract, BaseCriteria model for criteria with nullable fields
  2. Replace Criteria with BaseCriteria as parent class for ContributorCriteria and MaintainerCriteria; change ContributorCriteria.criteria and MaintainerCriteria.criteria to explicitly set primary key
  3. Create migrations, python manage.py makemigrations myapp --name add_basecriteria_fields
  4. Create data migration with operation to copy data from Criteria to ContributorCriteria and MaintainerCriteria as necessary, python manage.py makemigrations myapp --empty --name populate_basecriteria_fields
  5. Update the application code to only rely on ContributorCriteria and MaintainerCriteria
  6. Change ContributorCriteria.criteria and MaintainerCriteria.criteria to a primary key, integer field2
  7. Change BaseCriteria model to not have nullable fields
  8. Create migrations, python manage.py makemigrations myapp --name remove_criteria_relation

That is quite a few steps for something that feels small. However, we’re just going to focus on the first two steps because that’s where the problem is.

Let’s assume we have our original models all set up. The migration operations for that would be:

operations = [
    migrations.CreateModel(
        name="Criteria",
        fields=[
            (
                "id",
                models.BigAutoField(
                    auto_created=True,
                    primary_key=True,
                    serialize=False,
                    verbose_name="ID",
                ),
            ),
            ("enabled", models.BooleanField()),
            (
                "tenant",
                models.ForeignKey(
                    on_delete=django.db.models.deletion.CASCADE, to="myapp.tenant"
                ),
            ),
        ],
    ),
    migrations.CreateModel(
        name="ContributorCriteria",
        fields=[
            (
                "criteria",
                models.OneToOneField(
                    on_delete=django.db.models.deletion.CASCADE,
                    parent_link=True,
                    primary_key=True,
                    related_name="contributor_criteria",
                    serialize=False,
                    to="myapp.criteria",
                ),
            ),
            ("commits_greater_than", models.IntegerField()),
            ("commits_less_than", models.IntegerField()),
        ],
        bases=("myapp.criteria",),
    ),
    migrations.CreateModel(
        name="MaintainerCriteria",
        fields=[
            (
                "criteria",
                models.OneToOneField(
                    on_delete=django.db.models.deletion.CASCADE,
                    parent_link=True,
                    primary_key=True,
                    related_name="maintainer_criteria",
                    serialize=False,
                    to="myapp.criteria",
                ),
            ),
            ("reviews_greater_than", models.IntegerField()),
            ("reviews_less_than", models.IntegerField()),
        ],
        bases=("myapp.criteria",),
    ),
]

They key part that we will come back to is the bases=("myapp.criteria",) part of the migrations.CreateModel() operations. That’s where the problem will come from.

If we do step 1, “Create new abstract, BaseCriteria model for criteria with nullable fields,” and step 2, “Replace Criteria with BaseCriteria as parent class for ContributorCriteria and MaintainerCriteria, change ContributorCriteria.criteria and MaintainerCriteria.criteria to explicitly set primary key,” our models will be:

class Criteria(models.Model):
    """
    Base Criteria model to identify users who need to be
    alerted based on specific data
    """
    tenant = models.ForeignKey("other_app.Tenant", on_delete=models.CASCADE)
    enabled = models.BooleanField()


class BaseCriteria(models.Model):
    """
    Base Criteria model to identify users who need to be
    alerted based on specific data
    """
    class Meta:
        abstract = True

    tenant = models.ForeignKey(Tenant, on_delete=models.CASCADE, null=True)
    enabled = models.BooleanField(null=True)


class MaintainerCriteria(BaseCriteria):
    """Criteria specifically for maintainers"""
    criteria = models.OneToOneField(Criteria, primary_key=True, parent_link=True, related_name="maintainer_criteria", on_delete=models.CASCADE)
    reviews_greater_than = models.IntegerField()
    reviews_less_than = models.IntegerField()


class ContributorCriteria(BaseCriteria):
    """Criteria specifically for contributors"""
    criteria = models.OneToOneField(Criteria, primary_key=True, parent_link=True, related_name="contributor_criteria", on_delete=models.CASCADE)
    commits_greater_than = models.IntegerField()
    commits_less_than = models.IntegerField()

Now if we do step 3 and create the migrations, we get a new migration file with the following operations:

operations = [
    migrations.AddField(
        model_name="contributorcriteria",
        name="enabled",
        field=models.BooleanField(null=True),
    ),
    migrations.AddField(
        model_name="contributorcriteria",
        name="tenant",
        field=models.ForeignKey(
            null=True,
            on_delete=django.db.models.deletion.CASCADE,
            to="myapp.tenant",
        ),
    ),
    migrations.AddField(
        model_name="maintainercriteria",
        name="enabled",
        field=models.BooleanField(null=True),
    ),
    migrations.AddField(
        model_name="maintainercriteria",
        name="tenant",
        field=models.ForeignKey(
            null=True,
            on_delete=django.db.models.deletion.CASCADE,
            to="myapp.tenant",
        ),
    ),
]

This looks reasonable; we’re adding new, nullable columns to our child models. BUT! Remember that Django migrations have their own state for our models. If the migration’s model state is representative of our application’s model state, where is the change that indicates ContributorCriteria and MaintainerCriteria no longer inherit from Criteria? Exactly, there isn’t one. And Django won’t catch it for you until you try to generate the SQL for this migration with either python manage.py migrate or python manage.py sqlmigrate myapp <migration>.

Being proper, defensive engineers, let’s try to look at the SQL for this migration with python manage.py sqlmigrate myapp <migration>. You’ll see an error similar to:

django.core.exceptions.FieldError: Local field 'enabled' in class 'ContributorCriteria' clashes with field of the same name from base class 'Criteria'.

This makes total sense if we understand that the migration’s model state still believes that ContributorCriteria inherits from Criteria. Removing Criteria as a model in our migration state isn’t a valid option since we want to keep the underlying table around to copy data to and from. Faking a migration that removes the model could work, but it’s really, really hacky and will likely cause a problem.

What we really want is a way to keep the Criteria model and table around but have our migration model state’s ContributorCriteria and MaintainerCriteria no longer inherit from it.

The first step to doing that is to understand how that relationship is defined in our migrations. Remember that bases=("myapp.criteria",) part of CreateModel() from earlier?

Exactly. That’s the part of the migration model state that we need to manipulate. Unfortunately, there’s no existing migration operation to manipulate that. AlterModelTable and AlterModelOptions are the closest options, and neither of them supports changing bases. In fact, if you search that page of the documentation, bases is only mentioned in CreateModel.

Welp, thankfully, a hero named Andrii on the interwebs did the work of creating a migration operation class that would allow changing bases. From that code, we can create a slightly more reusable operation:

from django.db.migrations.operations.models import ModelOptionOperation

class SetModelBasesOptionOperation(ModelOptionOperation):
    """
    A migration operation that updates the bases of a model.
    This can be used to separate a model from its parent. Specifically
    when multi-table inheritance is used.
    """
    def __init__(self, name, bases):
        super().__init__(name)
        self.bases = bases

    def deconstruct(self):
        return (self.__class__.__qualname__, [], {"bases": self.bases})

    def state_forwards(self, app_label, state):
        model_state = state.models[app_label, self.name_lower]
        model_state.bases = self.bases
        state.reload_model(app_label, self.name_lower, delay=True)

    def database_forwards(self, app_label, schema_editor, from_state, to_state):
        pass

    def database_backwards(self, app_label, schema_editor, from_state, to_state):
        pass

    def describe(self):
        return "Update bases of the model %s" % self.name

    @property
    def migration_name_fragment(self):
        return "set_%s_bases" % self.name_lower

In our prior migration, if we start the operations with a SetModelBasesOptionOperation, the migration will work as we want.

from django.db import models
from myapp.operations import SetModelBasesOptionOperation

operations = [
    SetModelBasesOptionOperation("ContributorCriteria", (models.Model, )),
    SetModelBasesOptionOperation("MaintainerCriteria", (models.Model, )),
    migrations.AddField(
        model_name="contributorcriteria",
        name="enabled",
        field=models.BooleanField(null=True),
    ),
    migrations.AddField(
        model_name="contributorcriteria",
        name="tenant",
        field=models.ForeignKey(
            null=True,
            on_delete=django.db.models.deletion.CASCADE,
            to="myapp.tenant",
        ),
    ),
    migrations.AddField(
        model_name="maintainercriteria",
        name="enabled",
        field=models.BooleanField(null=True),
    ),
    migrations.AddField(
        model_name="maintainercriteria",
        name="tenant",
        field=models.ForeignKey(
            null=True,
            on_delete=django.db.models.deletion.CASCADE,
            to="myapp.tenant",
        ),
    ),
]

Now running python manage.py sqlmigrate myapp <migration> will generate the following SQL:

BEGIN;
--
-- Update bases of the model ContributorCriteria
--
-- (no-op)
--
-- Update bases of the model MaintainerCriteria
--
-- (no-op)
--
-- Add field enabled to contributorcriteria
--
ALTER TABLE "myapp_contributorcriteria" ADD COLUMN "enabled" bool NULL;
--
-- Add field tenant to contributorcriteria
--
ALTER TABLE "myapp_contributorcriteria" ADD COLUMN "tenant_id" bigint NULL REFERENCES "myapp_tenant" ("id") DEFERRABLE INITIALLY DEFERRED;
--
-- Add field enabled to maintainercriteria
--
ALTER TABLE "myapp_maintainercriteria" ADD COLUMN "enabled" bool NULL;
--
-- Add field tenant to maintainercriteria
--
ALTER TABLE "myapp_maintainercriteria" ADD COLUMN "tenant_id" bigint NULL REFERENCES "myapp_tenant" ("id") DEFERRABLE INITIALLY DEFERRED;
CREATE INDEX "myapp_contributorcriteria_tenant_id_1863210b" ON "myapp_contributorcriteria" ("tenant_id");
CREATE INDEX "myapp_maintainercriteria_tenant_id_73eb13a6" ON "myapp_maintainercriteria" ("tenant_id");
COMMIT;

At this point, we’re now free of on Criteria and can move forward unencumbered by multi-table inheritance!

I plan on submitting this as a ticket, so hopefully in the future this change will be automatically detected by makemigrations.

If you have thoughts, comments or questions, please let me know. You can find me on the Fediverse, Django Discord server or via email.

  1. Did you know that models that inherit from non-abstract models can’t use bulk_create? That factored in here as well. 

  2. criteria = models.IntegerField(Criteria, primary_key=True, db_column="criteria_id") specifically, then rename criteria to criteria_id