Skip to content

Commit

Permalink
Merge pull request #23277 from kbrock/preload_associations
Browse files Browse the repository at this point in the history
Avoid scope query when preloading
  • Loading branch information
Fryguy authored Nov 22, 2024
2 parents 66de366 + 0da1cae commit 82d5b62
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 48 deletions.
4 changes: 1 addition & 3 deletions lib/miq_preloader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ module MiqPreloader
# Currently an array does not work
# @return [Array<ActiveRecord::Base>] records
def self.preload(records, associations, preload_scope = nil)
# Rails 7 changed the interface. See rails commit: e3b9779cb701c63012bc1af007c71dc5a888d35a
# Note, added Array(records) as it could be a single element
ActiveRecord::Associations::Preloader.new(:records => Array(records), :associations => associations, :available_records => Array(preload_scope)).call
ActiveRecord::Associations::Preloader.new(:records => Array(records), :associations => associations, :scope => preload_scope).call
end

# for a record, cache results. Also cache the children's links back
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/extensions/ar_to_model_hash_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@

def assert_preloaded(associations)
allow(ActiveRecord::Associations::Preloader).to receive(:new)
.with(:records => kind_of(Array), :associations => array_including(associations), :available_records => kind_of(Array))
.with(:records => kind_of(Array), :associations => array_including(associations), :scope => nil)
.and_return(mocked_preloader)
expect(mocked_preloader).to receive(:call)

Expand Down
48 changes: 4 additions & 44 deletions spec/lib/miq_preloader_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,38 +46,28 @@
emses = ExtManagementSystem.all.load
vms = Vm.where(:ems_id => emses.select(:id))

# TODO: With rails 7, the query count below increased by 1.
# This PR says only the singular associations are preloaded in rails 7:
# https://github.com/rails/rails/pull/42654
expect { preload(emses, :vms, vms) }.to make_database_queries(:count => 2)
expect { preload(emses, :vms, vms) }.to make_database_queries(:count => 1)
expect { expect(emses.first.vms.size).to eq(2) }.not_to make_database_queries
end

# original behavior - not calling our code
it "preloads with an unloaded simple relation" do
vms = Vm.where(:ems_id => ems.id)
vms2 = vms.order(:id).to_a # preloaded vms to be used in tests
vms2 = vms.order(:id).load # preloaded vms to be used in tests

# TODO: With rails 7, the query count below increased by 1.
# This PR says only the singular associations are preloaded in rails 7:
# https://github.com/rails/rails/pull/42654
expect { preload(ems, :vms, vms) }.to make_database_queries(:count => 2)
expect { preload(ems, :vms, vms) }.to make_database_queries(:count => 1)

expect { preload(ems, :vms, vms) }.not_to make_database_queries
expect { expect(ems.vms).to match_array(vms2) }.not_to make_database_queries

# TODO: With rails 7, the query count below decreased by 1.
expect { expect(vms.first.ext_management_system).to eq(ems) }.to make_database_queries(:count => 1)
expect { expect(vms.first.ext_management_system).to eq(ems) }.to make_database_queries(:count => 2)
end

it "preloads with a loaded relation (records is a relation)" do
ems
emses = ExtManagementSystem.all.load
vms = Vm.where(:ems_id => emses.select(:id)).load

# TODO: With rails 7, the query count below increased by 1.
# This PR says only the singular associations are preloaded in rails 7:
# https://github.com/rails/rails/pull/42654
expect { preload(emses, :vms, vms) }.to make_database_queries(:count => 1)

expect { preload(emses, :vms, vms) }.not_to make_database_queries
Expand All @@ -97,36 +87,6 @@
expect { expect(vms.first.ext_management_system).to eq(emses.first) }.not_to make_database_queries
end

it "preloads with an array (records is a relation)" do
ems
emses = ExtManagementSystem.all.load
vms = Vm.where(:ems_id => emses.select(:id)).to_a
expect { preload(emses, :vms, vms) }.to make_database_queries(:count => 1)

expect { preload(emses, :vms, vms) }.not_to make_database_queries
expect { expect(emses.first.vms.size).to eq(2) }.not_to make_database_queries

# TODO: With rails 7, the query count below increased by 1.
# This PR says only the singular associations are preloaded in rails 7:
# https://github.com/rails/rails/pull/42654
expect { expect(vms.first.ext_management_system).to eq(ems) }.to make_database_queries(:count => 1)
end

it "preloads with an array (records is an array)" do
ems
emses = ExtManagementSystem.all.load.to_a
vms = Vm.where(:ems_id => emses.map(&:id)).to_a
expect { preload(emses, :vms, vms) }.to make_database_queries(:count => 1)

expect { preload(emses, :vms, vms) }.not_to make_database_queries
expect { expect(emses.first.vms.size).to eq(2) }.not_to make_database_queries

# TODO: With rails 7, the query count below increased by 1.
# This PR says only the singular associations are preloaded in rails 7:
# https://github.com/rails/rails/pull/42654
expect { expect(vms.first.ext_management_system).to eq(ems) }.to make_database_queries(:count => 1)
end

it "preloads a through with a loaded scope" do
image
nodes = ContainerNode.all.load
Expand Down

0 comments on commit 82d5b62

Please sign in to comment.