Opened 9 years ago
Closed 6 years ago
#26488 closed Bug (duplicate)
migrate crashes when renaming a multi-table inheritance base model
Reported by: | Christopher Neugebauer | Owned by: | nobody |
---|---|---|---|
Component: | Migrations | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Matthijs Kooijman, Aaron Riedener | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | 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 (18)
comment:1 by , 9 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 9 years ago
Summary: | migration raises django.db.migrations.exceptions.InvalidBasesError if you rename a base model → migrate raises InvalidBasesError if you rename a multitable inheritance base model |
---|
comment:3 by , 8 years ago
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)
comment:4 by , 8 years ago
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.
follow-up: 6 comment:5 by , 7 years ago
Summary: | migrate raises InvalidBasesError if you rename a multitable inheritance base model → migrate 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 by , 7 years ago
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 by , 7 years ago
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 by , 7 years ago
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.
comment:9 by , 7 years ago
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 by , 7 years ago
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 by , 7 years ago
Needs tests: | set |
---|---|
Version: | 1.9 → master |
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 by , 7 years ago
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.
comment:13 by , 7 years ago
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).
comment:14 by , 7 years ago
Cc: | added |
---|
comment:15 by , 7 years ago
Cc: | added |
---|
comment:16 by , 7 years ago
@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
comment:17 by , 6 years ago
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', ), ]
comment:18 by , 6 years ago
Needs tests: | unset |
---|---|
Resolution: | → duplicate |
Status: | new → closed |
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.
May be related or a duplicate of #23521. Created #26490 for a regression in master which hides the
InvalidBasesError
behind aRecursionError: maximum recursion depth exceeded