Skip to content

Commit

Permalink
Fixed #22432: SQLite M2M repointing now works. Thanks to xelnor.
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewgodwin committed May 20, 2014
1 parent 125b3d4 commit 03900a0
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 24 deletions.
6 changes: 6 additions & 0 deletions django/db/backends/schema.py
Expand Up @@ -499,6 +499,12 @@ def alter_field(self, model, old_field, new_field, strict=False):
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
Expand Down
44 changes: 21 additions & 23 deletions 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
Expand Down Expand Up @@ -86,6 +86,12 @@ def _remake_table(self, model, create_fields=[], delete_fields=[], alter_fields=
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,
Expand All @@ -96,6 +102,7 @@ def _remake_table(self, model, create_fields=[], delete_fields=[], alter_fields=
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)
Expand Down Expand Up @@ -148,28 +155,8 @@ def remove_field(self, model, field):
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)])

Expand All @@ -186,6 +173,17 @@ def _alter_many_to_many(self, model, old_field, new_field, strict):
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
Expand Down
45 changes: 44 additions & 1 deletion tests/migrations/test_operations.py
Expand Up @@ -36,12 +36,23 @@ def unapply_operations(self, app_label, project_state, operations):
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:
Expand All @@ -50,6 +61,10 @@ def set_up_test_model(self, app_label, second_model=False, related_model=False,
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",
Expand All @@ -66,6 +81,13 @@ def set_up_test_model(self, app_label, second_model=False, related_model=False,
("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",
Expand Down Expand Up @@ -537,6 +559,27 @@ def test_alter_field_m2m(self):
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)

Expand Down

0 comments on commit 03900a0

Please sign in to comment.