Skip to content

Is there n+1 queries issue? #358

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
khanetor opened this issue Nov 9, 2016 · 11 comments
Closed

Is there n+1 queries issue? #358

khanetor opened this issue Nov 9, 2016 · 11 comments

Comments

@khanetor
Copy link

khanetor commented Nov 9, 2016

A common problem with ORM is the n+1 queries issue, which can be avoided with prefetch_related or select_related. Do we have this problem when using this library with Django + sql?

@yfilali
Copy link

yfilali commented Nov 9, 2016

Short answer is yes. I suggested a way to deal with it here by delaying the query as much as possible in other to be able to ask the ORM to select_related:

#348 (comment)

@yfilali
Copy link

yfilali commented Nov 9, 2016

Also, cache all the things, in memory preferably, per request at the very least.

@khanetor
Copy link
Author

khanetor commented Nov 9, 2016

Do you have an example? From the doc of graphene-django, there is nowhere that we have to construct the query manually.

On Nov 10, 2016, 12:38 AM +0200, Yacine Filali [email protected], wrote:

Also, cache all the things, in memory preferably, per request at the very least.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub (#358 (comment)), or mute the thread (https://github.com/notifications/unsubscribe-auth/ADTJWKlZ6Xfi2UXZGPuRGJy8qKmnncsDks5q8kuBgaJpZM4KuD8A).

@khanetor
Copy link
Author

khanetor commented Nov 9, 2016

Or sorry, I missed your included link.

On Nov 10, 2016, 12:38 AM +0200, Yacine Filali [email protected], wrote:

Also, cache all the things, in memory preferably, per request at the very least.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub (#358 (comment)), or mute the thread (https://github.com/notifications/unsubscribe-auth/ADTJWKlZ6Xfi2UXZGPuRGJy8qKmnncsDks5q8kuBgaJpZM4KuD8A).

@khanetor
Copy link
Author

khanetor commented Nov 9, 2016

I don't quite get the proxy object. Is it something from Graphene?

Anyway, I think what I can do is when resolving a parent object, I should also prefetch related entities. This is fine for now, but I wonder if graphene is flexible enough to only get related entities when required; sometimes users just want, for e.g., a list of articles without included comments

@yfilali
Copy link

yfilali commented Nov 9, 2016

The proxy object is just there to lazy load the parent object. The problem is that you need to know what related entities to prefetch with the parent object so instead of resolving it, you return a proxy that resolves itself only when the attributes are accessed. It allows you to delay the actual DB query until you've hit the resolvers for the related objects.

Each ORM integration with graphene deals with this differently. I wrote the integration with PynamoDB, and there, I resolve all relationships into proxy objects which only query the db when the non-id attributes are accessed.

@bforchhammer
Copy link

I've stumbled onto this problem a couple of days ago as well. My issue is that I have to request data from another service for one of the nodes in my GraphQL structure, and ideally I would like to execute only one bulk request for all required objects instead of 10 separate requests. Unfortunately the Proxy-Object method did not quite work for me, because the node can appear in different parts of the requested graph and I did not have one ConnectionField being initiated with the list of IDs as seems to be done with the PynamoDB solution...

Here's what I ended up with instead:

  • I created a class for lazy promises, which are not evaluated until resolution is explicitly triggered
  • I trigger the resolution in a custom GraphQL executor by overriding Executor.wait_until_finished (i.e. pretty much at the end of the request cycle)
  • When instantiating the lazy promise, I also register all values which I need to retrieve data for, so I can do one bulk request in the end.

You can find all the code with an example in this gist, feel free to use/copy/adapt it for anything if you find it useful 😃

@yfilali
Copy link

yfilali commented Feb 27, 2017

This is what I ended up implementing for SQLAlchemy:
GraphQL, Graphene, SqlAlchemy and the N+1 problem

The problem with lazy promises is that the query is executed when it is trimmed, as it is treated as a list, so it needs to know in the initial resolver what it needs to include.
https://github.com/graphql-python/graphql-relay-py/blob/c4c65337df2e169ae5c178dc79d25e8e234debd5/graphql_relay/connection/arrayconnection.py#L81

@richmondwang
Copy link

richmondwang commented Apr 7, 2017

@yfilali thanks for the post you created! It worked for me like a charm.

Also, I somehow "forked" it and updated it to select only the requested fields/columns from the info. Here is my gist (https://gist.github.com/richmondwang/cc9ba2db32e44795359417dd08326c31).
It worked, though I haven't created test script for it yet.

@chanind
Copy link

chanind commented Jun 8, 2017

Another option is to use an intelligent lazy loader. I'm not sure about Django, but we use the following lazy loader for SqlAlchemy which should solve the n+1 query problem without needing any custom code. We open-sourced the lazy loader here in case it's helpful for anyone else: https://github.com/operator/sqlalchemy_bulk_lazy_loader

@jkimbo
Copy link
Member

jkimbo commented Feb 18, 2018

Hi @nlhkh . We're currently going through old issues that appear to have gone stale (ie. not updated in about the last 6 months) to try and clean up the issue tracker. If this is still important to you please comment and we'll re-open this.

In regards to this particular issue you might want to look at dataloader as a solution.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants