Skip to content

Commit 6074c59

Browse files
committed
Call run after preloading records
Ref: rails#41385 Otherwise the association isn't marked as loaded, and a ThroughAssociation might perform the same query uselessly. `already_loaded?` also has to be made lazy, because rails#41285 made it so that alls the preloaders are instiated together before being ran.
1 parent ee7cf8c commit 6074c59

File tree

3 files changed

+35
-7
lines changed

3 files changed

+35
-7
lines changed

activerecord/lib/active_record/associations/preloader.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ def call
103103

104104
loaders = build_preloaders
105105
group_and_load_similar(loaders)
106-
loaders.map(&:run)
106+
loaders.each(&:run)
107107

108108
child_preloaders.each { |reflection, child, parents| build_child_preloader(reflection, child, parents) }
109109

activerecord/lib/active_record/associations/preloader/association.rb

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@ def self.load_records_in_batch(scope, association_key_name, loaders)
1111
loaders.each { |l| l.set_inverse(record) }
1212
end
1313

14-
loaders.each { |l| l.load_records(raw_records) }
14+
loaders.each do |loader|
15+
loader.load_records(raw_records)
16+
loader.run
17+
end
1518
end
1619

1720
def initialize(klass, owners, reflection, preload_scope, associate_by_default = true)
@@ -21,16 +24,14 @@ def initialize(klass, owners, reflection, preload_scope, associate_by_default =
2124
@preload_scope = preload_scope
2225
@associate = associate_by_default || !preload_scope || preload_scope.empty_scope?
2326
@model = owners.first && owners.first.class
24-
25-
@already_loaded = owners.all? { |o| o.association(reflection.name).loaded? }
2627
end
2728

2829
def already_loaded?
29-
@already_loaded
30+
@already_loaded ||= owners.all? { |o| o.association(reflection.name).loaded? }
3031
end
3132

3233
def run
33-
if @already_loaded
34+
if already_loaded?
3435
fetch_from_preloaded_records
3536
return self
3637
end

activerecord/test/cases/associations_test.rb

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@
3030

3131
class AssociationsTest < ActiveRecord::TestCase
3232
fixtures :accounts, :companies, :developers, :projects, :developers_projects,
33-
:computers, :people, :readers, :authors, :author_addresses, :author_favorites
33+
:computers, :people, :readers, :authors, :author_addresses, :author_favorites,
34+
:comments, :posts
3435

3536
def test_eager_loading_should_not_change_count_of_children
3637
liquid = Liquid.create(name: "salty")
@@ -407,12 +408,30 @@ def test_preload_groups_queries_with_same_scope
407408
assert_queries(1) do
408409
preloader = ActiveRecord::Associations::Preloader.new(records: [book, post], associations: :author)
409410
preloader.call
411+
end
410412

413+
assert_no_queries do
411414
book.author
412415
post.author
413416
end
414417
end
415418

419+
def test_preload_through
420+
comments = [
421+
comments(:eager_sti_on_associations_s_comment1),
422+
comments(:eager_sti_on_associations_s_comment2),
423+
]
424+
425+
assert_queries(2) do
426+
preloader = ActiveRecord::Associations::Preloader.new(records: comments, associations: [:author, :post])
427+
preloader.call
428+
end
429+
430+
assert_no_queries do
431+
comments.each(&:author)
432+
end
433+
end
434+
416435
def test_preload_with_grouping_sets_inverse_association
417436
mary = authors(:mary)
418437
bob = authors(:bob)
@@ -423,7 +442,9 @@ def test_preload_with_grouping_sets_inverse_association
423442
assert_queries(1) do
424443
preloader = ActiveRecord::Associations::Preloader.new(records: favorites, associations: [:author, :favorite_author])
425444
preloader.call
445+
end
426446

447+
assert_no_queries do
427448
favorites.first.author
428449
favorites.first.favorite_author
429450
end
@@ -440,7 +461,9 @@ def test_preload_does_not_group_same_class_different_scope
440461
assert_queries(2) do
441462
preloader = ActiveRecord::Associations::Preloader.new(records: [post, postesque], associations: :author_with_the_letter_a)
442463
preloader.call
464+
end
443465

466+
assert_no_queries do
444467
post.author_with_the_letter_a
445468
postesque.author_with_the_letter_a
446469
end
@@ -452,7 +475,9 @@ def test_preload_does_not_group_same_class_different_scope
452475
assert_queries(3) do
453476
preloader = ActiveRecord::Associations::Preloader.new(records: [post, postesque], associations: :author_with_address)
454477
preloader.call
478+
end
455479

480+
assert_no_queries do
456481
post.author_with_address
457482
postesque.author_with_address
458483
end
@@ -466,7 +491,9 @@ def test_preload_does_not_group_same_scope_different_key_name
466491
assert_queries(2) do
467492
preloader = ActiveRecord::Associations::Preloader.new(records: [post, postesque], associations: :author)
468493
preloader.call
494+
end
469495

496+
assert_no_queries do
470497
post.author
471498
postesque.author
472499
end

0 commit comments

Comments
 (0)