Ticket #22432: 22432-fix-m2m-repointing.patch

django/db/backends/schema.py

@@ -491 +491 @@
                 old_field,
                 new_field,
             ))
+
+        self._alter_field(model, old_field, new_field, old_type, new_type, old_db_params, new_db_params, strict)
+
+    def _alter_field(self, model, old_field, new_field, old_type, new_type, old_db_params, new_db_params, strict=False):
+        """Actually perform a "physical" (non-ManyToMany) field update."""
+
         # Has unique been removed?
         if old_field.unique and (not new_field.unique or (not old_field.primary_key and new_field.primary_key)):
             # Find the unique constraint for this field

django/db/backends/sqlite3/schema.py

@@ +1 @@
+import copy
+
 from decimal import Decimal
 from django.utils import six
 from django.apps.registry import Apps
@@ -63 +65 @@
             del mapping[old_field.column]
             body[new_field.name] = new_field
             mapping[new_field.column] = old_field.column
+
         # Remove any deleted fields
         for field in delete_fields:
             del body[field.name]
@@ -72 +75 @@
                 return self.delete_model(field.rel.through)
         # Work inside a new app registry
         apps = Apps()
+
+        # Provide isolated instances of the fields to the new model body
+        # Instantiating the new model with an alternate db_table will alter
+        # the internal references of some of the provided fields.
+        body = copy.deepcopy(body)
+
         # Construct a new model for the new state
         meta_contents = {
             'app_label': model._meta.app_label,
@@ -82 +91 @@
         meta = type("Meta", tuple(), meta_contents)
         body['Meta'] = meta
         body['__module__'] = model.__module__
+
         temp_model = type(model._meta.object_name, model.__bases__, body)
         # Create a new table with that format
         self.create_model(temp_model)
@@ -134 +144 @@
         else:
             self._remake_table(model, delete_fields=[field])
 
-    def alter_field(self, model, old_field, new_field, strict=False):
-        """
-        Allows a field's type, uniqueness, nullability, default, column,
-        constraints etc. to be modified.
-        Requires a copy of the old field as well so we can only perform
-        changes that are required.
-        If strict is true, raises errors if the old column does not match old_field precisely.
-        """
-        old_db_params = old_field.db_parameters(connection=self.connection)
-        old_type = old_db_params['type']
-        new_db_params = new_field.db_parameters(connection=self.connection)
-        new_type = new_db_params['type']
-        if old_type is None and new_type is None and (old_field.rel.through and new_field.rel.through and old_field.rel.through._meta.auto_created and new_field.rel.through._meta.auto_created):
-            return self._alter_many_to_many(model, old_field, new_field, strict)
-        elif old_type is None and new_type is None and (old_field.rel.through and new_field.rel.through and not old_field.rel.through._meta.auto_created and not new_field.rel.through._meta.auto_created):
-            # Both sides have through models; this is a no-op.
-            return
-        elif old_type is None or new_type is None:
-            raise ValueError("Cannot alter field %s into %s - they are not compatible types (you cannot alter to or from M2M fields, or add or remove through= on M2M fields)" % (
-                old_field,
-                new_field,
-            ))
+    def _alter_field(self, model, old_field, new_field, old_type, new_type, old_db_params, new_db_params, strict=False):
+        """Actually perform a "physical" (non-ManyToMany) field update."""
         # Alter by remaking table
         self._remake_table(model, alter_fields=[(old_field, new_field)])
 
@@ -172 +162 @@
         Alters M2Ms to repoint their to= endpoints.
         """
         if old_field.rel.through._meta.db_table == new_field.rel.through._meta.db_table:
+            # The field name didn't change, but some options did; we have to propagate this altering.
+            self._remake_table(
+                old_field.rel.through,
+                alter_fields=[(
+                    # We need the field that points to the target model, so we can tell alter_field to change it -
+                    # this is m2m_reverse_field_name() (as opposed to m2m_field_name, which points to our model)
+                    old_field.rel.through._meta.get_field_by_name(old_field.m2m_reverse_field_name())[0],
+                    new_field.rel.through._meta.get_field_by_name(new_field.m2m_reverse_field_name())[0],
+                )],
+                override_uniques=(new_field.m2m_field_name(), new_field.m2m_reverse_field_name()),
+            )
             return
 
         # Make a new through table

tests/migrations/test_operations.py

@@ -34 +34 @@
         with connection.schema_editor() as editor:
             return migration.unapply(project_state, editor)
 
-    def set_up_test_model(self, app_label, second_model=False, related_model=False, mti_model=False):
+    def set_up_test_model(self, app_label, second_model=False, third_model=False, related_model=False, mti_model=False):
         """
         Creates a test model state and database table.
         """
         # Delete the tables if they already exist
         with connection.cursor() as cursor:
+            # Start with ManyToMany tables
+            try:
+                cursor.execute("DROP TABLE %s_pony_stables" % app_label)
+            except DatabaseError:
+                pass
+            try:
+                cursor.execute("DROP TABLE %s_pony_vans" % app_label)
+            except DatabaseError:
+                pass
+
+            # Then standard model tables
             try:
                 cursor.execute("DROP TABLE %s_pony" % app_label)
             except DatabaseError:
@@ -48 +59 @@
                 cursor.execute("DROP TABLE %s_stable" % app_label)
             except DatabaseError:
                 pass
+            try:
+                cursor.execute("DROP TABLE %s_van" % app_label)
+            except DatabaseError:
+                pass
         # Make the "current" state
         operations = [migrations.CreateModel(
             "Pony",
@@ -64 +79 @@
                     ("id", models.AutoField(primary_key=True)),
                 ]
             ))
+        if third_model:
+            operations.append(migrations.CreateModel(
+                "Van",
+                [
+                    ("id", models.AutoField(primary_key=True)),
+                ]
+            ))
         if related_model:
             operations.append(migrations.CreateModel(
                 "Rider",
@@ -405 +427 @@
         Pony = new_apps.get_model("test_alflmm", "Pony")
         self.assertTrue(Pony._meta.get_field('stables').blank)
 
+    def test_repoint_field_m2m(self):
+        project_state = self.set_up_test_model("test_alflmm", second_model=True, third_model=True)
+
+        project_state = self.apply_operations("test_alflmm", project_state, operations=[
+            migrations.AddField("Pony", "places", models.ManyToManyField("Stable", related_name="ponies"))
+        ])
+        new_apps = project_state.render()
+        Pony = new_apps.get_model("test_alflmm", "Pony")
+
+        project_state = self.apply_operations("test_alflmm", project_state, operations=[
+            migrations.AlterField("Pony", "places", models.ManyToManyField(to="Van", related_name="ponies"))
+        ])
+
+        # Ensure the new field actually works
+        new_apps = project_state.render()
+        Pony = new_apps.get_model("test_alflmm", "Pony")
+        p = Pony.objects.create(pink=False, weight=4.55)
+        p.places.create()
+        self.assertEqual(p.places.count(), 1)
+        p.places.all().delete()
+
     def test_remove_field_m2m(self):
         project_state = self.set_up_test_model("test_rmflmm", second_model=True)