-
Notifications
You must be signed in to change notification settings - Fork 58
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
[WIP] Add Query Optimizer that selects fields from query AST #56
base: main
Are you sure you want to change the base?
Conversation
What do you think @zerolab?? |
5b78d91
to
b7a0d19
Compare
b7a0d19
to
641bcba
Compare
@@ -178,3 +179,7 @@ | |||
"ROUTING": "grapple.urls.channel_routing", | |||
} | |||
} | |||
|
|||
# Query Optimisation helpers | |||
SHELL_PLUS_PRINT_SQL = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these should be enabled only locally, maybe?
@@ -10,3 +10,4 @@ factory-boy==2.12.0 | |||
wagtail-factories==2.0.0 | |||
django-cors-headers==3.0.2 | |||
wagtail-headless-preview==0.1.4 | |||
django_extensions==2.2.9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not convinced we need the extra dependency. Should only be a dev dependency
|
||
if id is not None: | ||
qs = qs.filter(pk=id) | ||
qs = qs.get(id=id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can throw a DoesNotExist
should the provided id not be there anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had a quick look and left a few comments.
Will try to review more in depth this weekend.
I am curious, do you have any before/after profiling stats?
incidentally, https://github.com/tolomea/django-auto-prefetch and https://github.com/jmcarp/nplusone popped up this week -- there some good ideas there
Why not consider using https://github.com/tfoxy/graphene-django-optimizer ? |
Hi @fabienheureux , I did try GDQ but it doesn't work with stream field and that's a big issue for us. Instead of hack GDQ into Grapple, I decided to create our own way of doing it that can be fine tuned and expanded in the future. |
# Conflicts: # example/home/models.py # grapple/urls.py
@NathHorrigan any chance you can bring this to the finish line? |
@NathHorrigan any news on this PR? I'm going to fix the merge conflicts 🙂 . |
I started a PR on the |
This is the initial code for optimizing GraphQL queries to reduce the number of database calls. The gist of this PR is that it analyses from the queries' AST what fields of what models it needs and then appends that to the Queryset object, further along the line when a
.specific()
call is applied these field selects are called on the Queryset using.only(..)
,.select_related(..)
&.prefetch_related(..)
.More accurate data will follow...