Opened 9 years ago
Last modified 8 years ago
#25247 new Cleanup/optimization
makemigrations unable to generate necessary migration for making a superclass abstract
Reported by: | David Sanders | Owned by: | nobody |
---|---|---|---|
Component: | Migrations | Version: | 1.8 |
Severity: | Normal | Keywords: | abstract makemigrations |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
If a user has an existing model state that uses model inheritance and changes a superclass to become abstract then makemigrations is unable to produce a suitable migration. In fact the resulting migration will actually raise an exception when run.
This is non-trivial as a generated migration may require some data migration operations.
This was reported by a user on #django and I was able to reproduce with a small test:
# Initial model state: class Foo(models.Model): name = models.CharField(max_length=255) class Bar(Foo): pass # Subsequent model state: class Foo(models.Model): name = models.CharField(max_length=255) class Meta: abstract=True class Bar(Foo): pass # running makemigrations results in: class Migration(migrations.Migration): dependencies = [ ('foobar', '0001_initial'), ] operations = [ migrations.RemoveField( model_name='bar', name='foo_ptr', ), migrations.AddField( model_name='bar', name='id', field=models.AutoField(auto_created=True, primary_key=True, default=1, serialize=False, verbose_name='ID'), preserve_default=False, ), migrations.AddField( model_name='bar', name='name', field=models.CharField(default='asdf', max_length=255), preserve_default=False, ), migrations.DeleteModel( name='Foo', ), ] # which results in the following when attempting to run migrate: (env)dsanders ~/test/abstract_update/test_abstract $ ./manage.py migrate Operations to perform: Synchronize unmigrated apps: staticfiles, messages Apply all migrations: admin, foobar, contenttypes, auth, sessions Synchronizing apps without migrations: Creating tables... Running deferred SQL... Installing custom SQL... Running migrations: Rendering model states...Traceback (most recent call last): File "./manage.py", line 10, in <module> execute_from_command_line(sys.argv) File "/Users/dsanders/test/abstract_update/env/lib/python2.7/site-packages/django/core/management/__init__.py", line 338, in execute_from_command_line utility.execute() File "/Users/dsanders/test/abstract_update/env/lib/python2.7/site-packages/django/core/management/__init__.py", line 330, in execute self.fetch_command(subcommand).run_from_argv(self.argv) File "/Users/dsanders/test/abstract_update/env/lib/python2.7/site-packages/django/core/management/base.py", line 393, in run_from_argv self.execute(*args, **cmd_options) File "/Users/dsanders/test/abstract_update/env/lib/python2.7/site-packages/django/core/management/base.py", line 444, in execute output = self.handle(*args, **options) File "/Users/dsanders/test/abstract_update/env/lib/python2.7/site-packages/django/core/management/commands/migrate.py", line 221, in handle executor.migrate(targets, plan, fake=fake, fake_initial=fake_initial) File "/Users/dsanders/test/abstract_update/env/lib/python2.7/site-packages/django/db/migrations/executor.py", line 104, in migrate state = migration.mutate_state(state, preserve=do_run) File "/Users/dsanders/test/abstract_update/env/lib/python2.7/site-packages/django/db/migrations/migration.py", line 83, in mutate_state operation.state_forwards(self.app_label, new_state) File "/Users/dsanders/test/abstract_update/env/lib/python2.7/site-packages/django/db/migrations/operations/fields.py", line 51, in state_forwards state.reload_model(app_label, self.model_name_lower) File "/Users/dsanders/test/abstract_update/env/lib/python2.7/site-packages/django/db/migrations/state.py", line 152, in reload_model self.apps.render_multiple(states_to_be_rendered) File "/Users/dsanders/test/abstract_update/env/lib/python2.7/site-packages/django/db/migrations/state.py", line 262, in render_multiple model.render(self) File "/Users/dsanders/test/abstract_update/env/lib/python2.7/site-packages/django/db/migrations/state.py", line 546, in render body, File "/Users/dsanders/test/abstract_update/env/lib/python2.7/site-packages/django/db/models/base.py", line 254, in __new__ 'base class %r' % (field.name, name, base.__name__) django.core.exceptions.FieldError: Local field u'id' in class 'Bar' clashes with field of similar name from base class 'Foo'
Change History (6)
comment:1 by , 9 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Uncategorized → Cleanup/optimization |
comment:2 by , 9 years ago
I agree that generating a data migration might be risky but we could at least try do express a OneToOneField(auto_created=True, primary_key=True)
-> AutoField(auto_created=True, primary_key=True)
change as an AlterField
operation.
comment:3 by , 9 years ago
Even that is not really feasible, Simon, as PostgreSQL will automatically populate the new id column with sequence values, potentially changing the order of the result set when ordering by primary key.
comment:4 by , 9 years ago
@MarkusH, I think it shouldn't be an issue if the sequence is started at the referenced field's serial current value:
CREATE TABLE foo ( id serial primary key ); CREATE TABLE bar ( foo_ptr_id integer primary key REFERENCES foo (id) ); INSERT INTO foo DEFAULT VALUES; INSERT INTO foo DEFAULT VALUES; INSERT INTO foo DEFAULT VALUES; SELECT * FROM foo; id ---- 1 2 3 (3 rows) INSERT INTO bar SELECT id FROM foo; SELECT * FROM bar; foo_ptr_id ------------ 1 2 3 (3 rows) CREATE SEQUENCE bar_id_seq; SELECT setval('bar_id_seq', currval('foo_id_seq')); ALTER TABLE bar RENAME COLUMN foo_ptr_id TO id; ALTER TABLE bar ALTER COLUMN id SET DEFAULT nextval('bar_id_seq'); ALTER TABLE bar DROP CONSTRAINT bar_foo_ptr_id_fkey; INSERT INTO bar DEFAULT VALUES; INSERT INTO bar DEFAULT VALUES; SELECT * FROM bar; id ---- 1 2 3 4 5 (5 rows)
comment:5 by , 9 years ago
It looks like it's only a matter of teaching the auto detector to consider auto_created
primary keys changes as automatic rename and alters since most the integer
to serial
alteration logic is already implemented.
I guess it's probably not feasible to allow
makemigrations
to generate a correct migration in cases like this (e.g. auto-generating data migration seems risky), but we could at least throw a more helpful error message or add some documentation about how to handle this case todocs/howto/writing-migrations.txt
.