Added a bulk option to RelatedManager remove() and clear() methods

Refs #21169

6 files changed, 90 insertions, 37 deletions

diff --git a/django/contrib/contenttypes/generic.py b/django/contrib/contenttypes/generic.py
index 469e64db33..9ef499bb16 100644
--- a/django/contrib/contenttypes/generic.py
+++ b/django/contrib/contenttypes/generic.py
@@ -8,7 +8,7 @@ from functools import partial
 
 from django.core.exceptions import ObjectDoesNotExist
 from django.db import connection
-from django.db import models, router, DEFAULT_DB_ALIAS
+from django.db import models, router, transaction, DEFAULT_DB_ALIAS
 from django.db.models import signals
 from django.db.models.fields.related import ForeignObject, ForeignObjectRel
 from django.db.models.related import PathInfo
@@ -382,16 +382,29 @@ def create_generic_related_manager(superclass):
                 obj.save()
         add.alters_data = True
 
-        def remove(self, *objs):
-            db = router.db_for_write(self.model, instance=self.instance)
-            self.using(db).filter(pk__in=[o.pk for o in objs]).delete()
+        def remove(self, *objs, **kwargs):
+            if not objs:
+                return
+            bulk = kwargs.pop('bulk', True)
+            self._clear(self.filter(pk__in=[o.pk for o in objs]), bulk)
         remove.alters_data = True
 
-        def clear(self):
-            db = router.db_for_write(self.model, instance=self.instance)
-            self.using(db).delete()
+        def clear(self, **kwargs):
+            bulk = kwargs.pop('bulk', True)
+            self._clear(self, bulk)
         clear.alters_data = True
 
+        def _clear(self, queryset, bulk):
+            db = router.db_for_write(self.model, instance=self.instance)
+            queryset = queryset.using(db)
+            if bulk:
+                queryset.delete()
+            else:
+                with transaction.commit_on_success_unless_managed(using=db, savepoint=False):
+                    for obj in queryset:
+                        obj.delete()
+        _clear.alters_data = True
+
         def create(self, **kwargs):
             kwargs[self.content_type_field_name] = self.content_type
             kwargs[self.object_id_field_name] = self.pk_val
diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py
index 62c7248de2..4a945e0f45 100644
--- a/django/db/models/fields/related.py
+++ b/django/db/models/fields/related.py
@@ -463,13 +463,11 @@ def create_foreign_related_manager(superclass, rel_field, rel_model):
 
         # remove() and clear() are only provided if the ForeignKey can have a value of null.
         if rel_field.null:
-            def remove(self, *objs):
-                # If there aren't any objects, there is nothing to do.
+            def remove(self, *objs, **kwargs):
                 if not objs:
                     return
-
+                bulk = kwargs.pop('bulk', True)
                 val = rel_field.get_foreign_related_value(self.instance)
-
                 old_ids = set()
                 for obj in objs:
                     # Is obj actually part of this descriptor set?
@@ -477,14 +475,26 @@ def create_foreign_related_manager(superclass, rel_field, rel_model):
                         old_ids.add(obj.pk)
                     else:
                         raise rel_field.rel.to.DoesNotExist("%r is not related to %r." % (obj, self.instance))
-
-                self.filter(pk__in=old_ids).update(**{rel_field.name: None})
+                self._clear(self.filter(pk__in=old_ids), bulk)
             remove.alters_data = True
 
-            def clear(self):
-                self.update(**{rel_field.name: None})
+            def clear(self, **kwargs):
+                bulk = kwargs.pop('bulk', True)
+                self._clear(self, bulk)
             clear.alters_data = True
 
+            def _clear(self, queryset, bulk):
+                db = router.db_for_write(self.model, instance=self.instance)
+                queryset = queryset.using(db)
+                if bulk:
+                    queryset.update(**{rel_field.name: None})
+                else:
+                    with transaction.commit_on_success_unless_managed(using=db, savepoint=False):
+                        for obj in queryset:
+                            setattr(obj, rel_field.name, None)
+                            obj.save()
+            _clear.alters_data = True
+
     return RelatedManager
 
 
@@ -657,6 +667,7 @@ def create_many_related_manager(superclass, rel):
             signals.m2m_changed.send(sender=self.through, action="pre_clear",
                 instance=self.instance, reverse=self.reverse,
                 model=self.model, pk_set=None, using=db)
+
             filters = self._build_remove_filters(super(ManyRelatedManager, self).get_queryset().using(db))
             self.through._default_manager.using(db).filter(filters).delete()
 
@@ -746,8 +757,6 @@ def create_many_related_manager(superclass, rel):
             # source_field_name: the PK colname in join table for the source object
             # target_field_name: the PK colname in join table for the target object
             # *objs - objects to remove
-
-            # If there aren't any objects, there is nothing to do.
             if not objs:
                 return
 
diff --git a/django/db/models/query.py b/django/db/models/query.py
index 59a2b45d02..1737626d16 100644
--- a/django/db/models/query.py
+++ b/django/db/models/query.py
@@ -1030,7 +1030,7 @@ class QuerySet(object):
         """
         Checks if this QuerySet has any filtering going on. Note that this
         isn't equivalent for checking if all objects are present in results,
-        for example qs[1:]._has_filters() -> True.
+        for example qs[1:]._has_filters() -> False.
         """
         return self.query.has_filters()
 
diff --git a/docs/ref/models/relations.txt b/docs/ref/models/relations.txt
index 2fca954676..0c47bc4047 100644
--- a/docs/ref/models/relations.txt
+++ b/docs/ref/models/relations.txt
@@ -110,6 +110,17 @@ Related objects reference
         the ``blog`` :class:`~django.db.models.ForeignKey` doesn't have
         ``null=True``, this is invalid.
 
+        .. versionchanged 1.7::
+
+        For :class:`~django.db.models.ForeignKey` objects, this method accepts
+        a ``bulk`` argument to control how to perform the operation.
+        If ``True`` (the default), ``QuerySet.update()`` is used.
+        If ``bulk=False``, the ``save()`` method of each individual model
+        instance is called instead. This triggers the
+        :data:`~django.db.models.signals.pre_save` and
+        :data:`~django.db.models.signals.post_save` signals and comes at the
+        expense of performance.
+
     .. method:: clear()
 
         Removes all objects from the related object set::
@@ -121,7 +132,8 @@ Related objects reference
         them.
 
         Just like ``remove()``, ``clear()`` is only available on
-        :class:`~django.db.models.ForeignKey`\s where ``null=True``.
+        :class:`~django.db.models.ForeignKey`\s where ``null=True`` and it also
+        accepts the ``bulk`` keyword argument.
 
     .. note::
 
diff --git a/docs/releases/1.7.txt b/docs/releases/1.7.txt
index a453301999..f8cdcf1f3b 100644
--- a/docs/releases/1.7.txt
+++ b/docs/releases/1.7.txt
@@ -430,6 +430,11 @@ Models
 * :class:`F expressions <django.db.models.F>` support the power operator
   (``**``).
 
+* The ``remove()`` and ``clear()`` methods of the related managers created by
+  ``ForeignKey`` and ``GenericForeignKey`` now accept the ``bulk`` keyword
+  argument to control whether or not to perform operations in bulk
+  (i.e. using ``QuerySet.update()``). Defaults to ``True``.
+
 Signals
 ^^^^^^^
 
@@ -589,11 +594,13 @@ Fixing the issues introduced some backward incompatible changes:
 - The default implementation of ``remove()`` for ``ForeignKey`` related managers
   changed from a series of ``Model.save()`` calls to a single
   ``QuerySet.update()`` call. The change means that ``pre_save`` and
-  ``post_save`` signals aren't sent anymore.
+  ``post_save`` signals aren't sent anymore. You can use the ``bulk=False``
+  keyword argument to revert to the previous behavior.
 
 - The ``remove()`` and ``clear()`` methods for ``GenericForeignKey`` related
   managers now perform bulk delete. The ``Model.delete()`` method isn't called
-  on each instance anymore.
+  on each instance anymore. You can use the ``bulk=False`` keyword argument to
+  revert to the previous behavior.
 
 - The ``remove()`` and ``clear()`` methods for ``ManyToManyField`` related
   managers perform nested queries when filtering is involved, which may or
diff --git a/tests/custom_managers/tests.py b/tests/custom_managers/tests.py
index af36110eca..440274e0e0 100644
--- a/tests/custom_managers/tests.py
+++ b/tests/custom_managers/tests.py
@@ -226,11 +226,11 @@ class CustomManagerTests(TestCase):
             ordered=False,
         )
 
-    def test_removal_through_default_fk_related_manager(self):
+    def test_removal_through_default_fk_related_manager(self, bulk=True):
         bugs = FunPerson.objects.create(first_name="Bugs", last_name="Bunny", fun=True, favorite_book=self.b1)
         droopy = FunPerson.objects.create(first_name="Droopy", last_name="Dog", fun=False, favorite_book=self.b1)
 
-        self.b1.fun_people_favorite_books.remove(droopy)
+        self.b1.fun_people_favorite_books.remove(droopy, bulk=bulk)
         self.assertQuerysetEqual(
             FunPerson._base_manager.filter(favorite_book=self.b1), [
                 "Bugs",
@@ -240,7 +240,7 @@ class CustomManagerTests(TestCase):
             ordered=False,
         )
 
-        self.b1.fun_people_favorite_books.remove(bugs)
+        self.b1.fun_people_favorite_books.remove(bugs, bulk=bulk)
         self.assertQuerysetEqual(
             FunPerson._base_manager.filter(favorite_book=self.b1), [
                 "Droopy",
@@ -251,7 +251,7 @@ class CustomManagerTests(TestCase):
         bugs.favorite_book = self.b1
         bugs.save()
 
-        self.b1.fun_people_favorite_books.clear()
+        self.b1.fun_people_favorite_books.clear(bulk=bulk)
         self.assertQuerysetEqual(
             FunPerson._base_manager.filter(favorite_book=self.b1), [
                 "Droopy",
@@ -260,12 +260,15 @@ class CustomManagerTests(TestCase):
             ordered=False,
         )
 
-    def test_removal_through_specified_fk_related_manager(self):
+    def test_slow_removal_through_default_fk_related_manager(self):
+        self.test_removal_through_default_fk_related_manager(bulk=False)
+
+    def test_removal_through_specified_fk_related_manager(self, bulk=True):
         Person.objects.create(first_name="Bugs", last_name="Bunny", fun=True, favorite_book=self.b1)
         droopy = Person.objects.create(first_name="Droopy", last_name="Dog", fun=False, favorite_book=self.b1)
 
         # Check that the fun manager DOESN'T remove boring people.
-        self.b1.favorite_books(manager='fun_people').remove(droopy)
+        self.b1.favorite_books(manager='fun_people').remove(droopy, bulk=bulk)
         self.assertQuerysetEqual(
             self.b1.favorite_books(manager='boring_people').all(), [
                 "Droopy",
@@ -274,7 +277,7 @@ class CustomManagerTests(TestCase):
             ordered=False,
         )
         # Check that the boring manager DOES remove boring people.
-        self.b1.favorite_books(manager='boring_people').remove(droopy)
+        self.b1.favorite_books(manager='boring_people').remove(droopy, bulk=bulk)
         self.assertQuerysetEqual(
             self.b1.favorite_books(manager='boring_people').all(), [
             ],
@@ -285,7 +288,7 @@ class CustomManagerTests(TestCase):
         droopy.save()
 
         # Check that the fun manager ONLY clears fun people.
-        self.b1.favorite_books(manager='fun_people').clear()
+        self.b1.favorite_books(manager='fun_people').clear(bulk=bulk)
         self.assertQuerysetEqual(
             self.b1.favorite_books(manager='boring_people').all(), [
                 "Droopy",
@@ -300,11 +303,14 @@ class CustomManagerTests(TestCase):
             ordered=False,
         )
 
-    def test_removal_through_default_gfk_related_manager(self):
+    def test_slow_removal_through_specified_fk_related_manager(self):
+        self.test_removal_through_specified_fk_related_manager(bulk=False)
+
+    def test_removal_through_default_gfk_related_manager(self, bulk=True):
         bugs = FunPerson.objects.create(first_name="Bugs", last_name="Bunny", fun=True, favorite_thing=self.b1)
         droopy = FunPerson.objects.create(first_name="Droopy", last_name="Dog", fun=False, favorite_thing=self.b1)
 
-        self.b1.fun_people_favorite_things.remove(droopy)
+        self.b1.fun_people_favorite_things.remove(droopy, bulk=bulk)
         self.assertQuerysetEqual(
             FunPerson._base_manager.order_by('first_name').filter(favorite_thing_id=self.b1.pk), [
                 "Bugs",
@@ -314,7 +320,7 @@ class CustomManagerTests(TestCase):
             ordered=False,
         )
 
-        self.b1.fun_people_favorite_things.remove(bugs)
+        self.b1.fun_people_favorite_things.remove(bugs, bulk=bulk)
         self.assertQuerysetEqual(
             FunPerson._base_manager.order_by('first_name').filter(favorite_thing_id=self.b1.pk), [
                 "Droopy",
@@ -325,7 +331,7 @@ class CustomManagerTests(TestCase):
         bugs.favorite_book = self.b1
         bugs.save()
 
-        self.b1.fun_people_favorite_things.clear()
+        self.b1.fun_people_favorite_things.clear(bulk=bulk)
         self.assertQuerysetEqual(
             FunPerson._base_manager.order_by('first_name').filter(favorite_thing_id=self.b1.pk), [
                 "Droopy",
@@ -334,12 +340,15 @@ class CustomManagerTests(TestCase):
             ordered=False,
         )
 
-    def test_removal_through_specified_gfk_related_manager(self):
+    def test_slow_removal_through_default_gfk_related_manager(self):
+        self.test_removal_through_default_gfk_related_manager(bulk=False)
+
+    def test_removal_through_specified_gfk_related_manager(self, bulk=True):
         Person.objects.create(first_name="Bugs", last_name="Bunny", fun=True, favorite_thing=self.b1)
         droopy = Person.objects.create(first_name="Droopy", last_name="Dog", fun=False, favorite_thing=self.b1)
 
         # Check that the fun manager DOESN'T remove boring people.
-        self.b1.favorite_things(manager='fun_people').remove(droopy)
+        self.b1.favorite_things(manager='fun_people').remove(droopy, bulk=bulk)
         self.assertQuerysetEqual(
             self.b1.favorite_things(manager='boring_people').all(), [
                 "Droopy",
@@ -349,7 +358,7 @@ class CustomManagerTests(TestCase):
         )
 
         # Check that the boring manager DOES remove boring people.
-        self.b1.favorite_things(manager='boring_people').remove(droopy)
+        self.b1.favorite_things(manager='boring_people').remove(droopy, bulk=bulk)
         self.assertQuerysetEqual(
             self.b1.favorite_things(manager='boring_people').all(), [
             ],
@@ -360,7 +369,7 @@ class CustomManagerTests(TestCase):
         droopy.save()
 
         # Check that the fun manager ONLY clears fun people.
-        self.b1.favorite_things(manager='fun_people').clear()
+        self.b1.favorite_things(manager='fun_people').clear(bulk=bulk)
         self.assertQuerysetEqual(
             self.b1.favorite_things(manager='boring_people').all(), [
                 "Droopy",
@@ -375,6 +384,9 @@ class CustomManagerTests(TestCase):
             ordered=False,
         )
 
+    def test_slow_removal_through_specified_gfk_related_manager(self):
+        self.test_removal_through_specified_gfk_related_manager(bulk=False)
+
     def test_removal_through_default_m2m_related_manager(self):
         bugs = FunPerson.objects.create(first_name="Bugs", last_name="Bunny", fun=True)
         self.b1.fun_authors.add(bugs)