Draft of #18556 - .remove()/.add() should execute 1 query where possible

django/db/models/fields/related.py

@@ -517,11 +517,18 @@ def get_prefetch_query_set(self, instances):
                 return qs, rel_obj_attr, instance_attr, False, cache_name
 
             def add(self, *objs):
+                ids = []
                 for obj in objs:
                     if not isinstance(obj, self.model):
                         raise TypeError("'%s' instance expected, got %r" % (self.model._meta.object_name, obj))
                     setattr(obj, rel_field.name, self.instance)
-                    obj.save()
+                    if obj.pk is None:
+                        obj.save()
+                    else:
+                        ids.append(obj.pk)
+
+                if ids:
+                    self.model.objects.filter(pk__in=ids).update(**{rel_field.name: self.instance})
             add.alters_data = True
 
             def create(self, **kwargs):
@@ -542,13 +549,15 @@ def get_or_create(self, **kwargs):
             if rel_field.null:
                 def remove(self, *objs):
                     val = getattr(self.instance, attname)
+                    ids = []
                     for obj in objs:
                         # Is obj actually part of this descriptor set?
                         if getattr(obj, rel_field.attname) == val:
                             setattr(obj, rel_field.name, None)
-                            obj.save()
+                            ids.append(obj.pk)
                         else:
                             raise rel_field.rel.to.DoesNotExist("%r is not related to %r." % (obj, self.instance))
+                    self.filter(pk__in=ids).update(**{rel_field.name: None})
                 remove.alters_data = True
 
                 def clear(self):

docs/ref/models/relations.txt

@@ -44,6 +44,14 @@ Related objects reference
             >>> e = Entry.objects.get(id=234)
             >>> b.entry_set.add(e) # Associates Entry e with Blog b.
 
+        .. versionchanged:: 1.6
+
+        The ``add()`` method executes one query regardless of the number of
+        objects being added, (unless the objects have not yet been saved to the
+        database in which case that is done). In prior versions, it executed
+        two queries per object by calling the ``save()`` method of each object
+        being added.
+
     .. method:: create(**kwargs)
 
         Creates a new object, saves it and puts it in the related object set.
@@ -91,6 +99,12 @@ Related objects reference
         :class:`~django.db.models.ForeignKey` doesn't have ``null=True``, this
         is invalid.
 
+        .. versionchanged:: 1.6
+
+        The ``remove()`` method executes one query regardless of the number of
+        objects being removed. In prior versions, it executed two queries per
+        object by calling the ``save()`` method of each object being removed.
+
     .. method:: clear()
 
         Removes all objects from the related object set::

docs/releases/1.6.txt

@@ -147,6 +147,14 @@ Backwards incompatible changes in 1.6
     deprecation timeline for a given feature, its removal may appear as a
     backwards incompatible change.
 
+*  The :meth:`~django.db.models.fields.related.RelatedManager.add` and
+   :meth:`~django.db.models.fields.related.RelatedManager.remove` methods on
+   a reverse foreign key now does one query regardless of the number of objects
+   added or removed rather than one query per object. The ``save()`` method of
+   the objects being removed is no longer called and thus
+   :data:`~django.db.models.signals.pre_save` and
+   :data:`~django.db.models.signals.post_save` signals are no longer sent.
+
 Features deprecated in 1.6
 ==========================
 

tests/modeltests/many_to_one_null/tests.py

@@ -86,10 +86,30 @@ def test_assign_clear_related_set(self):
         self.assertQuerysetEqual(Article.objects.filter(reporter__isnull=True),
                                  ['<Article: First>', '<Article: Fourth>'])
 
+    def test_add_efficiency(self):
+        r = Reporter.objects.create()
+        articles = []
+        for _ in xrange(3):
+            articles.append(Article.objects.create())
+        with self.assertNumQueries(1):
+            r.article_set.add(*articles)
+        self.assertEqual(r.article_set.count(), 3)
+
     def test_clear_efficiency(self):
         r = Reporter.objects.create()
         for _ in range(3):
             r.article_set.create()
         with self.assertNumQueries(1):
             r.article_set.clear()
         self.assertEqual(r.article_set.count(), 0)
+
+    def test_remove_efficiency(self):
+        r = Reporter.objects.create()
+        articles = []
+        for _ in xrange(3):
+            articles.append(r.article_set.create())
+        with self.assertNumQueries(1):
+            r.article_set.remove(*articles)
+        self.assertEqual(r.article_set.count(), 0)
+        for article in articles:
+            self.assertEqual(article.reporter, None)