From 03900a02d5fd2efd6ae98bdb9974b78146f63040 Mon Sep 17 00:00:00 2001 From: Andrew Godwin Date: Tue, 20 May 2014 16:25:59 +0100 Subject: [PATCH] Fixed #22432: SQLite M2M repointing now works. Thanks to xelnor. --- django/db/backends/schema.py | 6 ++++ django/db/backends/sqlite3/schema.py | 44 +++++++++++++-------------- tests/migrations/test_operations.py | 45 +++++++++++++++++++++++++++- 3 files changed, 71 insertions(+), 24 deletions(-) diff --git a/django/db/backends/schema.py b/django/db/backends/schema.py index ba8118cb0f..2bca940459 100644 --- a/django/db/backends/schema.py +++ b/django/db/backends/schema.py @@ -499,6 +499,12 @@ class BaseDatabaseSchemaEditor(object): 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 diff --git a/django/db/backends/sqlite3/schema.py b/django/db/backends/sqlite3/schema.py index b7a945080d..31d72f6537 100644 --- a/django/db/backends/sqlite3/schema.py +++ b/django/db/backends/sqlite3/schema.py @@ -1,5 +1,5 @@ import codecs - +import copy from decimal import Decimal from django.utils import six from django.apps.registry import Apps @@ -86,6 +86,12 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): 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, @@ -96,6 +102,7 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): 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) @@ -148,28 +155,8 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): 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)]) @@ -186,6 +173,17 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): 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 diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index 85fd42b60f..2366ef9ab8 100644 --- a/tests/migrations/test_operations.py +++ b/tests/migrations/test_operations.py @@ -36,12 +36,23 @@ class OperationTests(MigrationTestBase): 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: @@ -50,6 +61,10 @@ class OperationTests(MigrationTestBase): 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", @@ -66,6 +81,13 @@ class OperationTests(MigrationTestBase): ("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", @@ -537,6 +559,27 @@ class OperationTests(MigrationTestBase): 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)