[Django] #26488: migration raises django.db.migrations.exceptions.InvalidBasesError if you rename a base model
Django
rename a base model
----------------------------+--------------------
Reporter: chrisjrn | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.9
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------+--------------------
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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26488>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
rename a base model
Reporter: chrisjrn | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.9
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* needs_tests: => 0
* needs_docs: => 0
Comment:
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`
--
Ticket URL: <https://code.djangoproject.com/ticket/26488#comment:1>
Django
base model
----------------------------+------------------------------------
Reporter: chrisjrn | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.9
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
--
Ticket URL: <https://code.djangoproject.com/ticket/26488#comment:2>
Django
base model
----------------------------+------------------------------------
Type: Bug | Status: new
Component: Migrations | Version: 1.9
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by simonphilips):
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:
{{{#!python
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().state_forwards(app_label, state)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26488#comment:3>
Django
base model
----------------------------+------------------------------------
Type: Bug | Status: new
Component: Migrations | Version: 1.9
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by simonphilips):
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.
{{{
#!python
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26488#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Christopher | Owner: nobody
Neugebauer |
Type: Bug | Status: new
Component: Migrations | Version: 1.9
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by 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."
--
Ticket URL: <https://code.djangoproject.com/ticket/26488#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Christopher | Owner: nobody
Neugebauer |
Component: Migrations | Version: 1.9
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by kesterlester):
Replying to [comment:5 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26488#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Christopher | Owner: nobody
Neugebauer |
Component: Migrations | Version: 1.9
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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?
--
Ticket URL: <https://code.djangoproject.com/ticket/26488#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Christopher | Owner: nobody
Neugebauer |
Component: Migrations | Version: 1.9
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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.
1. Create a model `SubModel`, it must derive from `models.Model`.
2. Create a new base class, `BaseModel`, it must derive from
`models.Model`
3. Add: `basemodel_ptr = OneToOneField(BaseModel, null=True)` on
`SubModel`. The field *must* be named like the class name of the new base
class.
4. Automatically create a migration
5. Create a data migration that creates a new `BaseModel(id=submodel.id)`
for each `SubModel` instance. This step is only necessary if you have
created `SubModel` objects. Otherwise, just use an automatic default.
6. Set the base model of `SubModel` to `BaseModel`
7. Automatically create a migration
8. Rename `BaseModel` to `RenamedBaseModel`
9. Automatically create a migration, accepting the two automatic
suggestions
10. Migrate!
At this point, 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26488#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Christopher | Owner: nobody
Neugebauer |
Component: Migrations | Version: 1.9
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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.`
--
Ticket URL: <https://code.djangoproject.com/ticket/26488#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Christopher | Owner: nobody
Neugebauer |
Component: Migrations | Version: 1.9
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26488#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Christopher | Owner: nobody
Neugebauer |
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Markus Holtermann):
* version: 1.9 => master
* needs_tests: 0 => 1
Comment:
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
--
Ticket URL: <https://code.djangoproject.com/ticket/26488#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: Christopher | Owner: nobody
Neugebauer |
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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.
[https://github.com/MarkusH/django/commit/b7bf79ed38bb1a5d730c9b106ef6e5819ebb3a4e
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26488#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: Christopher | Owner: nobody
Neugebauer |
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by 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).
--
Ticket URL: <https://code.djangoproject.com/ticket/26488#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: Christopher | Owner: nobody
Neugebauer |
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Matthijs Kooijman):
* cc: Matthijs Kooijman (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/26488#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: Christopher | Owner: nobody
Neugebauer |
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Aaron Riedener):
* cc: Aaron Riedener (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/26488#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: Christopher | Owner: nobody
Neugebauer |
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by 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?
--
Ticket URL: <https://code.djangoproject.com/ticket/26488#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: Christopher | Owner: nobody
Neugebauer |
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by 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='CloudOld',
),
]
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26488#comment:17>
Django
-------------------------------------+-------------------------------------
Reporter: Christopher | Owner: nobody
Neugebauer |
Component: Migrations | Version: master
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* status: new => closed
* resolution: => duplicate
* needs_tests: 1 => 0
Comment:
Adding `migrations.AlterModelBases()` to a migration `operations` from
[https://github.com/django/django/pull/11222 patch] proposed for #23521
fix this issue for me, e.g.
{{{ #!python
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26488#comment:18>