Commit 5a9338ae by Diederik van der Boor

Optimize the query in `bulk_lookup_kwargs` for QuerySet args.

When passing a QuerySet to bulk_lookup_kwargs(), the queryset was
undesirably executed to get a list of IDs. This causes an unnecessary
performance loss.

There is no need to loop over a QuerySet to execute a IN (1, 3, 4) query.
Instead, just allow the ORM and database to execute a IN (SELECT id FROM ..) query.
In a real life app with a large set of objects, the difference in performance is noticeable.
parent 36f6dabc
...@@ -2,6 +2,7 @@ import django ...@@ -2,6 +2,7 @@ import django
from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes.models import ContentType
from django.contrib.contenttypes.generic import GenericForeignKey from django.contrib.contenttypes.generic import GenericForeignKey
from django.db import models, IntegrityError, transaction from django.db import models, IntegrityError, transaction
from django.db.models.query import QuerySet
from django.template.defaultfilters import slugify as default_slugify from django.template.defaultfilters import slugify as default_slugify
from django.utils.translation import ugettext_lazy as _, ugettext from django.utils.translation import ugettext_lazy as _, ugettext
...@@ -137,11 +138,18 @@ class GenericTaggedItemBase(ItemBase): ...@@ -137,11 +138,18 @@ class GenericTaggedItemBase(ItemBase):
@classmethod @classmethod
def bulk_lookup_kwargs(cls, instances): def bulk_lookup_kwargs(cls, instances):
# TODO: instances[0], can we assume there are instances. if isinstance(instances, QuerySet):
return { # Can do a real object_id IN (SELECT ..) query.
"object_id__in": [instance.pk for instance in instances], return {
"content_type": ContentType.objects.get_for_model(instances[0]), "object_id__in": instances,
} "content_type": ContentType.objects.get_for_model(instances.model),
}
else:
# TODO: instances[0], can we assume there are instances.
return {
"object_id__in": [instance.pk for instance in instances],
"content_type": ContentType.objects.get_for_model(instances[0]),
}
@classmethod @classmethod
def tags_for(cls, model, instance=None): def tags_for(cls, model, instance=None):
......
...@@ -215,6 +215,27 @@ class TaggableManagerTestCase(BaseTaggingTestCase): ...@@ -215,6 +215,27 @@ class TaggableManagerTestCase(BaseTaggingTestCase):
[kitty.pk, cat.pk] [kitty.pk, cat.pk]
) )
def test_lookup_bulk(self):
apple = self.food_model.objects.create(name="apple")
pear = self.food_model.objects.create(name="pear")
apple.tags.add('fruit', 'green')
pear.tags.add('fruit', 'yummie')
def lookup_qs():
# New fix: directly allow WHERE object_id IN (SELECT id FROM ..)
objects = self.food_model.objects.all()
lookup = self.taggeditem_model.bulk_lookup_kwargs(objects)
list(self.taggeditem_model.objects.filter(**lookup))
def lookup_list():
# Simulate old situation: iterate over a list.
objects = list(self.food_model.objects.all())
lookup = self.taggeditem_model.bulk_lookup_kwargs(objects)
list(self.taggeditem_model.objects.filter(**lookup))
self.assert_num_queries(1, lookup_qs)
self.assert_num_queries(2, lookup_list)
def test_exclude(self): def test_exclude(self):
apple = self.food_model.objects.create(name="apple") apple = self.food_model.objects.create(name="apple")
apple.tags.add("red", "green", "delicious") apple.tags.add("red", "green", "delicious")
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment