#33651 closed New feature (fixed)
Support prefetch GenericForeignKey with custom queryset.
| Reported by: | elonzh | Owned by: | Clément Escolano |
|---|---|---|---|
| Component: | contrib.contenttypes | Version: | 4.0 |
| Severity: | Normal | Keywords: | GenericForeignKey |
| Cc: | David Wobrock, Todor Velichkov, Rohan Nahata, joeli, Clément Escolano | Triage Stage: | Ready for checkin |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
For example:
class Node(models.Model):
content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE)
object_id = models.PositiveIntegerField()
content_object = GenericForeignKey("content_type", "object_id")
class AbstractNodeItem(models.Model):
name = models.CharField(max_length=100)
class ItemA(AbstractNodeItem):
a_content = models.TextField()
class ItemB(AbstractNodeItem):
b_content = models.TextField()
Currently we can't list all node with only content_object.name prefetched.
Change History (30)
comment:1 by , 4 years ago
comment:2 by , 4 years ago
| Component: | Database layer (models, ORM) → contrib.contenttypes |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
comment:3 by , 4 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:5 by , 4 years ago
| Needs documentation: | set |
|---|
comment:6 by , 4 years ago
| Needs documentation: | unset |
|---|
comment:7 by , 4 years ago
| Needs documentation: | set |
|---|
comment:8 by , 4 years ago
| Needs documentation: | unset |
|---|
comment:9 by , 3 years ago
| Patch needs improvement: | set |
|---|
comment:10 by , 3 years ago
| Cc: | added |
|---|
comment:11 by , 3 years ago
| Patch needs improvement: | unset |
|---|
comment:12 by , 3 years ago
| Patch needs improvement: | set |
|---|
comment:13 by , 3 years ago
| Patch needs improvement: | unset |
|---|
comment:14 by , 3 years ago
#24272 was a duplicate. One thing I noticed while reviewing it is that the signature of GenericPrefetch would likely be ergonomic as GenericPrefetch(relation: str, querysets: list[QuerySet]) instead of querysets: dict[Model, QuerySet] since the model can be inferred from queryset.model anyway.
comment:15 by , 3 years ago
| Cc: | added |
|---|
comment:16 by , 3 years ago
Yes Simon. The change of querysets from dictionary to list is also suggested by Mariusz Felisiak. I have recently updated the PR with this change.
Thanks.
comment:18 by , 3 years ago
| Cc: | added |
|---|
follow-up: 20 comment:19 by , 3 years ago
Hi everyone, I took a brief look at the conversation in the pull request #15636 and I have a question about the issue being discussed there (passing list of querysets into an queryset argument).
Is there anything specific about this, that is enforcing the use of a list of querysets? Is it possible to send as many GenericPrefetch objects as the number of querysets being required ? For example:
Node.objects.prefetch_related(
GenericPrefetch("content_object", queryset=ItemA.objects.all()),
GenericPrefetch("content_object", queryset=ItemB.objects.all()),
)
this is in the spirit of the current Prefetch class, which actually allows you to prefetch multiple levels of objects, splitted into different prefetch calls. i.e:
Book.objects.prefetch_related(
Prefetch("author", queryset=Author.objects.all()),
Prefetch("author__house", queryset=House.objects.all()),
)
And now I'm a little bit repeating myself with ticket 24272, but this could probably open the door for reusing related_query_name when there is a defined GenericRelation which could give us the following interface w/o using the generic content_object argument:
TaggedItem.objects.all().prefetch_related(
GenericPrefetch('books', queryset=Book.objects.all().select_related('author')),
GenericPrefetch('movies', queryset=Movie.objects.all().select_related('director')),
)
comment:20 by , 3 years ago
Hi,
I think there is no way to add two lookups on same foreign key on same model as currently we populate the prefetch cache for this foreign key. Anyway, for a generic foreign key currently giving a queryset is not supported. To add this support I think the best way is to have a class that extends the prefetch class which handles all the custom logic of prefetching which is what we are working on in this issue.
To implement what you said i.e adding two Prefetch of content object in prefetch_related function. Some of the logic will need to be handled in the prefetch_related function which is not good.
Replying to Todor Velichkov:
Hi everyone, I took a brief look at the conversation in the pull request #15636 and I have a question about the issue being discussed there (passing list of querysets into an queryset argument).
Is there anything specific about this, that is enforcing the use of a list of querysets? Is it possible to send as many
GenericPrefetchobjects as the number of querysets being required ? For example:
Node.objects.prefetch_related( GenericPrefetch("content_object", queryset=ItemA.objects.all()), GenericPrefetch("content_object", queryset=ItemB.objects.all()), )this is in the spirit of the current
Prefetchclass, which actually allows you to prefetch multiple levels of objects, splitted into different prefetch calls. i.e:
Book.objects.prefetch_related( Prefetch("author", queryset=Author.objects.all()), Prefetch("author__house", queryset=House.objects.all()), )And now I'm a little bit repeating myself with ticket 24272, but this could probably open the door for reusing
related_query_namewhen there is a definedGenericRelationwhich could give us the following interface w/o using the genericcontent_objectargument:
TaggedItem.objects.all().prefetch_related( GenericPrefetch('books', queryset=Book.objects.all().select_related('author')), GenericPrefetch('movies', queryset=Movie.objects.all().select_related('director')), )
comment:21 by , 3 years ago
| Cc: | added |
|---|
comment:22 by , 2 years ago
| Cc: | added |
|---|
comment:24 by , 2 years ago
| Patch needs improvement: | unset |
|---|
comment:25 by , 2 years ago
| Needs documentation: | set |
|---|---|
| Needs tests: | set |
| Patch needs improvement: | set |
comment:26 by , 2 years ago
| Needs documentation: | unset |
|---|---|
| Needs tests: | unset |
| Patch needs improvement: | unset |
comment:27 by , 2 years ago
| Patch needs improvement: | set |
|---|
comment:28 by , 2 years ago
| Patch needs improvement: | unset |
|---|---|
| Triage Stage: | Accepted → Ready for checkin |
If I understand correctly you'd like to be able to have the prefetch querysets issued when doing
Node.objects.prefetch_related('content_object')forItemAandItemBbe.objects.only('name').I've had to implement such a feature in the past and ended up with writing my own
GenericForeignKeySubclassthat would allow for the followingIf we were to do a proper implementation here though I think that a
GenericPrefetch(Prefetch)subclass would make for a better API as that's the current way custom querysets are provided to the prefetching logic. Something alongAccepting on the basis that I think this would be a valuable feature.