Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Improve migration optimizer to be able to optimize through other ops

  • Loading branch information...
commit 42f8666f6aed126d9606e00688087a76dd17afc6 1 parent 694d7da
@andrewgodwin andrewgodwin authored
View
12 django/db/migrations/operations/base.py
@@ -64,6 +64,18 @@ def describe(self):
"""
return "%s: %s" % (self.__class__.__name__, self._constructor_args)
+ def references_model(self, name, app_label=None):
+ """
+ Returns True if there is a chance this operation references the given
+ model name (as a string), with an optional app label for accuracy.
+
+ Used for optimization. If in doubt, return True;
+ returning a false positive will merely make the optimizer a little
+ less efficient, while returning a false negative may result in an
+ unusable optimized migration.
+ """
+ return True
+
def __repr__(self):
return "<%s %s%s>" % (
self.__class__.__name__,
View
30 django/db/migrations/operations/models.py
@@ -1,4 +1,5 @@
from .base import Operation
+from django.utils import six
from django.db import models, router
from django.db.models.options import normalize_unique_together
from django.db.migrations.state import ModelState
@@ -33,6 +34,23 @@ def database_backwards(self, app_label, schema_editor, from_state, to_state):
def describe(self):
return "Create model %s" % (self.name, )
+ def references_model(self, name, app_label=None):
+ strings_to_check = [self.name]
+ # Check we didn't inherit from the model
+ for base in self.bases:
+ if isinstance(base, six.string_types):
+ strings_to_check.append(base.split(".")[-1])
+ # Check we have no FKs/M2Ms with it
+ for fname, field in self.fields:
+ if field.rel:
+ if isinstance(field.rel.to, six.string_types):
+ strings_to_check.append(field.rel.to.split(".")[-1])
+ # Now go over all the strings and compare them
+ for string in strings_to_check:
+ if string.lower() == name.lower():
+ return True
+ return False
+
def __eq__(self, other):
return (
(self.__class__ == other.__class__) and
@@ -66,6 +84,9 @@ def database_backwards(self, app_label, schema_editor, from_state, to_state):
if router.allow_migrate(schema_editor.connection.alias, model):
schema_editor.create_model(model)
+ def references_model(self, name, app_label=None):
+ return name.lower() == self.name.lower()
+
def describe(self):
return "Delete model %s" % (self.name, )
@@ -97,6 +118,9 @@ def database_forwards(self, app_label, schema_editor, from_state, to_state):
def database_backwards(self, app_label, schema_editor, from_state, to_state):
return self.database_forwards(app_label, schema_editor, from_state, to_state)
+ def references_model(self, name, app_label=None):
+ return name.lower() == self.name.lower()
+
def describe(self):
return "Rename table for %s to %s" % (self.name, self.table)
@@ -131,6 +155,9 @@ def database_forwards(self, app_label, schema_editor, from_state, to_state):
def database_backwards(self, app_label, schema_editor, from_state, to_state):
return self.database_forwards(app_label, schema_editor, from_state, to_state)
+ def references_model(self, name, app_label=None):
+ return name.lower() == self.name.lower()
+
def describe(self):
return "Alter unique_together for %s (%s constraints)" % (self.name, len(self.unique_together))
@@ -164,5 +191,8 @@ def database_forwards(self, app_label, schema_editor, from_state, to_state):
def database_backwards(self, app_label, schema_editor, from_state, to_state):
return self.database_forwards(app_label, schema_editor, from_state, to_state)
+ def references_model(self, name, app_label=None):
+ return name.lower() == self.name.lower()
+
def describe(self):
return "Alter index_together for %s (%s constraints)" % (self.name, len(self.index_together))
View
25 django/db/migrations/optimizer.py
@@ -11,7 +11,7 @@ class MigrationOptimizer(object):
nothing.
"""
- def optimize(self, operations):
+ def optimize(self, operations, app_label=None):
"""
Main optimization entry point. Pass in a list of Operation instances,
get out a new list of Operation instances.
@@ -27,17 +27,20 @@ def optimize(self, operations):
The inner loop is run until the starting list is the same as the result
list, and then the result is returned. This means that operation
optimization must be stable and always return an equal or shorter list.
+
+ The app_label argument is optional, but if you pass it you'll get more
+ efficient optimization.
"""
# Internal tracking variable for test assertions about # of loops
self._iterations = 0
while True:
- result = self.optimize_inner(operations)
+ result = self.optimize_inner(operations, app_label)
self._iterations += 1
if result == operations:
return result
operations = result
- def optimize_inner(self, operations):
+ def optimize_inner(self, operations, app_label=None):
"""
Inner optimization loop.
"""
@@ -52,7 +55,7 @@ def optimize_inner(self, operations):
new_operations.extend(operations[i+1:i+1+j])
new_operations.extend(operations[i+j+2:])
return new_operations
- if not self.can_optimize_through(operation, other):
+ if not self.can_optimize_through(operation, other, app_label):
new_operations.append(operation)
break
else:
@@ -95,10 +98,22 @@ def reduce_model_alter_delete(self, operation, other):
#### THROUGH CHECKS ####
- def can_optimize_through(self, operation, other):
+ def can_optimize_through(self, operation, other, app_label=None):
"""
Returns True if it's possible to optimize 'operation' with something
the other side of 'other'. This is possible if, for example, they
affect different models.
"""
+ MODEL_LEVEL_OPERATIONS = (
+ migrations.CreateModel,
+ migrations.DeleteModel,
+ migrations.AlterModelTable,
+ migrations.AlterUniqueTogether,
+ migrations.AlterIndexTogether,
+ )
+ # If it's a model level operation, let it through if there's
+ # nothing that looks like a reference to us in 'other'.
+ if isinstance(operation, MODEL_LEVEL_OPERATIONS):
+ if not other.references_model(operation.name, app_label):
+ return True
return False
View
62 tests/migrations/test_optimizer.py
@@ -93,3 +93,65 @@ def test_create_alter_delete_model(self):
],
[],
)
+
+ def test_optimize_through_create(self):
+ """
+ We should be able to optimize away create/delete through a create or delete
+ of a different model, but only if the create operation does not mention the model
+ at all.
+ """
+ # These should work
+ self.assertOptimizesTo(
+ [
+ migrations.CreateModel("Foo", [("name", models.CharField(max_length=255))]),
+ migrations.CreateModel("Bar", [("size", models.IntegerField())]),
+ migrations.DeleteModel("Foo"),
+ ],
+ [
+ migrations.CreateModel("Bar", [("size", models.IntegerField())]),
+ ],
+ )
+ self.assertOptimizesTo(
+ [
+ migrations.CreateModel("Foo", [("name", models.CharField(max_length=255))]),
+ migrations.CreateModel("Bar", [("size", models.IntegerField())]),
+ migrations.DeleteModel("Bar"),
+ migrations.DeleteModel("Foo"),
+ ],
+ [],
+ )
+ self.assertOptimizesTo(
+ [
+ migrations.CreateModel("Foo", [("name", models.CharField(max_length=255))]),
+ migrations.CreateModel("Bar", [("size", models.IntegerField())]),
+ migrations.DeleteModel("Foo"),
+ migrations.DeleteModel("Bar"),
+ ],
+ [],
+ )
+ # This should not work - FK should block it
+ self.assertOptimizesTo(
+ [
+ migrations.CreateModel("Foo", [("name", models.CharField(max_length=255))]),
+ migrations.CreateModel("Bar", [("other", models.ForeignKey("testapp.Foo"))]),
+ migrations.DeleteModel("Foo"),
+ ],
+ [
+ migrations.CreateModel("Foo", [("name", models.CharField(max_length=255))]),
+ migrations.CreateModel("Bar", [("other", models.ForeignKey("testapp.Foo"))]),
+ migrations.DeleteModel("Foo"),
+ ],
+ )
+ # This should not work - bases should block it
+ self.assertOptimizesTo(
+ [
+ migrations.CreateModel("Foo", [("name", models.CharField(max_length=255))]),
+ migrations.CreateModel("Bar", [("size", models.IntegerField())], bases=("testapp.Foo", )),
+ migrations.DeleteModel("Foo"),
+ ],
+ [
+ migrations.CreateModel("Foo", [("name", models.CharField(max_length=255))]),
+ migrations.CreateModel("Bar", [("size", models.IntegerField())], bases=("testapp.Foo", )),
+ migrations.DeleteModel("Foo"),
+ ],
+ )
Please sign in to comment.
Something went wrong with that request. Please try again.