[Django] #25247: makemigrations unable to generate necessary migration for making a superclass abstract
Django
superclass abstract
-------------------------------+-------------------------------------
Reporter: rapilabs | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Keywords: abstract makemigrations
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-------------------------------------
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:
{{{#!python
# 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'
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25247>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
superclass abstract
Reporter: rapilabs | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: abstract | Triage Stage: Accepted
makemigrations |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_docs: => 0
* needs_better_patch: => 0
* type: Uncategorized => Cleanup/optimization
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
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 to `docs/howto
/writing-migrations.txt`.
--
Ticket URL: <https://code.djangoproject.com/ticket/25247#comment:1>
Django
superclass abstract
Reporter: rapilabs | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: abstract | Triage Stage: Accepted
makemigrations |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by charettes):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25247#comment:2>
Django
superclass abstract
Reporter: rapilabs | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: abstract | Triage Stage: Accepted
makemigrations |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MarkusH):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25247#comment:3>
Django
superclass abstract
Reporter: rapilabs | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: abstract | Triage Stage: Accepted
makemigrations |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by charettes):
@MarkusH, I think it shouldn't be an issue if the sequence is started at
the referenced field's serial current value:
{{{#!sql
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)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25247#comment:4>
Django
superclass abstract
Reporter: rapilabs | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: abstract | Triage Stage: Accepted
makemigrations |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by charettes):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25247#comment:5>
Django
superclass abstract
Reporter: David Sanders | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: abstract | Triage Stage: Accepted
makemigrations |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
#27940 and #27941 are duplicates.
--
Ticket URL: <https://code.djangoproject.com/ticket/25247#comment:6>