[Django] #24264: foreign key constraint error migrating integer pk to CharField
Django
--------------------------------------+---------------------------
Reporter: timgraham | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8alpha1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
--------------------------------------+---------------------------
Discovered while trying to update djangoproject.com to 1.8a1. See
https://github.com/django/djangoproject.com/pull/373 for steps to
reproduce.
{{{
$ python manage.py test fundraising
...
foreign key constraint
"fundrais_donor_id_50d7dcbcff28bd90_fk_fundraising_djangohero_id" cannot
be implemented
DETAIL: Key columns "donor_id" and "id" are of incompatible types:
integer and character varying.
}}}
Bisected to bbbed99f6260a8b3e65cb990e49721b1ce4a441b
Here's a test for `tests/schema/tests.py` which is at least a start (it
fails on 1.7.x too, so it's probably not quite right):
{{{
def test_change_int_pk_to_char(self):
with connection.schema_editor() as editor:
editor.create_model(Author)
editor.create_model(Book)
new_field = CharField(max_length=15, primary_key=True)
new_field.set_attributes_from_name("id")
new_field.model = Author
with connection.schema_editor() as editor:
editor.alter_field(Author, Author._meta.get_field("id"),
new_field)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24264>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: timgraham | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8alpha1
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 MarkusH):
* cc: MarkusH (added)
* stage: Unreviewed => Accepted
Comment:
Although I haven't tried to reproduce this particular issue, I wouldn't be
surprised if this is related to #24241
A brief discussion on IRC mentioned this traceback:
{{{#!python
Traceback (most recent call last):
File "manage.py", line 10, in <module>
execute_from_command_line(sys.argv)
File "/home/tim/code/django/django/core/management/__init__.py", line
338, in execute_from_command_line
utility.execute()
File "/home/tim/code/django/django/core/management/__init__.py", line
330, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/home/tim/code/django/django/core/management/commands/test.py",
line 30, in run_from_argv
super(Command, self).run_from_argv(argv)
File "/home/tim/code/django/django/core/management/base.py", line 390,
in run_from_argv
self.execute(*args, **cmd_options)
File "/home/tim/code/django/django/core/management/commands/test.py",
line 74, in execute
super(Command, self).execute(*args, **options)
File "/home/tim/code/django/django/core/management/base.py", line 441,
in execute
output = self.handle(*args, **options)
File "/home/tim/code/django/django/core/management/commands/test.py",
line 90, in handle
failures = test_runner.run_tests(test_labels)
File "/home/tim/code/django/django/test/runner.py", line 210, in
run_tests
old_config = self.setup_databases()
File "/home/tim/code/django/django/test/runner.py", line 166, in
setup_databases
**kwargs
File "/home/tim/code/django/django/test/runner.py", line 370, in
setup_databases
serialize=connection.settings_dict.get("TEST", {}).get("SERIALIZE",
True),
File "/home/tim/code/django/django/db/backends/base/creation.py", line
369, in create_test_db
test_flush=True,
File "/home/tim/code/django/django/core/management/__init__.py", line
120, in call_command
return command.execute(*args, **defaults)
File "/home/tim/code/django/django/core/management/base.py", line 441,
in execute
output = self.handle(*args, **options)
File "/home/tim/code/django/django/core/management/commands/migrate.py",
line 213, in handle
executor.migrate(targets, plan, fake=options.get("fake", False))
File "/home/tim/code/django/django/db/migrations/executor.py", line 93,
in migrate
self.apply_migration(states[migration], migration, fake=fake)
File "/home/tim/code/django/django/db/migrations/executor.py", line 129,
in apply_migration
state = migration.apply(state, schema_editor)
File "/home/tim/code/django/django/db/migrations/migration.py", line
110, in apply
operation.database_forwards(self.app_label, schema_editor, old_state,
project_state)
File "/home/tim/code/django/django/db/migrations/operations/fields.py",
line 200, in database_forwards
schema_editor.alter_field(from_model, from_field, to_field)
File "/home/tim/code/django/django/db/backends/base/schema.py", line
484, in alter_field
old_db_params, new_db_params, strict)
File "/home/tim/code/django/django/db/backends/base/schema.py", line
633, in _alter_field
params,
File "/home/tim/code/django/django/db/backends/base/schema.py", line
107, in execute
cursor.execute(sql, params)
File "/home/tim/code/django/django/db/backends/utils.py", line 65, in
execute
return self.cursor.execute(sql, params)
File "/home/tim/code/django/django/db/utils.py", line 95, in __exit__
six.reraise(dj_exc_type, dj_exc_value, traceback)
File "/home/tim/code/django/django/db/backends/utils.py", line 65, in
execute
return self.cursor.execute(sql, params)
django.db.utils.ProgrammingError: foreign key constraint
"fundrais_donor_id_50d7dcbcff28bd90_fk_fundraising_djangohero_id" cannot
be implemented
DETAIL: Key columns "donor_id" and "id" are of incompatible types:
integer and character varying.
}}}
and this SQL
{{{#!sql
BEGIN;
ALTER TABLE "fundraising_donation" DROP CONSTRAINT
"fundraising_donation_donor_id_50d7dcbcff28bd90_fk";
ALTER TABLE "fundraising_djangohero" ALTER COLUMN "id" TYPE varchar(12);
ALTER TABLE "fundraising_donation" ALTER COLUMN "donor_id" TYPE
varchar(12);
ALTER TABLE "fundraising_donation" ADD CONSTRAINT
"fundraising_donation_donor_id_50d7dcbcff28bd90_fk" FOREIGN KEY
("donor_id") REFERENCES "fundraising_djangohero" ("id") DEFERRABLE
INITIALLY DEFERRED;
}}}
The interesting part is the difference between the constraint names.
--
Ticket URL: <https://code.djangoproject.com/ticket/24264#comment:1>
Django
Reporter: timgraham | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8alpha1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MarkusH):
I looked into the issue: it is neither related nor unrelated to the
bisected commit mentioned above. The problem is that in
[https://github.com/django/django/blob/38c17871bb6dafd489367f6fe8bc56199223adb8/django/db/backends/base/schema.py#L520
django.db.backends.base.schema._alter_field()]
`new_field.model._meta.related_objects` for `DjangoHero` is an empty list
despite `Donation` having a FK.
The
[https://github.com/django/django/blob/38c17871bb6dafd489367f6fe8bc56199223adb8/django/db/models/options.py#L685
related_objects_graph] after the first `for` loop looks like:
{{{
{<Options for DjangoHero>: [<django.db.models.fields.related.ForeignKey:
donor>],
<Options for Site>: [<django.db.models.fields.related.ForeignKey: site>,
<django.db.models.fields.related.ManyToManyField:
sites>],
<Options for FlatPage>: [<django.db.models.fields.related.ForeignKey:
flatpage>],
<Options for DocumentRelease>:
[<django.db.models.fields.related.ForeignKey: release>],
<Options for Category>: [<django.db.models.fields.related.ForeignKey:
category>,
<django.db.models.fields.related.ForeignKey:
category>,
<django.db.models.fields.related.ForeignKey:
category>,
<django.db.models.fields.related.ForeignKey:
category>],
<Options for ContentType>: [<django.db.models.fields.related.ForeignKey:
content_type>],
<Options for CCLA>: [<django.db.models.fields.related.ForeignKey: ccla>],
<Options for ICLA>: [<django.db.models.fields.related.ForeignKey: icla>],
<Options for Group>: [<django.db.models.fields.related.ForeignKey:
group>,
<django.db.models.fields.related.ForeignKey:
group>,
<django.db.models.fields.related.ManyToManyField:
groups>],
<Options for Permission>: [<django.db.models.fields.related.ForeignKey:
permission>,
<django.db.models.fields.related.ManyToManyField: user_permissions>],
<Options for User>: [<django.db.models.fields.related.ForeignKey: user>,
<django.db.models.fields.related.ForeignKey: user>,
<django.db.models.fields.related.ForeignKey: user>,
<django.db.models.fields.related.ForeignKey: user>],
<Options for Permission>: [<django.db.models.fields.related.ForeignKey:
permission>,
<django.db.models.fields.related.ManyToManyField: permissions>],
<Options for FeedType>: [<django.db.models.fields.related.ForeignKey:
feed_type>],
<Options for Feed>: [<django.db.models.fields.related.ForeignKey: feed>],
<Options for User>: [<django.db.models.fields.related.OneToOneField:
user>,
<django.db.models.fields.related.ForeignKey: user>,
<django.db.models.fields.related.ForeignKey:
owner>],
<Options for ContentType>: [<django.db.models.fields.related.ForeignKey:
content_type>,
<django.db.models.fields.related.ForeignKey:
content_type>]}
}}}
However, due to state changes during migrations `self.model`
(`new_field.model._meta.model`) is not the same instance as the
`DjangoHero` model from
[https://github.com/django/django/blob/38c17871bb6dafd489367f6fe8bc56199223adb8/django/db/models/options.py#L687
all_models()]
--
Ticket URL: <https://code.djangoproject.com/ticket/24264#comment:2>
Django
Reporter: timgraham | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8alpha1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MarkusH):
This patch on stable/1.8.x (edbf6de7536f7a6c1e5df019a5e1947d2c9dadf8)
fixes the problem for me, but is more a band-aid than a valid fix:
{{{#!patch
diff --git a/django/db/models/options.py b/django/db/models/options.py
index fa97265..e64990d 100644
--- a/django/db/models/options.py
+++ b/django/db/models/options.py
@@ -687,7 +687,8 @@ class Options(object):
for f in fields_with_relations:
if not isinstance(f.rel.to, six.string_types):
# Set options_instance -> field
- related_objects_graph[f.rel.to._meta].append(f)
+ opts = f.rel.to._meta
+ related_objects_graph[opts.app_label,
opts.object_name].append(f)
for model in all_models:
# Set the relation_tree using the internal __dict__. In this
way
@@ -695,7 +696,8 @@ class Options(object):
# __dict__ takes precedence over a data descriptor (such as
# @cached_property). This means that the _meta._relation_tree
is
# only called if related_objects is not in __dict__.
- related_objects = related_objects_graph[model._meta]
+ opts = model._meta
+ related_objects = related_objects_graph[opts.app_label,
opts.object_name]
# If related_objects are empty, it makes sense to set
# EMPTY_RELATION_TREE. This will avoid allocating multiple
empty
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24264#comment:3>
Django
Reporter: timgraham | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8alpha1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MarkusH):
I managed to construct a test_case for this issue.
I made the FK span two apps to make sure it works across apps. Within the
same app it boils down to the same problem.
{{{#!diff
diff --git a/tests/migrations/migrations_test_apps/alter_fk/__init__.py
b/tests/migrations/migrations_test_apps/alter_fk/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git
a/tests/migrations/migrations_test_apps/alter_fk/author_app/__init__.py
b/tests/migrations/migrations_test_apps/alter_fk/author_app/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git
a/tests/migrations/migrations_test_apps/alter_fk/author_app/migrations/0001_initial.py
b/tests/migrations/migrations_test_apps/alter_fk/author_app/migrations/0001_initial.py
new file mode 100644
index 0000000..6017d16
--- /dev/null
+++
b/tests/migrations/migrations_test_apps/alter_fk/author_app/migrations/0001_initial.py
@@ -0,0 +1,20 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import models, migrations
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ]
+
+ operations = [
+ migrations.CreateModel(
+ name='Author',
+ fields=[
+ ('id', models.AutoField(serialize=False,
auto_created=True, primary_key=True)),
+ ('name', models.CharField(max_length=50)),
+ ],
+ ),
+ ]
diff --git
a/tests/migrations/migrations_test_apps/alter_fk/author_app/migrations/0002_alter_id.py
b/tests/migrations/migrations_test_apps/alter_fk/author_app/migrations/0002_alter_id.py
new file mode 100644
index 0000000..0256b75
--- /dev/null
+++
b/tests/migrations/migrations_test_apps/alter_fk/author_app/migrations/0002_alter_id.py
@@ -0,0 +1,20 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import models, migrations
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('author_app', '0001_initial'),
+ ('book_app', '0001_initial'), # Forces the book table to alter
the FK
+ ]
+
+ operations = [
+ migrations.AlterField(
+ model_name='author',
+ name='id',
+ field=models.CharField(max_length=10, primary_key=True),
+ ),
+ ]
diff --git
a/tests/migrations/migrations_test_apps/alter_fk/author_app/migrations/__init__.py
b/tests/migrations/migrations_test_apps/alter_fk/author_app/migrations/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git
a/tests/migrations/migrations_test_apps/alter_fk/book_app/__init__.py
b/tests/migrations/migrations_test_apps/alter_fk/book_app/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git
a/tests/migrations/migrations_test_apps/alter_fk/book_app/migrations/0001_initial.py
b/tests/migrations/migrations_test_apps/alter_fk/book_app/migrations/0001_initial.py
new file mode 100644
index 0000000..eb1862d
--- /dev/null
+++
b/tests/migrations/migrations_test_apps/alter_fk/book_app/migrations/0001_initial.py
@@ -0,0 +1,22 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import models, migrations
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('author_app', '0001_initial'),
+ ]
+
+ operations = [
+ migrations.CreateModel(
+ name='Book',
+ fields=[
+ ('id', models.AutoField(serialize=False,
auto_created=True, primary_key=True)),
+ ('title', models.CharField(max_length=50)),
+ ('author', models.ForeignKey('author_app.Author')),
+ ],
+ ),
+ ]
diff --git
a/tests/migrations/migrations_test_apps/alter_fk/book_app/migrations/__init__.py
b/tests/migrations/migrations_test_apps/alter_fk/book_app/migrations/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/tests/migrations/test_executor.py
b/tests/migrations/test_executor.py
index 12385ac..30de553 100644
--- a/tests/migrations/test_executor.py
+++ b/tests/migrations/test_executor.py
@@ -368,6 +368,41 @@ class ExecutorTests(MigrationTestBase):
]
self.assertEqual(call_args_list, expected)
+ @override_settings(
+ INSTALLED_APPS=[
+ "migrations.migrations_test_apps.alter_fk.author_app",
+ "migrations.migrations_test_apps.alter_fk.book_app",
+ ]
+ )
+ def test_alter_id_type_with_fk(self):
+ try:
+ executor = MigrationExecutor(connection)
+ self.assertTableNotExists("author_app_author")
+ self.assertTableNotExists("book_app_book")
+ # Apply initial migrations
+ executor.migrate([
+ ("author_app", "0001_initial"),
+ ("book_app", "0001_initial"),
+ ])
+ self.assertTableExists("author_app_author")
+ self.assertTableExists("book_app_book")
+ # Rebuild the graph to reflect the new DB state
+ executor.loader.build_graph()
+
+ # Apply PK type alteration
+ executor.migrate([("author_app", "0002_alter_id")])
+
+ # Rebuild the graph to reflect the new DB state
+ executor.loader.build_graph()
+ finally:
+ # Cleanup
+ executor.migrate([
+ ("author_app", None),
+ ("book_app", None),
+ ])
+ self.assertTableNotExists("author_app_author")
+ self.assertTableNotExists("book_app_book")
+
class FakeLoader(object):
def __init__(self, graph, applied):
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24264#comment:4>
Django
Reporter: timgraham | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8alpha1
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):
* severity: Normal => Release blocker
--
Ticket URL: <https://code.djangoproject.com/ticket/24264#comment:5>
Django
Reporter: timgraham | Owner: MarkusH
Type: Bug | Status: assigned
Component: Migrations | Version: 1.8alpha1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* owner: nobody => MarkusH
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/24264#comment:6>
Django
Reporter: timgraham | Owner: MarkusH
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
* needs_tests: 0 => 1
* needs_docs: 0 => 1
Comment:
Based on Claude's initial patch for #24225 I started working on bigger
migration issue regarding states. More on the PR
https://github.com/django/django/pull/4097
--
Ticket URL: <https://code.djangoproject.com/ticket/24264#comment:7>
Django
Reporter: timgraham | Owner: MarkusH
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 1 => 0
* needs_docs: 1 => 0
* needs_tests: 1 => 0
* stage: Accepted => Ready for checkin
Comment:
The PR fixes the issue I encountered on djangoproject.com.
--
Ticket URL: <https://code.djangoproject.com/ticket/24264#comment:8>
Django
Reporter: timgraham | Owner: MarkusH
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Markus Holtermann <info@…>):
In [changeset:"cc22b009e05456e3d9cf3c152fe47fa27772be5e"]:
{{{
#!CommitTicketReference repository=""
revision="cc22b009e05456e3d9cf3c152fe47fa27772be5e"
Refs #24264 -- Added failing test case for updating a FK when changing a
PK
When the primary key column is altered, foreign keys of referencing
models must be aware of a possible data type change as well and thus
need to be re-rendered.
Thanks Tim Graham for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24264#comment:9>
Django
Reporter: timgraham | Owner: MarkusH
Component: Migrations | Version: 1.8alpha1
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Markus Holtermann <info@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"b29f3b51204d53c1c8745966476543d068c173a2"]:
{{{
#!CommitTicketReference repository=""
revision="b29f3b51204d53c1c8745966476543d068c173a2"
Fixed #24225, #24264, #24282 -- Rewrote model reloading in migration
project state
Instead of naively reloading only directly related models (FK, O2O, M2M
relationship) the project state needs to reload their relations as well
as the model changes as well. Furthermore inheriting models (and super
models) need to be reloaded in order to keep inherited fields in sync.
To prevent endless recursive calls an iterative approach is taken.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24264#comment:10>
Django
Reporter: timgraham | Owner: MarkusH
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Markus Holtermann <info@…>):
In [changeset:"8ca0eb2af70b7fbfd9689553d40ef6023433f94e"]:
{{{
#!CommitTicketReference repository=""
revision="8ca0eb2af70b7fbfd9689553d40ef6023433f94e"
[1.8.x] Refs #24264 -- Added failing test case for updating a FK when
changing a PK
When the primary key column is altered, foreign keys of referencing
models must be aware of a possible data type change as well and thus
need to be re-rendered.
Thanks Tim Graham for the report.
Backport of cc22b009e05456e3d9cf3c152fe47fa27772be5e from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24264#comment:11>
Django
Reporter: timgraham | Owner: MarkusH
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Markus Holtermann <info@…>):
In [changeset:"a1ba4627931591b80afa46e38e261f354151d91a"]:
{{{
#!CommitTicketReference repository=""
revision="a1ba4627931591b80afa46e38e261f354151d91a"
[1.8.x] Fixed #24225, #24264, #24282 -- Rewrote model reloading in
migration project state
Instead of naively reloading only directly related models (FK, O2O, M2M
relationship) the project state needs to reload their relations as well
as the model changes as well. Furthermore inheriting models (and super
models) need to be reloaded in order to keep inherited fields in sync.
To prevent endless recursive calls an iterative approach is taken.
Backport of b29f3b51204d53c1c8745966476543d068c173a2 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24264#comment:12>