exAspArk / batch-loader

:zap: Powerful tool for avoiding N+1 DB or HTTP queries
https://engineering.universe.com/batching-a-powerful-way-to-solve-n-1-queries-every-rubyist-should-know-24e20c6e7b94
MIT License
1.04k stars 52 forks source link

N+1 queries when batch loading has many association #59

Closed tjvc closed 4 years ago

tjvc commented 4 years ago

Nice gem - thanks!

This isn't an issue per se - I already have a working solution based on the second example from this section in the README: https://github.com/exAspArk/batch-loader#loading-multiple-items.

I'm curious though as to why the following approach does not prevent an N+1 query.

Given these models:

class Post < ApplicationRecord
  has_many :comments
end

class Comment < ApplicationRecord
  belongs_to :post
end

And a GraphQL schema with a postsConnection on the root query type and a commentsConnection on the post type.

The following seems to trigger an N+1 query for the comments association on each post when used to resolve the commentsConnection for each post:

BatchLoader::GraphQL.for(post.id).batch(default_value: []) do |post_ids, loader|
  Post.
    where(id: post_ids).
    includes(:comments).
    each { |post| loader.call(post.id, post.comments) }
end
exAspArk commented 4 years ago

Hey @tjvc,

Interesting. A few questions:

tjvc commented 4 years ago

Hey @exAspArk. Thanks for the suggestion to try reproducing this in a smaller repo. I tried in a new application and couldn't. Digging into this a bit more, I discovered that it was related to our Pundit integration with GraphQL::Pro, and specifically the chaining of policy scopes onto batch-loaded relations, which triggered fresh queries. So I'll close this now as it's definitely not an issue with BatchLoader. Thanks for your time!