RStankov / SearchObjectGraphQL

GraphQL plugin for SearchObject gem
MIT License
159 stars 25 forks source link

Combining SearchObject with lazy loading #24

Closed hschne closed 4 years ago

hschne commented 4 years ago

I'm using SearchObject for GraphQL on on a type that is used within collections. Due to some N+1 queries I'm trying to use batching to improve performance - while still keeping the resolver intact (because filtering with SearchObject is awesome :rocket:)

That is, we might have something like this:

class MyQuery < GraphQL::Schema::Object
  field :items, [ItemType]

  def items
    Items.all
  end
end

class ItemType < BaseObject
  field :children, [ChildType], resolver: Resolvers::ChildrenSearch
end

In order to avoid N+1 queries and improve performance I use a batching library to avoid repeatedly fetching children for each item. In my case that is graphql-batch.

Without filtering, something like this does the trick (using AssociationLoader):

class ItemType < BaseObject
  field :children, [ChildType]

  def children
      AssociationLoader.for(Item, :children).load(object)
  end
end

Now I would like to combine lazy loading with filtering provided by SearchObject.

I managed to accomplish this by creating a resolver like this:

class ChildrenSearch < BaseResolver
  include SearchObject.module(:graphql)

  type [Types::ChildType], null: false
  # Scope is not actually used as it is disregarded because resolve_with_support is overwritten
  scope { object.children }

  # Just an example option 
  option(:limit, type: Integer, description: 'Limit number of children') do |scope, value|
    scope.first(value)
  end

  # This is where the magic happens
  def resolve_with_support(args = {})
    self.params = args.to_h
    AssociationLoader.for(Item, :children).load(object).then do |children|
      config  = self.class.config
      @search = SearchObject::Search.new(
        scope:    attachments,
        options:  config[:options],
        defaults: config[:defaults],
        params:   self.params
      )
      results
    end
  end
end

My question is, since my knowledge of SearchObject is limited: Is there a more elegant way to accomplish this? Also, can you think of a solution that could be built into the GraphQLSearch itself to accomplish this?

I would be happy to work on this, just not sure how to best proceed.

Thank you very much in advance! :bow:

RStankov commented 4 years ago

Hey, thanks for the good writeup. 🙌

Your solution is quite elegant.

This is a problem, and I also have in my GraphQL project.

If I understand your solution:

Your solution fetches all "children" for all items with one query, and then you use those "children" as an array.

One issue will be that even if you do limit, you are still fetching all those rows, and you can't do a more complex search filter.

Check "Batch loading many records more efficiently" from this article

I was thinking about how I can integrate batching in SearchObject. I don't know a good solution yet (maybe you can help).

Mine line of thought is to replace "resolve_with_support" from "SeachObject::Plugin::Graphql" with something like:

def resolve_with_support(args = {})
  self.params = args.to_h

  SearchObjectLoader.for(object.class, self.class.name).load(result)
end

The "result" should just return a scope so that we can batch on it.

I don't know how to implement the "SearchObjectLoader" in such a way that it only makes one query, which combines all scopes. 😅

So a query like the following can be just one search query:

query {
  itemsAll {
    chilren(active: true) {
      id
    }
  }
}

You can try to implement "SearchObjectLoader" and I'll help as much as I can.

hschne commented 4 years ago

Thanks for your response! Good to hear that I'm not the only person having this issue :sweat_smile:

Your solution fetches all "children" for all items with one query, and then you use those "children" as an array.

Yes that is correct. The crux here is that while we fetch all children, at least those children are fetched lazily, which results in a single query instead of N.

One issue will be that even if you do limit, you are still fetching all those rows, and you can't do a more complex search filter.

True. The items that we filter on are already loaded though, so no additional queries are performed, but it would still be better if only the items that the filter applies to are loaded in the first place.

I don't know how to implement the "SearchObjectLoader" in such a way that it only makes one query, which combines all scopes. sweat_smile

I don't know either :sweat_smile:

I don't feel like I know enough about how how graphql-batch works internally yet. Maybe I can think of something. I'll let you know if I can come up with something.

Thanks so much for your help :raised_hands:

RStankov commented 4 years ago

No worries 🙌

Ping me if you have more help.