django-polymorphic
django-polymorphic copied to clipboard
select_related and prefetch_related for inherited models
Regarding the documentation:
select_related() works just as usual, but it can not (yet) be used to select relations in inherited models (like ModelA.objects.select_related('ModelC___fieldxy') )
Is there a chance you will implement this? I mean select_related and prefetch_related for the inherited models, It would be really helpful. Because I don't know how to optimize some requests now. Thank you.
Thanks for the report! You've identified a spot where polymorphic isn't present yet. I'm not sure this can be solved; the .filter(ModelC___fieldxy=..) already is somewhat of a hack. I'm not sure we can hook into the select-related logic but you're welcome to give it a try - granted that you'l add unit tests for it.
also see #37 for a related issue.
I played around with select related in the child models, and figured I need to create a manager for the base_objects attribute on the child models, that performs a select_related for every queryset, like:
class ChildManager(models.Manager):
def get_queryset(self):
return super(ChildManager, self).get_queryset().select_related('some_field')
class ChildModel(ParentModel):
some_field = models.ForeignKey(...)
base_objects = ChildManager()
However the select_related config for the queryset on the childmodel will get overriden by this line: https://github.com/django-polymorphic/django-polymorphic/blob/master/polymorphic/query.py#L351
I think that if this line didn't exist, we would have a good chance so we can do select_related on child models.
I traced the history to 2010 and previous of this line. It seems to be there since the beginning. Could we do something like:
if not real_objects.query.select_related:
real_objects.query.select_related = self.query.select_related
So that we don't override the select related config here?
Are there any plans on supporting this and/or is any help needed? Supporting select_related and prefetch_related would greatly improve the performance of some of our code that uses polymorphic models.
I like the assignment of the select_related. We'll have to make sure though that it doesn't trigger Django errors when that field doesn't exist in a particular subclass. A well tested pull request on this is welcome!
Actually I think it shall trigger any Django errors if something is wrong. The real_objects queryset comes from the base_objects manager. And the base_objects is used specifically for retrieving objects of the subclass itself. So polymorphic shouldn't do any magic in this place.
You would make me the happiest man in the world if there was a solution to this. I have many relationships in child models and the number of queries is too high.
Thank you for your work @vdboor!
While working on #244, I uncovered something that might be useful, assume these models:
class Parent(PolymorphicModel):
pass
class ChildA(Parent):
name = models.CharField()
class ChildB(Parent):
height = models.IntegerField()
This works:
obj = Parent.objects.non_polymorphic().select_related('childa__name')
obj.childa.name # return the name!
Although this doesn't:
Parent.objects.select_related('childa__name')
# childa is an invalid field name here
So we clearly don't need to fiddle with django's internals to get this issue fixed. I believe that we can actually deal with the select_related when casting the polymorphic instances to their real child class. It's just that the API select_related(*fields) doesn't feel right. Maybe polymorphic querysets need something different, like a mapping of child_class: related_fields.
Project maintainers: my employer is interested in putting a bounty on this ticket, for supporting both select_related and prefetch_related for inherited models. Is that alright by you? Can we lock down the requirements, such that a PR is likely to be accepted? For example, deciding on the syntax of the lookup.
@vdboor ^^
FWIW, I'd chime in with a few dollars to a bounty for this -- I'd be able to make huge optimizations if this feature works. The same goes for #244.
I want to press the question that @smcoll asked here, and express my own strong interest in getting this problem solved.
Can we lock down the requirements, such that a PR is likely to be accepted? For example, deciding on the syntax of the lookup.
Frankly I don't have a problem with the syntax Parent.objects.select_related('childa__name'). Is there any use-case this would not address? Would a solution of this form be accepted?
I would love a solution for this as well... I have to find a solution for the relationships in my child models in order to get a reasonable performance...
Also interested in putting some dollars in a bounty. Please notice that we need to make this work also with relations that span from other models that are not polymorphic, like:
NonPolymorphicModel.objects.select_related('relation_to_polymorphic_base__childA__name')
Any updates on this? I would love to put in a bounty for this as well.
Maybe we can use something like Bountysource to actually make these bounties a reality?
There's plenty of us really wanting this implemented!
@vdboor what factors would need to be considered for a PR to be merged (like the lookup syntax)? That could be the scope of the bounty ticket. There has been plenty of interest in funding a solution- we just need coordination from a project maintainer so we know the funded work will be accepted into the codebase.
Ideally, the syntax should be the same as django's select_related, although it would only apply for subclasses that actually have the requested field.
Another alternative is to have something like:
queryset.select_polymorphic_related(
SubClassA,
'relation__to___subclass__a',
)
This allows more explicit checks that the field actually exists for some subclass.
Any updates on this? I'm currently having this issue.
EDIT: Nevermind, I think this addresses a different issue
Does this solve the problem for anyone? https://gist.github.com/Finndersen/3a9647718b35d6776b0dc3808a501519 Adds new select_polymorphic_related() Queryset method. Might look into having it handle it seamlessly in standard select_related()
Based on work by AndySun25 from this discussion: https://github.com/django-polymorphic/django-polymorphic/issues/244
By the way it appears doing a plain select_related() (without specifying fields) on the parent queryset works to select related instances on child classes, if this helps anyone
This limitation made me switch to using the InheritanceManager from django-model-utils instead. I found this walkthrough of the alternatives helpful: https://confuzeus.com/hub/django-web-framework/model-polymorphism/
https://github.com/django-polymorphic/django-polymorphic/pull/531
Seems like this guy solved this issue. Need more tests I think, but I've tested tons of cases and so far so good.
#531
Seems like this guy solved this issue. Need more tests I think, but I've tested tons of cases and so far so good.
if only this project wasn't abandoned...
edit:
This limitation made me switch to using the InheritanceManager from django-model-utils instead. I found this walkthrough of the alternatives helpful: https://confuzeus.com/hub/django-web-framework/model-polymorphism/
thanks for this. InheritanceManager might be the move
I was able to prefetch related polymorphic child subclass using Prefetch
Given this relations:
#models.py
class Respondent(PolymorphicModel):
name = models.CharField()
class ChildRespondent(Respondent):
extra = models.CharField()
class Answer(models.Model):
respondent = models.ForeignKey(Respondent)
from django.db.models import Prefetch
# Get answers together with ChildRespondent instances
for answer in Answer.objects.prefetch_related(Prefetch("respondent", queryset=ChildRespondent.objects.all(), to_attr="child_respondent"):
assert isinstance(answer.child_respondent, ChildRespondent)
Not the cleanest solution since polymorphism is lost, but at least the n+1 problem is solved.