foreign key constraint error migrating integer pk to CharField

[Tim Graham]

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)

[Markus Holtermann]

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:

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

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.

[Markus Holtermann]

I looked into the issue: it is neither related nor unrelated to the bisected commit mentioned above. The problem is that in ​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 ​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 ​all_models()

[Markus Holtermann]

This patch on stable/1.8.x (edbf6de7536f7a6c1e5df019a5e1947d2c9dadf8) fixes the problem for me, but is more a band-aid than a valid fix:

django/db/models/options.py

@@ -687 +687 @@
             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 +696 @@
             # __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

[Markus Holtermann]

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.

new file tests/migrations/migrations_test_apps/alter_fk/author_app/migrations/0001_initial.py

@@ +1 @@
+# -*- 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)),
+            ],
+        ),
+    ]

new file tests/migrations/migrations_test_apps/alter_fk/author_app/migrations/0002_alter_id.py

@@ +1 @@
+# -*- 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),
+        ),
+    ]

new file tests/migrations/migrations_test_apps/alter_fk/book_app/migrations/0001_initial.py

@@ +1 @@
+# -*- 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')),
+            ],
+        ),
+    ]

tests/migrations/test_executor.py

@@ -368 +368 @@
         ]
         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):

[Markus Holtermann]

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

[Tim Graham]

The PR fixes the issue I encountered on djangoproject.com.

[Markus Holtermann <info@…>]

In 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.

[Markus Holtermann <info@…>]

In 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.

[Markus Holtermann <info@…>]

In 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

[Markus Holtermann <info@…>]

In 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