Skip to content

Highlight select_related and prefetch_related usage. #1977

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

Closed
tomchristie opened this issue Oct 22, 2014 · 21 comments
Closed

Highlight select_related and prefetch_related usage. #1977

tomchristie opened this issue Oct 22, 2014 · 21 comments

Comments

@tomchristie
Copy link
Member

In both the generic view and serializer sections we need to make sure we're pointing out appropriate usage of select_related/prefetch_related. (To review: is there anything in there at the moment?)

Also see #1976.

@tomchristie tomchristie changed the title Call out select_related and prefetch_related in generic views / serializers. Call out select_related and prefetch_related usage. Oct 22, 2014
@tomchristie tomchristie changed the title Call out select_related and prefetch_related usage. Highlight select_related and prefetch_related usage. Oct 22, 2014
@GeoffThorpeFT
Copy link

I note this issue is still open. Does that imply that the documentation improvements have yet to be made?
If some draft or partial documentation is available please include a link.
Thanks

@xordoquy
Copy link
Collaborator

xordoquy commented Sep 6, 2016

As far as I can tell, someone needs to step up and do the work (tm)

@kevin-brown
Copy link
Member

I'm going to throw a note down here for whoever ends up doing this ticket: prefetch_related can only be used on read-only APIs. The reasoning has to do with #2442.

@spectras
Copy link

spectras commented Nov 4, 2016

@kevin-brown > Or at least the prefetched objects associated with the instance should not be modified.
Or your code updating the relations should also update the prefetch cache.
Or you can always manually reload the instance after updating the relations.

Referencing #4553 here, as it fixes the issue (though is an unsatisfactory way imho).

@wodCZ
Copy link

wodCZ commented Jan 2, 2017

I had issues described in #2442 with OneToOne relation using select_related - it returned old, not-updated objects after put/patch requests.
It works fine with prefetch_related (just one more query to fetch related objects instead of joining them).

It would be worth mentioning in documentation. I didn't expect this behaviour and it was not that simple to debug.

@tomchristie
Copy link
Member Author

@wodCZ Note that #2442 is resolved in recent releases.

@carltongibson
Copy link
Collaborator

I'm going to de-milestone this. We'll reassess after v3.7.

@carltongibson carltongibson removed this from the 3.6.5 Release milestone Sep 28, 2017
@johnthagen
Copy link
Contributor

johnthagen commented Feb 17, 2019

@carltongibson Any update this a couple years later? As someone new to DRF and hitting some massive performance issues (that are almost certainly related to not doing select_related and prefetch_related correctly), some docs and examples would be great! I'd be happy to contribute but I haven't been able to solve this issue myself yet.

Some links I've accumulated on the topic that may be useful:

@carltongibson
Copy link
Collaborator

carltongibson commented Feb 18, 2019

Hi @johnthagen. Good set of links.

I haven't been able to solve this issue myself yet.

I'm not sure there is a solution, in the sense of anything that will do this automatically for you. Bottom line is you need to look at the queries performed by each end point and carefully optimise them.

I'm not entirely convinced this is an actionable issue. (Other than adding some links back to Django's docs maybe.) It's a good topic for blog posts.

@johnthagen
Copy link
Contributor

@carltongibson

I'm not sure there is a solution, in the sense of anything that will do this automatically for you

Yeah I tend to agree that there isn't something automatic, but I think that examples / some discussion can go a long way. The issue with only using blog posts is they become stale / out-of-date. Having official docs allows people to contribute / keep the information up to date.

The caching docs seem to be set up in a way I was imagining:

It has a concrete example and some discussion.

@auvipy
Copy link
Member

auvipy commented May 11, 2020

@carltongibson

I'm not sure there is a solution, in the sense of anything that will do this automatically for you

Yeah I tend to agree that there isn't something automatic, but I think that examples / some discussion can go a long way. The issue with only using blog posts is they become stale / out-of-date. Having official docs allows people to contribute / keep the information up to date.

The caching docs seem to be set up in a way I was imagining:

* https://www.django-rest-framework.org/api-guide/caching/

It has a concrete example and some discussion.

it would be great if you come up with a PR

@auvipy
Copy link
Member

auvipy commented Jul 30, 2020

@auvipy
Copy link
Member

auvipy commented Jul 30, 2020

IMHO it's more of a Django ORM good practices with DRF. what if modifying some examples using the aforementioned possible current good practices?

@thetarby
Copy link
Contributor

thetarby commented Aug 4, 2020

Hello everyone,

We faced the same issues in some projects and I have created a simple package to use in our projects. Then I have decided to publish it in github and pypi for anyone who is interested. Btw it is still being tested and there might be bugs. You can check it out on https://github.com/thetarby/django-auto-related

Simply what it does:

What django-rest gives us is the ability to know what will be accessed from the object before it is accessed by inspecting the source attribute of the fields defined in a serializer.

Then it traces those sources back on django fields instances and translates them to select_related, prefetch_related and only methods of django. Note that it does not use django-rest fields to decide what to prefetch but django fields because what django-rest says a related_field may not be a related field for the django or vice versa.

For instance, serializers.CharField(source=related_model.title) is not a related field for djangorest but in reality related_model should be prefetched to avoid n+1 problems.

If you have any feedback I would be glad to hear from you on github.

@auvipy
Copy link
Member

auvipy commented Aug 13, 2020

Hello everyone,

We faced the same issues in some projects and I have created a simple package to use in our projects. Then I have decided to publish it in github and pypi for anyone who is interested. Btw it is still being tested and there might be bugs. You can check it out on https://github.com/thetarby/django-auto-related

Simply what it does:

What django-rest gives us is the ability to know what will be accessed from the object before it is accessed by inspecting the source attribute of the fields defined in a serializer.

Then it traces those sources back on django fields instances and translates them to select_related, prefetch_related and only methods of django. Note that it does not use django-rest fields to decide what to prefetch but django fields because what django-rest says a related_field may not be a related field for the django or vice versa.

For instance, serializers.CharField(source=related_model.title) is not a related field for djangorest but in reality related_model should be prefetched to avoid n+1 problems.

If you have any feedback I would be glad to hear from you on github.

great package! i checked the code and like it. will use in my projects in near future.

@thetarby
Copy link
Contributor

great package! i checked the code and like it. will use in my projects in near future.

Thank you. I am glad that you think that way.

@stale
Copy link

stale bot commented May 2, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label May 2, 2022
@johnthagen
Copy link
Contributor

Bumping this issue to un-stale it, as I think this is an important topic for beginners that could be improved in the docs.

@stale stale bot removed the stale label May 2, 2022
@stale
Copy link

stale bot commented Jul 14, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 14, 2022
@auvipy
Copy link
Member

auvipy commented Jul 14, 2022

can we close this based on #7610?

@stale stale bot removed the stale label Jul 14, 2022
@stale
Copy link

stale bot commented Sep 21, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Sep 21, 2022
@auvipy auvipy closed this as completed Dec 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants