lighthouse icon indicating copy to clipboard operation
lighthouse copied to clipboard

Add optimized selects

Open DDynamic opened this issue 4 years ago • 5 comments

  • [ ] Added or updated tests
  • [ ] Documented user facing changes
  • [ ] Updated CHANGELOG.md

Resolves #1356

Changes This MR optimizes queries generated by eloquent by only including the requested fields.

type Query {
  posts: [Post] @all
}

type Post {
  id: ID
  likes: Int
  title: String
  uppercaseTitle: String @select(fields: ["title"]) @method(name: "uppercaseTitle")
}
{
  posts {
    id
    uppercaseTitle
  }
}

The above schema definition and query cause SELECT * FROM posts to be executed. This MR adds adds in an opt-in configuration option to change this behavior to select only the fields required, SELECT id, title FROM posts. It adds a select directive to specify field dependencies, like in the case of uppercaseTitle.

@spawnia what do you think of this implementation idea? I could update the AllDirecrive to get the field selection with $resolveInfo, but I am not sure how to "look ahead" to field selection to see if there is a select directive present.

DDynamic avatar Nov 27 '20 04:11 DDynamic

First, I suggest to implement the trivial case without @select:

{
  posts {
    id
    title
  }
}
select id, title from posts;

Next, consider the implementation of https://github.com/nuwave/lighthouse/blob/master/src/Schema/Directives/CacheDirective.php, that should give you an idea how you can grab @select. You will have to combine the field selection from $resolveInfo with the type information from the schema. Tricky, but definitely doable.

The actual @select directive probably won't be much more than a marker and probably not contain much code.

Since many resolver directives can benefit from this optimization, it should be abstracted and made reusable. Maybe in a similar way as ArgumentSet::enhanceBuilder().

spawnia avatar Nov 27 '20 13:11 spawnia

Also, see https://github.com/nuwave/lighthouse/pull/1253

spawnia avatar Nov 27 '20 15:11 spawnia

Hey, @spawnia, thanks for checking in. I tried to get some work done on it this weekend, but I hit a wall with some edge cases and Eloquent limitations surrounding polymorphic relationships (example), so I decided to push what I had. The app that utilizes this only employs the all, paginate, and one-to-one and one-to-many relationship directives, so those use cases are working.

I'm not sure how many more hours I can put into this to address all of the edge cases. It would be an amazing edition though, nonetheless.

DDynamic avatar Jan 11 '21 02:01 DDynamic

A partial solution is better than no solution at all, if there are shortcomings we can document them and offer users the choice. Let's just make sure that whatever is implemented actually works.

spawnia avatar Jan 11 '21 13:01 spawnia