-
Notifications
You must be signed in to change notification settings - Fork 16
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
Subset GraphQL query selections #3196
Draft
yndajas
wants to merge
7
commits into
reduce-dataloader-queries
Choose a base branch
from
subset-graphql-query-selections
base: reduce-dataloader-queries
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Subset GraphQL query selections #3196
yndajas
wants to merge
7
commits into
reduce-dataloader-queries
from
subset-graphql-query-selections
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
7e7c61f
to
50c28a4
Compare
50c28a4
to
7894c42
Compare
7894c42
to
70f046a
Compare
available_translations aren't normal links, they have their own resolver method. `links_field` is for normal links and it defines a common resolver method.
6fa3bc5
to
3802f49
Compare
We're working towards SELECTing a subset of an Edition's columns, plus a subset of some JOINed associations' columns, too. Working with the sets, arrays and hashes required to build a complex SELECT argument is getting increasingly gross, so this object is intended to provide a smaller and neater interface for doing that. This also provides a constructor that converts the GraphQL query's edition selections to database column names. We include a list of valid database columns and a hash which maps GraphQL fields not in the list to the database columns they rely on Co-authored-by: Ynda Jas <yndajas@gmail.com>
We're hoping to speed up our GraphQL responses by reducing the amount of data retrieved from the database. By design we already only send the client the data they've requested, but until now that meant that we've been fetching a lot of data from the database that we don't actually make use of in serving the response to the client.
I originally named this variable that way because I was having trouble remembering what a "link type" even is and what its significance might be. We've been knee-deep in these Publishing API concepts for a couple of months now and as a result both of those things have become second nature and this variable name has become cumbersome. (I'm not sure whether the naming for reverse_links_field was ever quite accurate.)
Following on from the changes we made to the QueryType's database query for the root edition, this change aims to speed up GraphQL queries involving generic links by reducing the amount of data retrieved from the database. Co-authored-by: Ynda Jas <yndajas@gmail.com>
Just like the changes we've just made to the database queries for linked-to-editions, this reduces the amount of data retrieved from the database when resolving reverse-linked editions in an attempt to speed up response times. Co-authored-by: Ynda Jas <yndajas@gmail.com>
The idea with this change is to reduce the amount of data retrieved from the database in an attempt to speed up the overall response time to the GraphQL client. So far we've implemented this optimisation for the root edition, linked-to editions and reverse-linked editions. Here we're introducing the change for the person's role_appointments/roles fields specific to the ministers index type. We've followed the patterns established in the previous implementations with the big differences being mostly: * the larger number of JOINs needed in this query because we're traversing 2 levels of links * the convenience of only needing to handle link set links here because we know that those are the only types of links involved in linking the ministers index's person-role_appointments-roles Because of the large number of JOINS, we've resorted to supplying SQL strings in order to provide our own, hopefully clearer, join table aliases as opposed to leaving that to Active Record to generate.
3802f49
to
1e9b1f6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This application is owned by the publishing platform team. Please let us know in #govuk-publishing-platform when you raise any PRs.
Follow these steps if you are doing a Rails upgrade.