-
Notifications
You must be signed in to change notification settings - Fork 300
-
Status quo
Currently the get_queryset simply adopted the defined prefetch_related and select_related definitions. But there are still some existing bottlenecks:
Scenario: No include parameter
Given is: The used serializer has a ResourceRelatedField and no include parameter is used.
For that case no predefinied prefetch_related definition is used.
But the ResourceRelatedField needs all ids from the relation, so it will trigger the db to get it.
Solution: No include paramter
My current solution is to define conditional prefetch_related definitions inside the get_queryset() on my views.
Something like
def get_queryset(self): qs = super().get_queryset() include = self.request.GET.get("include", None) if not include or "fieldName" not in include: qs = qs.prefetch_related(Prefetch("field_name", queryset=Model.objects.only('id')) return qs
Without the prefetch_related definition it leads into the described m*(n+1) problems
But there is another point. The code below prefetches in any case. But if the api is called with sparse fieldsets and the related field is not part of the request, the prefetching is not needed.
So it need another logic to get the fields parameters and analyze them against the prefetching logic. Something like:
fields = self.request.GET.get("fields[ResourceName]", "").split(',') if fields and "fieldName" in fields: # do prefetching else: # do not add default prefetching
So i think this is a general problem which hits every view with a ResourceRelatedField, for that it would be worth it to add some logic to the core package to handle this.
Beta Was this translation helpful? Give feedback.
All reactions
Replies: 1 comment
-
Thanks for bringing this up again. Indeed the prefetching implementation does not cover all use cases. There has been a discussion at #921 about it and also an attempt to implement it at #964 but got never finished. I am very open for improvements in this area but to get this right we most likely need a different approach as suggested in the above discussion than the two mixins we already have for this.
Beta Was this translation helpful? Give feedback.