Fixed #24109 -- Allowed RunSQL and RunPython operations to be elided.

Thanks to Markus Holtermann and Tim Graham for their review.

django/db/migrations/operations/base.py

@@ -30,6 +30,9 @@ class Operation(object):
     # DDL transaction support (i.e., does it have no DDL, like RunPython)
     atomic = False
 
+    # Should this operation be considered safe to elide and optimize across?
+    elidable = False
+
     serialization_expand_args = []
 
     def __new__(cls, *args, **kwargs):
@@ -117,6 +120,10 @@ class Operation(object):
         replaced with or a boolean that indicates whether or not the specified
         operation can be optimized across.
         """
+        if self.elidable:
+            return [operation]
+        elif operation.elidable:
+            return [self]
         return False
 
     def __repr__(self):

django/db/migrations/operations/fields.py

@@ -27,7 +27,10 @@ class FieldOperation(Operation):
         return self.is_same_model_operation(operation) and self.name_lower == operation.name_lower
 
     def reduce(self, operation, in_between, app_label=None):
-        return not operation.references_field(self.model_name, self.name, app_label)
+        return (
+            super(FieldOperation, self).reduce(operation, in_between, app_label=app_label) or
+            not operation.references_field(self.model_name, self.name, app_label)
+        )
 
 
 class AddField(FieldOperation):
@@ -333,4 +336,9 @@ class RenameField(FieldOperation):
                     operation.new_name,
                 ),
             ]
-        return not operation.references_field(self.model_name, self.new_name, app_label)
+        # Skip `FieldOperation.reduce` as we want to run `references_field`
+        # against self.new_name.
+        return (
+            super(FieldOperation, self).reduce(operation, in_between, app_label=app_label) or
+            not operation.references_field(self.model_name, self.new_name, app_label)
+        )

django/db/migrations/operations/models.py

@@ -21,7 +21,10 @@ class ModelOperation(Operation):
         return self.name.lower()
 
     def reduce(self, operation, in_between, app_label=None):
-        return not operation.references_model(self.name, app_label)
+        return (
+            super(ModelOperation, self).reduce(operation, in_between, app_label=app_label) or
+            not operation.references_model(self.name, app_label)
+        )
 
 
 class CreateModel(ModelOperation):
@@ -365,7 +368,12 @@ class RenameModel(ModelOperation):
                     operation.new_name,
                 ),
             ]
-        return not operation.references_model(self.new_name, app_label)
+        # Skip `ModelOperation.reduce` as we want to run `references_model`
+        # against self.new_name.
+        return (
+            super(ModelOperation, self).reduce(operation, in_between, app_label=app_label) or
+            not operation.references_model(self.new_name, app_label)
+        )
 
 
 class AlterModelTable(ModelOperation):

django/db/migrations/operations/special.py

@@ -71,11 +71,12 @@ class RunSQL(Operation):
     """
     noop = ''
 
-    def __init__(self, sql, reverse_sql=None, state_operations=None, hints=None):
+    def __init__(self, sql, reverse_sql=None, state_operations=None, hints=None, elidable=False):
         self.sql = sql
         self.reverse_sql = reverse_sql
         self.state_operations = state_operations or []
         self.hints = hints or {}
+        self.elidable = elidable
 
     def deconstruct(self):
         kwargs = {
@@ -138,7 +139,7 @@ class RunPython(Operation):
 
     reduces_to_sql = False
 
-    def __init__(self, code, reverse_code=None, atomic=True, hints=None):
+    def __init__(self, code, reverse_code=None, atomic=True, hints=None, elidable=False):
         self.atomic = atomic
         # Forwards code
         if not callable(code):
@@ -152,6 +153,7 @@ class RunPython(Operation):
                 raise ValueError("RunPython must be supplied with callable arguments")
             self.reverse_code = reverse_code
         self.hints = hints or {}
+        self.elidable = elidable
 
     def deconstruct(self):
         kwargs = {

docs/ref/migration-operations.txt

@@ -196,7 +196,7 @@ Special Operations
 RunSQL
 ------
 
-.. class:: RunSQL(sql, reverse_sql=None, state_operations=None, hints=None)
+.. class:: RunSQL(sql, reverse_sql=None, state_operations=None, hints=None, elidable=False)
 
 Allows running of arbitrary SQL on the database - useful for more advanced
 features of database backends that Django doesn't support directly, like
@@ -249,6 +249,9 @@ The optional ``hints`` argument will be passed as ``**hints`` to the
 routing decisions. See :ref:`topics-db-multi-db-hints` for more details on
 database hints.
 
+The optional ``elidable`` argument determines whether or not the operation will
+be removed (elided) when :ref:`squashing migrations <migration-squashing>`.
+
 .. attribute:: RunSQL.noop
 
     Pass the ``RunSQL.noop`` attribute to ``sql`` or ``reverse_sql`` when you
@@ -257,10 +260,14 @@ database hints.
 
 .. _sqlparse: https://pypi.python.org/pypi/sqlparse
 
+.. versionadded:: 1.10
+
+    The ``elidable`` argument was added.
+
 RunPython
 ---------
 
-.. class:: RunPython(code, reverse_code=None, atomic=True, hints=None)
+.. class:: RunPython(code, reverse_code=None, atomic=True, hints=None, elidable=False)
 
 Runs custom Python code in a historical context. ``code`` (and ``reverse_code``
 if supplied) should be callable objects that accept two arguments; the first is
@@ -278,6 +285,9 @@ The optional ``hints`` argument will be passed as ``**hints`` to the
 routing decision. See :ref:`topics-db-multi-db-hints` for more details on
 database hints.
 
+The optional ``elidable`` argument determines whether or not the operation will
+be removed (elided) when :ref:`squashing migrations <migration-squashing>`.
+
 You are advised to write the code as a separate function above the ``Migration``
 class in the migration file, and just pass it to ``RunPython``. Here's an
 example of using ``RunPython`` to create some initial objects on a ``Country``
@@ -366,6 +376,10 @@ attribute.
     you want the operation not to do anything in the given direction. This is
     especially useful in making the operation reversible.
 
+.. versionadded:: 1.10
+
+    The ``elidable`` argument was added.
+
 SeparateDatabaseAndState
 ------------------------
 

docs/releases/1.10.txt

@@ -239,6 +239,11 @@ Migrations
 
 * Added support for serialization of ``enum.Enum`` objects.
 
+* Added the ``elidable`` argument to the
+  :class:`~django.db.migrations.operations.RunSQL` and
+  :class:`~django.db.migrations.operations.RunPython` operations to allow them
+  to be removed when squashing migrations.
+
 Models
 ~~~~~~
 

docs/spelling_wordlist

@@ -230,6 +230,7 @@ dumpdata
 Dunck
 dwithin
 editability
+elidable
 encodings
 Endian
 endswith

tests/migrations/test_operations.py

@@ -1532,6 +1532,10 @@ class OperationTests(OperationTestBase):
         self.assertEqual(definition[0], "RunSQL")
         self.assertEqual(definition[1], [])
         self.assertEqual(sorted(definition[2]), ["reverse_sql", "sql", "state_operations"])
+        # And elidable reduction
+        self.assertIs(False, operation.reduce(operation, []))
+        elidable_operation = migrations.RunSQL('SELECT 1 FROM void;', elidable=True)
+        self.assertEqual(elidable_operation.reduce(operation, []), [operation])
 
     def test_run_sql_params(self):
         """
@@ -1705,6 +1709,10 @@ class OperationTests(OperationTestBase):
             operation.database_forwards("test_runpython", editor, project_state, new_state)
         self.assertEqual(project_state.apps.get_model("test_runpython", "Pony").objects.count(), 6)
         self.assertEqual(project_state.apps.get_model("test_runpython", "ShetlandPony").objects.count(), 2)
+        # And elidable reduction
+        self.assertIs(False, operation.reduce(operation, []))
+        elidable_operation = migrations.RunPython(inner_method, elidable=True)
+        self.assertEqual(elidable_operation.reduce(operation, []), [operation])
 
     def test_run_python_atomic(self):
         """

tests/migrations/test_optimizer.py

 # -*- coding: utf-8 -*-
 
 from django.db import migrations, models
+from django.db.migrations import operations
 from django.db.migrations.optimizer import MigrationOptimizer
 from django.test import SimpleTestCase
 
@@ -631,3 +632,22 @@ class OptimizerTests(SimpleTestCase):
                 migrations.CreateModel("Bar", [("width", models.IntegerField())]),
             ],
         )
+
+    def test_optimize_elidable_operation(self):
+        elidable_operation = operations.base.Operation()
+        elidable_operation.elidable = True
+        self.assertOptimizesTo(
+            [
+                elidable_operation,
+                migrations.CreateModel("Foo", [("name", models.CharField(max_length=255))]),
+                elidable_operation,
+                migrations.CreateModel("Bar", [("size", models.IntegerField())]),
+                elidable_operation,
+                migrations.RenameModel("Foo", "Phou"),
+                migrations.DeleteModel("Bar"),
+                elidable_operation,
+            ],
+            [
+                migrations.CreateModel("Phou", [("name", models.CharField(max_length=255))]),
+            ],
+        )