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

Best Practice Using Batch loader on Graphql 1.8.11 #30

Closed im-not-a-robot closed 5 years ago

im-not-a-robot commented 5 years ago

My code for batch loader :

BatchLoader.for(object.id).batch(default_value: []) do |province_ids, loader|
    City.where(province_id: province_ids).each do |city|
         loader.call(city.province_id) { |data| data << city }
     end
end

Result :

Province Load (0.7ms)  SELECT "provinces".* FROM "provinces"
↳ app/controllers/graphql_controller.rb:11
City Load (0.5ms)  SELECT "cities".* FROM "cities" WHERE "cities"."province_id" = $1  [["province_id", 1]]
↳ app/graphql/types/province_type.rb:21
 City Load (1.1ms)  SELECT "cities".* FROM "cities" WHERE "cities"."province_id" = $1  [["province_id", 2]]
↳ app/graphql/types/province_type.rb:21
City Load (0.5ms)  SELECT "cities".* FROM "cities" WHERE "cities"."province_id" = $1  [["province_id", 3]]

Here's my gemfile :

source 'https://rubygems.org'
git_source(:github) { |repo| "https://github.com/#{repo}.git" }

ruby '2.4.5'

# Bundle edge Rails instead: gem 'rails', github: 'rails/rails'
gem 'rails', '~> 5.2.1'
# Use sqlite3 as the database for Active Record
# gem 'sqlite3'
# Use postgresql as the database for Active Record
gem 'pg'
# Use Puma as the app server
gem 'puma', '~> 3.11'
# Build JSON APIs with ease. Read more: https://github.com/rails/jbuilder
# gem 'jbuilder', '~> 2.5'
# Use Redis adapter to run Action Cable in production
# gem 'redis', '~> 4.0'
# Use ActiveModel has_secure_password
# gem 'bcrypt', '~> 3.1.7'

# Use ActiveStorage variant
# gem 'mini_magick', '~> 4.8'

# Use Capistrano for deployment
# gem 'capistrano-rails', group: :development

# Reduces boot times through caching; required in config/boot.rb
gem 'bootsnap', '>= 1.1.0', require: false

# Use Rack CORS for handling Cross-Origin Resource Sharing (CORS), making cross-origin AJAX possible
# gem 'rack-cors'

group :development, :test do
  # Call 'byebug' anywhere in the code to stop execution and get a debugger console
  gem 'byebug', platforms: [:mri, :mingw, :x64_mingw]
end

group :development do
  gem 'listen', '>= 3.0.5', '< 3.2'
  # Spring speeds up development by keeping your application running in the background. Read more: https://github.com/rails/spring
  gem 'spring'
  gem 'spring-watcher-listen', '~> 2.0.0'
end

# Windows does not include zoneinfo files, so bundle the tzinfo-data gem
gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby]
gem 'graphql'

gem 'batch-loader'

Im already use your quick fix on this comment https://github.com/exAspArk/batch-loader/issues/22#issuecomment-413341798 It can solved this problem.

The other solution : https://github.com/rmosolgo/graphql-ruby/issues/1778#issuecomment-429496117 But it must make scope attribute to false

Is there best practice to solve this problem ? Thank you

exAspArk commented 5 years ago

Hi @im-not-a-robot!

Did you add use BatchLoader::GraphQL to your schema?

It should work without scope: false and any manual wrapping https://github.com/exAspArk/batch-loader/issues/22#issuecomment-443329419. Here is the test from the current master, which uses version 1.8.11 in the build:

https://github.com/exAspArk/batch-loader/blob/a7269949fab54adce0c554d4fec4d553fb63bc7c/spec/graphql_spec.rb#L4-L28

im-not-a-robot commented 5 years ago

Yes im already add use BatchLoader::GraphQLto my schema. I dont know why when im using make new rails project with new ruby version (2.4.5) and new graphql version (1.8.11), batch loader not work without scope: false or manual wrapping.

May if you want to see the issue, i can send you the project.

exAspArk commented 5 years ago

@im-not-a-robot yes, please 🙏 If you can send me the project (e.g. a github repo), that'll save me some time.

im-not-a-robot commented 5 years ago

Hi, you can get it here : https://github.com/im-not-a-robot/example-ruby-rails-graphql-batch-loader

FrancisMurilloDigix commented 5 years ago

Hello,

Currently having the same problem above and the scope: false fixed my issue as well.

ghost commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

im-not-a-robot commented 5 years ago

Hello @exAspArk Have you try that project?

exAspArk commented 5 years ago

Looking into it now

exAspArk commented 5 years ago

@im-not-a-robot unfortunately, I couldn't run your rails example app. Something is wrong with a DB: partially pg, partially sqlite, no migrations, no seed data, etc.

I created a new very basic Rails app which should be relatively simple to run https://github.com/exAspArk/batch-loader-graphql. I use graphql ruby gem version 1.8.13 with the latest batch-loader version 1.2.2, and I can't reproduce it:

image

Please let me know if you can reproduce it with that rails app. Maybe the latest graphql ruby gem versions fixed compatibility again.

im-not-a-robot commented 5 years ago

@im-not-a-robot unfortunately, I couldn't run your rails example app. Something is wrong with a DB: partially pg, partially sqlite, no migrations, no seed data, etc.

I created a new very basic Rails app which should be relatively simple to run https://github.com/exAspArk/batch-loader-graphql. I use graphql ruby gem version 1.8.13 with the latest batch-loader version 1.2.2, and I can't reproduce it:

image

Please let me know if you can reproduce it with that rails app. Maybe the latest graphql ruby gem versions fixed compatibility again.

@exAspArk Sorry for the problem. I'm already update the project, please check it again. Thank you. Note:

  1. Im using postgresql with default username password
  2. Im already add some data to database from seeds file
  3. Im already update gemfile
exAspArk commented 5 years ago

@im-not-a-robot awesome, thank you!

Could you please try these changes:

# Gemfile
gem 'batch-loader', github: 'exAspArk/batch-loader', branch: 'graphql'

You'd need to replace BatchLoader.for with BatchLoader::GraphQL.for to make it actually batch requests with the latest graphql versions. For example:

class PostType < GraphQL::Schema::Object
  field :user, UserType, null: false

  def user
    BatchLoader::GraphQL.for(object.user_id).batch do |user_ids, loader|
      User.where(id: user_ids).each { |user| loader.call(user.id, user) }
    end
  end
end

I will write more details on the issue later.

im-not-a-robot commented 5 years ago

@im-not-a-robot awesome, thank you!

Could you please try these changes https://github.com/exAspArk/batch-loader/compare/graphql?expand=1:

# Gemfile
gem 'batch-loader', github: 'exAspArk/batch-loader', branch: 'graphql'

You'd need to replace BatchLoader.for with BatchLoader::GraphQL.for to make it actually batch requests with the latest graphql versions. For example:

class PostType < GraphQL::Schema::Object
  field :user, UserType, null: false

  def user
    BatchLoader::GraphQL.for(object.user_id).batch do |user_ids, loader|
      User.where(id: user_ids).each { |user| loader.call(user.id, user) }
    end
  end
end

I will write more details on the issue later.

Yeah its works well. Thank you !

exAspArk commented 5 years ago

@im-not-a-robot perfect, thank you!

I wrote an explanation about the root cause in https://github.com/exAspArk/batch-loader/pull/32. Will try to ship it tomorrow.

exAspArk commented 5 years ago

Released these changes in v1.3.0:

TL;DR

  • If you use graphql gem version 1.8.6 or lower, you can continue using BatchLoader.
  • If you use graphql gem version 1.8.7 or greater, you need to replace BatchLoader.for with BatchLoader::GraphQL.for to make it work within your GraphQL resolvers (methods).