Skip to content

Optimize Django SQL queries based on the GraphQL queries fields#67

Closed
syrusakbary wants to merge 9 commits intomasterfrom
features/django-debug-optimize
Closed

Optimize Django SQL queries based on the GraphQL queries fields#67
syrusakbary wants to merge 9 commits intomasterfrom
features/django-debug-optimize

Conversation

@syrusakbary
Copy link
Copy Markdown
Member

As GraphQL provides a way for fetching only certain fields, the Django mapper should adapt for querying only what is requested. That means using model.objects.only(*requested_fields) as will be much more performant.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function seems useful outside Django, to get the fields from an info object. Is there any way this could be refactored into graphene core? I'm willing to do the work, but I'm not familiar enough with the codebase to know where this should fit.

Update: I'm looking into whether this can be directly added to the ResolveInfo class in graphql-core. It seems the most logical place for this to go.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch @bigblind - that would be great to have in graphene core/ResolveInfo

@syrusakbary syrusakbary force-pushed the features/django-debug-optimize branch from 1c3c511 to 7be53fa Compare January 20, 2016 03:58
Conflicts:
	graphene/contrib/django/converter.py
	graphene/contrib/django/fields.py
	graphene/contrib/django/tests/test_converter.py
@syrusakbary syrusakbary force-pushed the features/django-debug-optimize branch from 7be53fa to 6e3fe9c Compare January 20, 2016 04:03
@mjtamlyn
Copy link
Copy Markdown
Contributor

mjtamlyn commented May 5, 2016

My understanding is that .only() is in fact a performance hit in many cases. This may seem counter intuitive, but it comes down the the fact that the returned model object from an only query is much more complex than a raw one, and this extra time is a bigger hit that the extra work your database does.

See the documentation for only and defer, the note pretty much says "don't do this unless you absolutely have to".

Copy link
Copy Markdown

@phalt phalt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be part of graphene-django and not graphene? This PR is very old, so it's probably worth re-implementing from scratch.

@ProjectCheshire
Copy link
Copy Markdown
Member

@phalt this is when django was under graphene.contrib.django :) I the faq / upgrading it was moved in v1.0 to a seperate package

@phalt
Copy link
Copy Markdown

phalt commented May 16, 2019

@ProjectCheshire guess we can close it then!

@stale
Copy link
Copy Markdown

stale bot commented Jul 29, 2019

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 wontfix label Jul 29, 2019
@stale stale bot closed this Aug 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants