From 81b1c167bf919ddbd5aa0289f9f3761fc62addf3 Mon Sep 17 00:00:00 2001 From: DevilsAutumn Date: Fri, 11 Nov 2022 00:13:16 +0530 Subject: [PATCH] Fixed #28987 -- Fixed altering ManyToManyField when changing to self-referential. --- django/db/backends/sqlite3/schema.py | 49 ++++++++++++++++++---------- tests/migrations/test_operations.py | 37 +++++++++++++++++++++ 2 files changed, 68 insertions(+), 18 deletions(-) diff --git a/django/db/backends/sqlite3/schema.py b/django/db/backends/sqlite3/schema.py index 88fa466f7921..c9e924b182c7 100644 --- a/django/db/backends/sqlite3/schema.py +++ b/django/db/backends/sqlite3/schema.py @@ -174,7 +174,7 @@ def alter_field(self, model, old_field, new_field, strict=False): super().alter_field(model, old_field, new_field, strict=strict) def _remake_table( - self, model, create_field=None, delete_field=None, alter_field=None + self, model, create_field=None, delete_field=None, alter_fields=None ): """ Shortcut to transform a model from old_model into new_model @@ -213,15 +213,16 @@ def is_self_referential(f): # If any of the new or altered fields is introducing a new PK, # remove the old one restore_pk_field = None - if getattr(create_field, "primary_key", False) or ( - alter_field and getattr(alter_field[1], "primary_key", False) + alter_fields = alter_fields or [] + if getattr(create_field, "primary_key", False) or any( + getattr(new_field, "primary_key", False) for _, new_field in alter_fields ): for name, field in list(body.items()): - if field.primary_key and not ( + if field.primary_key and not any( # Do not remove the old primary key when an altered field # that introduces a primary key is the same field. - alter_field - and name == alter_field[1].name + name == new_field.name + for _, new_field in alter_fields ): field.primary_key = False restore_pk_field = field @@ -237,7 +238,7 @@ def is_self_referential(f): self.effective_default(create_field), ) # Add in any altered fields - if alter_field: + for alter_field in alter_fields: old_field, new_field = alter_field body.pop(old_field.name, None) mapping.pop(old_field.column, None) @@ -457,7 +458,7 @@ def _alter_field( ) ) # Alter by remaking table - self._remake_table(model, alter_field=(old_field, new_field)) + self._remake_table(model, alter_fields=[(old_field, new_field)]) # Rebuild tables with FKs pointing to this field. old_collation = old_db_params.get("collation") new_collation = new_db_params.get("collation") @@ -495,18 +496,30 @@ def _alter_many_to_many(self, model, old_field, new_field, strict): # propagate this altering. self._remake_table( old_field.remote_field.through, - alter_field=( - # The field that points to the target model is needed, 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.remote_field.through._meta.get_field( - old_field.m2m_reverse_field_name() + alter_fields=[ + ( + # The field that points to the target model is needed, + # so that table can be remade with the new m2m field - + # this is m2m_reverse_field_name(). + old_field.remote_field.through._meta.get_field( + old_field.m2m_reverse_field_name() + ), + new_field.remote_field.through._meta.get_field( + new_field.m2m_reverse_field_name() + ), ), - new_field.remote_field.through._meta.get_field( - new_field.m2m_reverse_field_name() + ( + # The field that points to the model itself is needed, + # so that table can be remade with the new self field - + # this is m2m_field_name(). + old_field.remote_field.through._meta.get_field( + old_field.m2m_field_name() + ), + new_field.remote_field.through._meta.get_field( + new_field.m2m_field_name() + ), ), - ), + ], ) return diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index e02dc9ef3f81..dc0d34eebd1d 100644 --- a/tests/migrations/test_operations.py +++ b/tests/migrations/test_operations.py @@ -1796,6 +1796,43 @@ def test_alter_model_table_m2m(self): self.assertTableExists(original_m2m_table) self.assertTableNotExists(new_m2m_table) + def test_alter_model_table_m2m_field(self): + app_label = "test_talm2mfl" + project_state = self.set_up_test_model(app_label, second_model=True) + # Add the M2M field. + project_state = self.apply_operations( + app_label, + project_state, + operations=[ + migrations.AddField( + "Pony", + "stables", + models.ManyToManyField("Stable"), + ) + ], + ) + m2m_table = f"{app_label}_pony_stables" + self.assertColumnExists(m2m_table, "pony_id") + self.assertColumnExists(m2m_table, "stable_id") + # Point the M2M field to self. + with_field_state = project_state.clone() + operations = [ + migrations.AlterField( + model_name="Pony", + name="stables", + field=models.ManyToManyField("self"), + ) + ] + project_state = self.apply_operations( + app_label, project_state, operations=operations + ) + self.assertColumnExists(m2m_table, "from_pony_id") + self.assertColumnExists(m2m_table, "to_pony_id") + # Reversal. + self.unapply_operations(app_label, with_field_state, operations=operations) + self.assertColumnExists(m2m_table, "pony_id") + self.assertColumnExists(m2m_table, "stable_id") + def test_alter_field(self): """ Tests the AlterField operation.