-
Notifications
You must be signed in to change notification settings - Fork 159
If order_with_respect_to is a foreign key of a foreign key it generates aditional queries. #293
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
Comments
I'm aiming for it not be an additional query because those fk values have already been fetched from the DB when the object was queried. This does not currently hold with a 'double underscore' relation, which looks to be your case. We spoke about how to move forward during review of that PR. It seems to me that there should be a modification to the At the moment the admin does multiple subqueries whilst constructing a similar, but different, map of the ordered set for use in constructing the admin change links. It is with optimising this that I do not want to defer the lookup of order fields until save occurs on the model. |
@shuckc, i'm personally always cautious when using For @rafammpp 's problem, i think this can be solved by annotating the values on the queryset, top of mind here's how i would implement it:
Here we are annotating the values of the related fields by default. Notes:
|
Yes that's the idea you mentioned previously, I put it together (using I annotated non-dunder FK's too, as that is presumably done at no cost and keeps the handling similar. |
You have a good point about needing a better fallback for During model |
* add failing test for django-ordered-model#293 * Query ForeignKey id values during load to optimise order_with_respect_to check during save() This favours a smaller overall query count over simpler queries. By adding attributes during evaluation of the queryset, we can store the prior value of ForeignKey relations, for the cost of a join for each entry within order_with_respect_to (nested joins if LOOKUP_SEPARATOR (__) is used). The fetched attibutes are not available during OrderedModel.__init__() so we defer creation of self._original_wrt_map until save(). Since this reads from attributes it no longer causes any additional queries. This seems to balance correctness in the case of modifying the fields you are ordered with respect to, against minimising extra queries during either get() or save() operations, and does not require the full related object to be fetched to detect the change. --------- Co-authored-by: Chris Shucksmith <chris@shucksmith.co.uk>
* add failing test for django-ordered-model#293 * Query ForeignKey id values during load to optimise order_with_respect_to check during save() This favours a smaller overall query count over simpler queries. By adding attributes during evaluation of the queryset, we can store the prior value of ForeignKey relations, for the cost of a join for each entry within order_with_respect_to (nested joins if LOOKUP_SEPARATOR (__) is used). The fetched attibutes are not available during OrderedModel.__init__() so we defer creation of self._original_wrt_map until save(). Since this reads from attributes it no longer causes any additional queries. This seems to balance correctness in the case of modifying the fields you are ordered with respect to, against minimising extra queries during either get() or save() operations, and does not require the full related object to be fetched to detect the change. --------- Co-authored-by: Chris Shucksmith <chris@shucksmith.co.uk>
Can we resurrect this? |
Uh oh!
There was an error while loading. Please reload this page.
Example
If I do a queryset of Articles, it make an aditional query for every article, even if I use
Article.objects.select_related('category__site')
.@ibaguio manage to fix this problem in c4630dc for a simple foreign key, but if you order with respect to a foreign key of a foreign key it makes a query to retrieve the category.
If I understand it well, you save the initial value of the wrt_map to the instance at init time to check for changes later on save, and then do the proper arragments if needed. wrt_map is a dict of every item in order_with_respect_to with it's respective values. But you only need it on save. My proposal is to get the saved value from the db, and then check if it's different from the current instance to be saved. Yes, we make an aditional query, but only when we need it (on save) and can be the cheapest query if we defer all values other than the pk.
I've a view that list articles. Currently it makes over a thousand queries, and with this patch implemented, or with the previous version of this package, it gets down to eight.
I'm gonna make a pull request with my proposal, see you there.
The text was updated successfully, but these errors were encountered: