-
-
Notifications
You must be signed in to change notification settings - Fork 438
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
Select only necessary columns for optimized queries #1356
Comments
Technically, this can be done by constraining the There are some pitfalls to this which require discussion:
|
Would you be so awesome as to provide an example? I've searched through the docs but I can't seem to find what I am looking for :-( Let's say I have a model type Nerd {
glasses: Int!
beard: String!
} and I want to query them with type Query {
nerds: [Nerd] @all
} and I only want the SQL statement to be |
public function __invoke($root, array $args, GraphQLContext $context, ResolveInfo $resolveInfo)
{
$resolveInfo->getFieldSelection();
$resolveInfo->lookAhead();
// TODO use the information from those methods to SELECT only the given fields
} |
Thanks! Just one more stupid question: where do I put that method? On the Eloquent model? |
Awesome, thanks! |
This kind of feature would be a huge performance improvement for the package. I'm implementing an endpoint for multi million record table with at least 50+ columns. I'm using query builder to make search in a given bounding box (https://wiki.openstreetmap.org/wiki/Bounding_Box) in PostgreSQL also implemented complex where conditions to filter the results. Only issue is Laravel generates SELECT * queries which affects the performance. I could use custom resolvers but when I do that I have to deal with pagination. Following is my current Query schema:
Is there any way I can modify select columns without writing custom resolver ? Because by this way with very little code I can do so much. |
I am not planning to implement such functionality in Lighthouse myself, but am open to accepting PRs. |
I understand the complexity and side affects like you mentioned above about relations and virtual properties. It would be nice to have some way to control select columns like in query builder @builder directive. Thanks, even like this Lighthouse is very useful. |
Can't quite see how this is duplicate. Could this be solved by implementing a class User extends Model
{
public function getNameAttribute()
{
return $this->first_name . ' ' . $this->last_name;
}
} type User {
id: ID!
first_name: String
last_name: String
name: String @select(fields: ["first_name", "last_name"])
} |
@andershagbard right, there is some overlap between the issue, but the intent is different.
That would be one part of the puzzle, good thinking. We might need a mechanism to make the optimized selects opt-in per field / type. Otherwise, it could be quite hard to migrate existing applications towards it, since now Lighthouse always selects all the fields. |
Trying to think of a good solution for this. Could this be an opt-in in the config file? If enabled it would use the field(s) in the @select, if @select is absent, use the field name. |
We might do that as an MVP. I am a bit worried about migrating big applications towards optimized selects, but maybe it is not a big issue in practice. I do not plan to implement such a feature myself, but am happy to aid in reviewing and refining a PR. |
@spawnia I am working on an implementation for this, and I am having trouble accessing child directives via |
That would give you access to client directives. This feature requires looking at schema directives. Look at other Lighthouse directives to see how that works. |
Could you give me an example of a parent directive searching through child directives? I can't think of any off the top of my head. |
I do not know what you mean by parent or child directives. Consider the Try posting a draft PR and we can discuss implementation details. |
I got to the point that I have created at trait that get the fields that should get selected but I am facing a serious issue with relations I can't apply the select method to belongsTo and hasOne and HasMany directives for some reason is there's any easier way to make the select rule more generic and we can apply it to all directives cause I don't see a way in here |
Hi all! other thing that I notice is about if in the
something similar for https://github.com/nuwave/lighthouse/blob/master/src/Pagination/PaginationArgs.php#L107 if we modify the below line to include the columns for the count process, the
This is because the Laravel https://github.com/laravel/framework/blob/8.x/src/Illuminate/Database/Eloquent/Builder.php#L808 |
Update: I opened a discussion about the paginate method laravel/framework#40114 |
@dmouse that's just a misconception |
Trying to do something similar and I found that brilliant piece you written here!! $resolveInfo->getFieldSelection(100); It gives the result in the most beautiful output I could have imagined to get, not sure about performance issues... |
$resolveInfo->getFieldSelection(100); That is absolutely fine, if it has the information you need. |
Thank you so much for explaining that, so now I know how to access arguments of sub-fields thanks to that method!
$_, array $args, GraphQLContext $context, ResolveInfo $resolveInfo $_ here gives the parent result, but can't access for instance parent args or args from parent of the parent...
Let's say that I have an array which I got based on the info I was provided in an upper-field now in a sub node (in the same branch) I want to access those data, how could I pass data from upper node to lower nodes in the branch? I thought context was there for this but I was wrong! |
I think those questions are off-topic, you may ask in StackOverflow or Slack. |
Right now, two pull requests are trying to solve this issue:
Going forward, I think the minimal viable solution for this issue is to implement the manual approach first. A marker directive needs to be added to fields to inform Lighthouse which columns need to be selected for it, e.g.: """
Defines which columns must be selected from the database to resolve the field.
If specified on at least one field of a type, Lighthouse will `SELECT` only the explicitly
mentioned columns matching the queried fields. For fields without this directive,
no columns will be selected, so make sure to specify this for all fields that need it.
"""
directive @select(columns: [String!]!) on FIELD_DEFINITION
type User {
id: ID! @select(columns: ["id"])
first_name: String! @select(columns: ["first_name"])
last_name: String! @select(columns: ["last_name"])
full_name: String! @select(columns: ["first_name", "last_name"])
rights: [Right!]! @hasMany
} Only if the fields of a type are explicitly marked with { users { id last_name full_name } }
# SELECT id, last_name, first_name |
@spawnia I agree on your approach on it being an entirely opt-in feature through the I've been following this for a while as I have one table that has a couple of large blob columns (not the best design but it's a legacy project), without constraining the |
@spawnia I'm currently using Lighthouse GraphQL(6.42.1) in my project, and I'm facing an issue similar to this. I'm looking for a proper solution because in my case, I have tables with around 100 columns. I'm utilizing the @paginate directive for optimization, but in some cases, I don't need the @paginate directive. Is there any solution that can handle all relationship queries? I've checked this pull request, but it hasn't been merged. if we can pass the selected column to the eloquent then it will decrease the API response time
|
What problem does this feature proposal attempt to solve?
I am trying out GraphQL, testing various features in order to include it in some of the web apps I am developing with laravel. Being curious, I created a Schema and fired up all sorts of queries, followed by poking around in the SQL server to see what was going on.
What I discovered was this:
Suppose I have a Type defined which corresponds to a Model, and the database table has an ID column and 26 more columns (A, B C, ..., Z).
No matter what the Type definition is, which in my case only includes some of the columns ( say, ID and A, B, C), I can see that the SQL server is asked to fetch all 27 columns from the DB, using a SELECT * FROM [Table] statement, even though given the definition of the type at most 4 of them would be returned via the resolver.
This is an unnecessary data transfer between the DB and PHP, in my opinion.
Which possible solutions should be considered?
I propose that we construct a more refined SQL query, selecting only the columns that are requested in the GraphQL query.
I am not sure I am competent enough to try and implement this on my own, but I will give it a shot. In any case, I thought I'd share my thoughts.
The text was updated successfully, but these errors were encountered: