Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #30493 -- Fixed prefetch_related() for GenericRelation with different content types. #11423

Merged
merged 2 commits into from May 31, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 27 additions & 14 deletions django/contrib/contenttypes/fields.py
@@ -1,3 +1,6 @@
import functools
import itertools
import operator
from collections import defaultdict

from django.contrib.contenttypes.models import ContentType
Expand Down Expand Up @@ -515,17 +518,18 @@ def __init__(self, instance=None):
self.instance = instance

self.model = rel.model

content_type = ContentType.objects.db_manager(instance._state.db).get_for_model(
instance, for_concrete_model=rel.field.for_concrete_model)
self.content_type = content_type
self.get_content_type = functools.partial(
ContentType.objects.db_manager(instance._state.db).get_for_model,
for_concrete_model=rel.field.for_concrete_model,
)
self.content_type = self.get_content_type(instance)
self.content_type_field_name = rel.field.content_type_field_name
self.object_id_field_name = rel.field.object_id_field_name
self.prefetch_cache_name = rel.field.attname
self.pk_val = instance.pk

self.core_filters = {
'%s__pk' % self.content_type_field_name: content_type.id,
'%s__pk' % self.content_type_field_name: self.content_type.id,
self.object_id_field_name: self.pk_val,
}

Expand Down Expand Up @@ -564,19 +568,28 @@ def get_prefetch_queryset(self, instances, queryset=None):

queryset._add_hints(instance=instances[0])
queryset = queryset.using(queryset._db or self._db)

query = {
'%s__pk' % self.content_type_field_name: self.content_type.id,
'%s__in' % self.object_id_field_name: {obj.pk for obj in instances}
}

# Group instances by content types.
content_type_queries = (
models.Q(**{
'%s__pk' % self.content_type_field_name: content_type_id,
'%s__in' % self.object_id_field_name: {obj.pk for obj in objs}
})
for content_type_id, objs in itertools.groupby(
sorted(instances, key=lambda obj: self.get_content_type(obj).pk),
lambda obj: self.get_content_type(obj).pk,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems rather inefficient... Doesn't self.get_content_type(obj) execute a query?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pope1ni no, it's heavily cached as it's using ContentType.objects.get_for_model.

)
)
query = functools.reduce(operator.or_, content_type_queries)
# We (possibly) need to convert object IDs to the type of the
# instances' PK in order to match up instances:
object_id_converter = instances[0]._meta.pk.to_python
return (
queryset.filter(**query),
lambda relobj: object_id_converter(getattr(relobj, self.object_id_field_name)),
lambda obj: obj.pk,
queryset.filter(query),
lambda relobj: (
object_id_converter(getattr(relobj, self.object_id_field_name)),
relobj.content_type_id
),
lambda obj: (obj.pk, self.get_content_type(obj).pk),
False,
self.prefetch_cache_name,
False,
Expand Down
18 changes: 18 additions & 0 deletions tests/generic_relations/tests.py
Expand Up @@ -546,6 +546,24 @@ def test_add_then_remove_after_prefetch(self):
platypus.tags.remove(weird_tag)
self.assertSequenceEqual(platypus.tags.all(), [furry_tag])

def test_prefetch_related_different_content_types(self):
TaggedItem.objects.create(content_object=self.platypus, tag='prefetch_tag_1')
TaggedItem.objects.create(
content_object=Vegetable.objects.create(name='Broccoli'),
tag='prefetch_tag_2',
)
TaggedItem.objects.create(
content_object=Animal.objects.create(common_name='Bear'),
tag='prefetch_tag_3',
)
qs = TaggedItem.objects.filter(
tag__startswith='prefetch_tag_',
).prefetch_related('content_object', 'content_object__tags')
with self.assertNumQueries(4):
tags = list(qs)
for tag in tags:
self.assertSequenceEqual(tag.content_object.tags.all(), [tag])


class ProxyRelatedModelTest(TestCase):
def test_default_behavior(self):
Expand Down