Improve content_ids handling by defered loading - #7867
Conversation
10d9549 to
0324f58
Compare
0324f58 to
be754c7
Compare
mdellweg
changed the title
pedro-psb
left a comment
pedro-psb
left a comment
There was a problem hiding this comment.
I was having a look into this, I think we could avoid hydrating content_ids for other methods as well, like added, removed and contains.
be754c7 to
bad7241
Compare
The array_field can be huge and most of the time we don't need the values in python. Some uses can even be handled by clever SQL without ever channelling all the data through the network.
bad7241 to
7c3b937
Compare
|
We have some overlapping changes here... Would you prefer that you merge this in first and then I rebase on the changes or should I undo my changes related to the deferment and have you rebase instead? |
|
|
||
| def get_queryset(self): | ||
| # Prevent the content_ids to be automatically hydrated. | ||
| return super().get_queryset().defer("content_ids") |
There was a problem hiding this comment.
get_queryset() is a Manager method, not a QuerySet method, so I don't think this would do anything
You might have meant to implement a manager
https://docs.djangoproject.com/en/6.0/topics/db/managers/#modifying-a-manager-s-initial-queryset
There was a problem hiding this comment.
RepositoryVersion uses objects = RepositoryVersionQuerySet.as_manager()
| return Content.objects.filter(pk__in=base_version.content_ids).exclude( | ||
| pk__in=self.content_ids | ||
| ) | ||
| return Content.objects.filter(pk__in=base_version.content).exclude(pk__in=self.content) |
There was a problem hiding this comment.
These are now double-nested subqueries again, I feel like that might have been part of the motivation for the original change?
| return Content.objects.filter(pk__in=self.content_ids).exclude( | ||
| pk__in=base_version.content_ids | ||
| ) | ||
| return Content.objects.filter(pk__in=self.content).exclude(pk__in=base_version.content) |
There was a problem hiding this comment.
If there are any instances where we are calling content.pk in rv.content_ids across different RVs, this might create N+1. I don't know that we do, just calling it out.
4 participants
The array_field can be huge and most of the time we don't need the values in python. Some uses can even be handled by clever SQL without ever channelling all the data through the network.
📜 Checklist
See: Pull Request Walkthrough