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:
- Create a new abstract,
BaseCriteria
model for criteria with nullable fields - Replace
Criteria
withBaseCriteria
as parent class forContributorCriteria
andMaintainerCriteria
; changeContributorCriteria.criteria
andMaintainerCriteria.criteria
to explicitly set primary key - Create migrations,
python manage.py makemigrations myapp --name add_basecriteria_fields
- Create data migration with operation to copy data from
Criteria
toContributorCriteria
andMaintainerCriteria
as necessary,python manage.py makemigrations myapp --empty --name populate_basecriteria_fields
- Update the application code to only rely on
ContributorCriteria
andMaintainerCriteria
- Change
ContributorCriteria.criteria
andMaintainerCriteria.criteria
to a primary key, integer field2 - Change
BaseCriteria
model to not have nullable fields - 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.