Skip to content

Commit

Permalink
MONGOID-4173 Fix nested eager loading
Browse files Browse the repository at this point in the history
  • Loading branch information
estolfo committed Aug 10, 2016
1 parent ba646f9 commit 7cca240
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 3 deletions.
6 changes: 3 additions & 3 deletions lib/mongoid/relations/eager/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def run
@loaded = []
while shift_metadata
preload
@loaded << @docs.collect { |d| d.send(@metadata.name) }
@loaded << @docs.collect { |d| d.send(@metadata.name) if d.respond_to?(@metadata.name) }
end
@loaded.flatten
end
Expand Down Expand Up @@ -99,12 +99,12 @@ def set_on_parent(id, element)
# @example Return a hash with the current documents grouped by key.
# loader.grouped_docs
#
# @return [ Hash ] hash with groupd documents.
# @return [ Hash ] hash with grouped documents.
#
# @since 4.0.0
def grouped_docs
@grouped_docs[@metadata.name] ||= @docs.group_by do |doc|
doc.send(group_by_key)
doc.send(group_by_key) if doc.respond_to?(group_by_key)
end
end

Expand Down
40 changes: 40 additions & 0 deletions spec/mongoid/relations/eager_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,46 @@
expect(Mongoid::Relations::Eager::HasAndBelongsToMany).to receive(:new).with([houses_metadata], docs).once.and_call_original
context.eager_load(docs)
end

context 'when one of the eager loading definitions is nested' do

before do
class User
include Mongoid::Document
end

class Unit
include Mongoid::Document
end

class Booking
include Mongoid::Document
belongs_to :unit
has_many :vouchers
end

class Voucher
include Mongoid::Document
belongs_to :booking
belongs_to :created_by, class_name: 'User'
end
end

it 'successfully loads all relations' do
user = User.create
unit = Unit.create
booking = Booking.create(unit: unit)
Voucher.create(booking: booking, created_by: user)

vouchers = Voucher.includes(:created_by, booking: [:unit])

vouchers.each do |voucher|
expect(voucher.created_by).to eql(user)
expect(voucher.booking).to eql(booking)
expect(voucher.booking.unit).to eql(unit)
end
end
end
end

context "when including two of the same relation type" do
Expand Down

3 comments on commit 7cca240

@Victorcorcos
Copy link

@Victorcorcos Victorcorcos commented on 7cca240 Aug 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nested Includes is not working 🚨

Looks like even using the nested includes approach, the N+1 Query Problem is still happening!

Hello @estolfo .

I tried to use the nested includes included in this commit, requested by this feature request but looks like it is not really avoiding the N+1 Query Problem for nested associations.

Scenario 🌍

class Sheet < ApplicationRecord
  include Mongoid::Document

  has_many :groups

  field :title, type: String
end

class Group < ApplicationRecord
  include Mongoid::Document

  belongs_to :sheet
  has_many :columns
end

class Column < ApplicationRecord
  include Mongoid::Document

  belongs_to :group
end

Steps to Reproduce 🚢 ...

Column.all.includes(group: [:sheet]).map do |column|
  column.group&.sheet&.title
end

β€· Result: Still fetches N sheets.

MONGODB | mongo2:30002 req:4884 conn:1:1 sconn:31 | pocket_api_development.find | STARTED | {"find"=>"columns", "filter"=>{}, "$readPreference"=>{"mode"=>"primaryPreferred"}, "$db"=>"pocket_api_development", "$clusterTime"=>{"clusterTime"=>#<BSON::Timestamp:0x00007f9eec2d0838 @seconds=1661976640, @increment=1>, "signature"=>{"hash"=><BSON::B...
MONGODB | mongo2:30002 req:4884 | pocket_api_development.find | SUCCEEDED | 0.012s
MONGODB | mongo2:30002 req:4885 conn:1:1 sconn:31 | pocket_api_development.getMore | STARTED | {"getMore"=>#<BSON::Int64:0x00007f9ed0a28340 @value=7552879692520447446>, "collection"=>"columns", "$db"=>"pocket_api_development", "$clusterTime"=>{"clusterTime"=>#<BSON::Timestamp:0x00007f9eec2d0838 @seconds=1661976640, @increment=1>, "signature"=>{...
MONGODB | mongo2:30002 req:4885 | pocket_api_development.getMore | SUCCEEDED | 0.022s

MONGODB | mongo2:30002 req:4886 conn:1:1 sconn:31 | pocket_api_development.find | STARTED | {"find"=>"groups", "filter"=>{"_id"=>{"$in"=>[BSON::ObjectId('5f10f17a1ada720136adb66c'), BSON::ObjectId('5f11c9b31ada720136adb694'), BSON::ObjectId('5f00e4901ada7201415fffb3'), BSON::ObjectId('5f41df3c1ada72016f436b8c'), BSON::ObjectId('5f4bab791ada7...
MONGODB | mongo2:30002 req:4886 | pocket_api_development.find | SUCCEEDED | 0.018s

MONGODB | mongo2:30002 req:4887 conn:1:1 sconn:31 | pocket_api_development.find | STARTED | {"find"=>"sheets", "filter"=>{"_id"=>{"$in"=>[BSON::ObjectId('5f10f07f1ada720136adb669'), BSON::ObjectId('5f11bbc41ada720136adb680'), BSON::ObjectId('5f00b36f1ada7201415fffb2'), BSON::ObjectId('5f41de9d1ada72016f436b8b'), BSON::ObjectId('5f4bab2e1ada7...
MONGODB | mongo2:30002 req:4887 | pocket_api_development.find | SUCCEEDED | 0.011s
MONGODB | mongo2:30002 req:4888 conn:1:1 sconn:31 | pocket_api_development.find | STARTED | {"find"=>"sheets", "filter"=>{"_id"=>BSON::ObjectId('5f10f07f1ada720136adb669')}, "limit"=>1, "singleBatch"=>true, "$readPreference"=>{"mode"=>"primaryPreferred"}, "$db"=>"pocket_api_development", "$clusterTime"=>{"clusterTime"=>#<BSON::Timestamp:0x00...
MONGODB | mongo2:30002 req:4888 | pocket_api_development.find | SUCCEEDED | 0.021s
MONGODB | mongo2:30002 req:4889 conn:1:1 sconn:31 | pocket_api_development.find | STARTED | {"find"=>"sheets", "filter"=>{"_id"=>BSON::ObjectId('5f11bbc41ada720136adb680')}, "limit"=>1, "singleBatch"=>true, "$readPreference"=>{"mode"=>"primaryPreferred"}, "$db"=>"pocket_api_development", "$clusterTime"=>{"clusterTime"=>#<BSON::Timestamp:0x00...
MONGODB | mongo2:30002 req:4889 | pocket_api_development.find | SUCCEEDED | 0.029s
MONGODB | mongo2:30002 req:4890 conn:1:1 sconn:31 | pocket_api_development.find | STARTED | {"find"=>"sheets", "filter"=>{"_id"=>BSON::ObjectId('5f00b36f1ada7201415fffb2')}, "limit"=>1, "singleBatch"=>true, "$readPreference"=>{"mode"=>"primaryPreferred"}, "$db"=>"pocket_api_development", "$clusterTime"=>{"clusterTime"=>#<BSON::Timestamp:0x00...
MONGODB | mongo2:30002 req:4890 | pocket_api_development.find | SUCCEEDED | 0.009s
MONGODB | mongo2:30002 req:4891 conn:1:1 sconn:31 | pocket_api_development.find | STARTED | {"find"=>"sheets", "filter"=>{"_id"=>BSON::ObjectId('5f41de9d1ada72016f436b8b')}, "limit"=>1, "singleBatch"=>true, "$readPreference"=>{"mode"=>"primaryPreferred"}, "$db"=>"pocket_api_development", "$clusterTime"=>{"clusterTime"=>#<BSON::Timestamp:0x00...
MONGODB | mongo2:30002 req:4891 | pocket_api_development.find | SUCCEEDED | 0.002s
MONGODB | mongo2:30002 req:4892 conn:1:1 sconn:31 | pocket_api_development.find | STARTED | {"find"=>"sheets", "filter"=>{"_id"=>BSON::ObjectId('5f4bab2e1ada720171c53ad5')}, "limit"=>1, "singleBatch"=>true, "$readPreference"=>{"mode"=>"primaryPreferred"}, "$db"=>"pocket_api_development", "$clusterTime"=>{"clusterTime"=>#<BSON::Timestamp:0x00...
MONGODB | mongo2:30002 req:4892 | pocket_api_development.find | SUCCEEDED | 0.002s
MONGODB | mongo2:30002 req:4893 conn:1:1 sconn:31 | pocket_api_development.find | STARTED | {"find"=>"sheets", "filter"=>{"_id"=>BSON::ObjectId('5f4bb4201ada720171c53c49')}, "limit"=>1, "singleBatch"=>true, "$readPreference"=>{"mode"=>"primaryPreferred"}, "$db"=>"pocket_api_development", "$clusterTime"=>{"clusterTime"=>#<BSON::Timestamp:0x00...
MONGODB | mongo2:30002 req:4893 | pocket_api_development.find | SUCCEEDED | 0.003s
MONGODB | mongo2:30002 req:4894 conn:1:1 sconn:31 | pocket_api_development.find | STARTED | {"find"=>"sheets", "filter"=>{"_id"=>BSON::ObjectId('5f4c21941ada720171c54055')}, "limit"=>1, "singleBatch"=>true, "$readPreference"=>{"mode"=>"primaryPreferred"}, "$db"=>"pocket_api_development", "$clusterTime"=>{"clusterTime"=>#<BSON::Timestamp:0x00...
MONGODB | mongo2:30002 req:4894 | pocket_api_development.find | SUCCEEDED | 0.002s
MONGODB | mongo2:30002 req:4895 conn:1:1 sconn:31 | pocket_api_development.find | STARTED | {"find"=>"sheets", "filter"=>{"_id"=>BSON::ObjectId('5f4c21941ada720171c54055')}, "limit"=>1, "singleBatch"=>true, "$readPreference"=>{"mode"=>"primaryPreferred"}, "$db"=>"pocket_api_development", "$clusterTime"=>{"clusterTime"=>#<BSON::Timestamp:0x00...
MONGODB | mongo2:30002 req:4895 | pocket_api_development.find | SUCCEEDED | 0.002s
MONGODB | mongo2:30002 req:4896 conn:1:1 sconn:31 | pocket_api_development.find | STARTED | {"find"=>"sheets", "filter"=>{"_id"=>BSON::ObjectId('5f3d77b01ada720171f580b5')}, "limit"=>1, "singleBatch"=>true, "$readPreference"=>{"mode"=>"primaryPreferred"}, "$db"=>"pocket_api_development", "$clusterTime"=>{"clusterTime"=>#<BSON::Timestamp:0x00...
MONGODB | mongo2:30002 req:4896 | pocket_api_development.find | SUCCEEDED | 0.002s
MONGODB | mongo2:30002 req:4897 conn:1:1 sconn:31 | pocket_api_development.find | STARTED | {"find"=>"sheets", "filter"=>{"_id"=>BSON::ObjectId('5f5c27ee1ada7201711f08b8')}, "limit"=>1, "singleBatch"=>true, "$readPreference"=>{"mode"=>"primaryPreferred"}, "$db"=>"pocket_api_development", "$clusterTime"=>{"clusterTime"=>#<BSON::Timestamp:0x00...
MONGODB | mongo2:30002 req:4897 | pocket_api_development.find | SUCCEEDED | 0.002s
MONGODB | mongo2:30002 req:4898 conn:1:1 sconn:31 | pocket_api_development.find | STARTED | {"find"=>"sheets", "filter"=>{"_id"=>BSON::ObjectId('5f5cd09b1ada7201711f0b0e')}, "limit"=>1, "singleBatch"=>true, "$readPreference"=>{"mode"=>"primaryPreferred"}, "$db"=>"pocket_api_development", "$clusterTime"=>{"clusterTime"=>#<BSON::Timestamp:0x00...
MONGODB | mongo2:30002 req:4898 | pocket_api_development.find | SUCCEEDED | 0.002s
MONGODB | mongo2:30002 req:4899 conn:1:1 sconn:31 | pocket_api_development.find | STARTED | {"find"=>"sheets", "filter"=>{"_id"=>BSON::ObjectId('5f5cdfc51ada7201711f0b46')}, "limit"=>1, "singleBatch"=>true, "$readPreference"=>{"mode"=>"primaryPreferred"}, "$db"=>"pocket_api_development", "$clusterTime"=>{"clusterTime"=>#<BSON::Timestamp:0x00...
MONGODB | mongo2:30002 req:4899 | pocket_api_development.find | SUCCEEDED | 0.004s
MONGODB | mongo2:30002 req:4900 conn:1:1 sconn:31 | pocket_api_development.find | STARTED | {"find"=>"sheets", "filter"=>{"_id"=>BSON::ObjectId('5f5b21fb1ada720171c540e3')}, "limit"=>1, "singleBatch"=>true, "$readPreference"=>{"mode"=>"primaryPreferred"}, "$db"=>"pocket_api_development", "$clusterTime"=>{"clusterTime"=>#<BSON::Timestamp:0x00...
MONGODB | mongo2:30002 req:4900 | pocket_api_development.find | SUCCEEDED | 0.002s
MONGODB | mongo2:30002 req:4901 conn:1:1 sconn:31 | pocket_api_development.find | STARTED | {"find"=>"sheets", "filter"=>{"_id"=>BSON::ObjectId('5f6261cd1ada720171b1ebbc')}, "limit"=>1, "singleBatch"=>true, "$readPreference"=>{"mode"=>"primaryPreferred"}, "$db"=>"pocket_api_development", "$clusterTime"=>{"clusterTime"=>#<BSON::Timestamp:0x00...
MONGODB | mongo2:30002 req:4901 | pocket_api_development.find | SUCCEEDED | 0.002s
MONGODB | mongo2:30002 req:4902 conn:1:1 sconn:31 | pocket_api_development.find | STARTED | {"find"=>"sheets", "filter"=>{"_id"=>BSON::ObjectId('5f6420c21ada720171b1ec8f')}, "limit"=>1, "singleBatch"=>true, "$readPreference"=>{"mode"=>"primaryPreferred"}, "$db"=>"pocket_api_development", "$clusterTime"=>{"clusterTime"=>#<BSON::Timestamp:0x00...
MONGODB | mongo2:30002 req:4902 | pocket_api_development.find | SUCCEEDED | 0.003s
MONGODB | mongo2:30002 req:4903 conn:1:1 sconn:31 | pocket_api_development.find | STARTED | {"find"=>"sheets", "filter"=>{"_id"=>BSON::ObjectId('5f66aadf1ada720171b1f0ed')}, "limit"=>1, "singleBatch"=>true, "$readPreference"=>{"mode"=>"primaryPreferred"}, "$db"=>"pocket_api_development", "$clusterTime"=>{"clusterTime"=>#<BSON::Timestamp:0x00...
MONGODB | mongo2:30002 req:4903 | pocket_api_development.find | SUCCEEDED | 0.002s
MONGODB | mongo2:30002 req:4904 conn:1:1 sconn:31 | pocket_api_development.find | STARTED | {"find"=>"sheets", "filter"=>{"_id"=>BSON::ObjectId('5f6e08fb1ada72017528a517')}, "limit"=>1, "singleBatch"=>true, "$readPreference"=>{"mode"=>"primaryPreferred"}, "$db"=>"pocket_api_development", "$clusterTime"=>{"clusterTime"=>#<BSON::Timestamp:0x00...
MONGODB | mongo2:30002 req:4904 | pocket_api_development.find | SUCCEEDED | 0.002s
MONGODB | mongo2:30002 req:4905 conn:1:1 sconn:31 | pocket_api_development.find | STARTED | {"find"=>"sheets", "filter"=>{"_id"=>BSON::ObjectId('5f6e9f601ada720171d0baa4')}, "limit"=>1, "singleBatch"=>true, "$readPreference"=>{"mode"=>"primaryPreferred"}, "$db"=>"pocket_api_development", "$clusterTime"=>{"clusterTime"=>#<BSON::Timestamp:0x00...

[... ad infinitum ...]

Conclusion πŸ’¬

Looks we are still having N+1 Query Problem for nested associations, even after using the nested includes approach implemented on this changelog.

Could you check why it is not working at all?

Thanks!

Versions πŸ“

  1. Ruby: ruby 2.6.8p205
  2. Rails: Rails 6.0.5.1
  3. Mongoid: 7.0.13
  4. Mongoid Includes: 3.0.0

@dblock
Copy link
Contributor

@dblock dblock commented on 7cca240 Sep 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Victorcorcos You should open an issue.

@Victorcorcos
Copy link

@Victorcorcos Victorcorcos commented on 7cca240 Sep 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @dblock !

The issue is opened and already fixed by mongoid maintainers!

β†’ https://jira.mongodb.org/browse/MONGOID-5473

Please sign in to comment.