Skip to content

Commit b3d7757

Browse files
jhawthornseejohnrunHParkereileencodesDinah Shi
committed
Allow preloads on instance dependent associations
Instance dependent associations are associations which are defined with a scope taking an argument which makes the scope they are selected from dependent on the owner record. This presented a challenge for preloading and eager_loading, since each record's association being loaded could require a different scope. Previously this raised an exception if attempted. This commit adds the ability to preload instance dependent scopes (eager loading is still unsupported). This is done by detecting instance dependent scopes in the preloader and building the scope and Preloader::Association for each record. Because we now merge similar scopes later in the preloading process, any records which end up with the same instance dependent scope will be loaded from a single query. In the case that there's a different scope for each record, this will perform N+1 queries. I think it's up to the app developer to consider this (this is essentially the same as preloading polymorphic scopes). Co-authored-by: John Crepezzi <[email protected]> Co-authored-by: Adam Hess <[email protected]> Co-authored-by: Eileen M. Uchitelle <[email protected]> Co-authored-by: Dinah Shi <[email protected]>
1 parent d6a1cff commit b3d7757

File tree

6 files changed

+141
-42
lines changed

6 files changed

+141
-42
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,12 @@ def load_records_in_batch(loaders)
4242

4343
attr_reader :klass
4444

45-
def initialize(klass, owners, reflection, preload_scope, associate_by_default = true)
45+
def initialize(klass, owners, reflection, preload_scope, reflection_scope, associate_by_default)
4646
@klass = klass
4747
@owners = owners.uniq(&:__id__)
4848
@reflection = reflection
4949
@preload_scope = preload_scope
50+
@reflection_scope = reflection_scope
5051
@associate = associate_by_default || !preload_scope || preload_scope.empty_scope?
5152
@model = owners.first && owners.first.class
5253
@run = false

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

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,19 @@ def grouped_records
8383
end
8484

8585
def preloaders_for_reflection(reflection, reflection_records)
86-
reflection_records.group_by { |record| record.association(reflection.name).klass }.map do |rhs_klass, rs|
87-
preloader_for(reflection).new(rhs_klass, rs, reflection, scope, associate_by_default)
86+
reflection_records.group_by do |record|
87+
klass = record.association(association).klass
88+
89+
if reflection.scope && reflection.scope.arity != 0
90+
# For instance dependent scopes, the scope is potentially
91+
# different for each record. To allow this we'll group each
92+
# object separately into its own preloader
93+
reflection_scope = reflection.join_scopes(klass.arel_table, klass.predicate_builder, klass, record).inject(&:merge!)
94+
end
95+
96+
[klass, reflection_scope]
97+
end.map do |(rhs_klass, reflection_scope), rs|
98+
preloader_for(reflection).new(rhs_klass, rs, reflection, scope, reflection_scope, associate_by_default)
8899
end
89100
end
90101

@@ -124,8 +135,6 @@ def build_children(children)
124135
# and attach it to a relation. The class returned implements a `run` method
125136
# that accepts a preloader.
126137
def preloader_for(reflection)
127-
reflection.check_preloadable!
128-
129138
if reflection.options[:through]
130139
ThroughAssociation
131140
else

activerecord/lib/active_record/reflection.rb

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -194,9 +194,9 @@ def join_scope(table, foreign_table, foreign_klass)
194194
klass_scope
195195
end
196196

197-
def join_scopes(table, predicate_builder, klass = self.klass) # :nodoc:
197+
def join_scopes(table, predicate_builder, klass = self.klass, record = nil) # :nodoc:
198198
if scope
199-
[scope_for(build_scope(table, predicate_builder, klass))]
199+
[scope_for(build_scope(table, predicate_builder, klass), record)]
200200
else
201201
[]
202202
end
@@ -483,18 +483,17 @@ def check_validity!
483483
check_validity_of_inverse!
484484
end
485485

486-
def check_preloadable!
486+
def check_eager_loadable!
487487
return unless scope
488488

489489
unless scope.arity == 0
490490
raise ArgumentError, <<-MSG.squish
491491
The association scope '#{name}' is instance dependent (the scope
492-
block takes an argument). Preloading instance dependent scopes is
493-
not supported.
492+
block takes an argument). Eager loading instance dependent scopes
493+
is not supported.
494494
MSG
495495
end
496496
end
497-
alias :check_eager_loadable! :check_preloadable!
498497

499498
def join_id_for(owner) # :nodoc:
500499
owner[join_foreign_key]
@@ -842,8 +841,8 @@ def scopes
842841
source_reflection.scopes + super
843842
end
844843

845-
def join_scopes(table, predicate_builder, klass = self.klass) # :nodoc:
846-
source_reflection.join_scopes(table, predicate_builder, klass) + super
844+
def join_scopes(table, predicate_builder, klass = self.klass, record = nil) # :nodoc:
845+
source_reflection.join_scopes(table, predicate_builder, klass, record) + super
847846
end
848847

849848
def has_scope?
@@ -1015,9 +1014,9 @@ def initialize(reflection, previous_reflection)
10151014
@previous_reflection = previous_reflection
10161015
end
10171016

1018-
def join_scopes(table, predicate_builder, klass = self.klass) # :nodoc:
1019-
scopes = @previous_reflection.join_scopes(table, predicate_builder) + super
1020-
scopes << build_scope(table, predicate_builder, klass).instance_exec(nil, &source_type_scope)
1017+
def join_scopes(table, predicate_builder, klass = self.klass, record = nil) # :nodoc:
1018+
scopes = @previous_reflection.join_scopes(table, predicate_builder, record) + super
1019+
scopes << build_scope(table, predicate_builder, klass).instance_exec(record, &source_type_scope)
10211020
end
10221021

10231022
def constraints

activerecord/test/cases/associations/eager_test.rb

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1493,36 +1493,32 @@ def test_preloading_has_many_through_with_custom_scope
14931493
assert_equal pets(:parrot), Owner.including_last_pet.first.last_pet
14941494
end
14951495

1496-
test "preloading and eager loading of instance dependent associations is not supported" do
1497-
message = "association scope 'posts_with_signature' is"
1498-
error = assert_raises(ArgumentError) do
1499-
Author.includes(:posts_with_signature).to_a
1496+
test "preloading of instance dependent associations is supported" do
1497+
authors = Author.preload(:posts_with_signature).to_a
1498+
assert_not authors.empty?
1499+
authors.each do |author|
1500+
assert_predicate author.posts_with_signature, :loaded?
15001501
end
1501-
assert_match message, error.message
1502-
1503-
error = assert_raises(ArgumentError) do
1504-
Author.preload(:posts_with_signature).to_a
1505-
end
1506-
assert_match message, error.message
1502+
end
15071503

1504+
test "eager loading of instance dependent associations is not supported" do
1505+
message = "association scope 'posts_with_signature' is"
15081506
error = assert_raises(ArgumentError) do
15091507
Author.eager_load(:posts_with_signature).to_a
15101508
end
15111509
assert_match message, error.message
15121510
end
15131511

1514-
test "preloading and eager loading of optional instance dependent associations is not supported" do
1515-
message = "association scope 'posts_mentioning_author' is"
1516-
error = assert_raises(ArgumentError) do
1517-
Author.includes(:posts_mentioning_author).to_a
1512+
test "preloading of optional instance dependent associations is supported" do
1513+
authors = Author.includes(:posts_mentioning_author).to_a
1514+
assert_not authors.empty?
1515+
authors.each do |author|
1516+
assert_predicate author.posts_mentioning_author, :loaded?
15181517
end
1519-
assert_match message, error.message
1520-
1521-
error = assert_raises(ArgumentError) do
1522-
Author.preload(:posts_mentioning_author).to_a
1523-
end
1524-
assert_match message, error.message
1518+
end
15251519

1520+
test "eager loading of optional instance dependent associations is not supported" do
1521+
message = "association scope 'posts_mentioning_author' is"
15261522
error = assert_raises(ArgumentError) do
15271523
Author.eager_load(:posts_mentioning_author).to_a
15281524
end
@@ -1542,11 +1538,12 @@ def test_preloading_has_many_through_with_custom_scope
15421538
end
15431539
end
15441540

1545-
test "including associations with extensions and an instance dependent scope is not supported" do
1546-
e = assert_raises(ArgumentError) do
1547-
Author.includes(:posts_with_extension_and_instance).to_a
1541+
test "including associations with extensions and an instance dependent scope is supported" do
1542+
authors = Author.includes(:posts_with_extension_and_instance).to_a
1543+
assert_not authors.empty?
1544+
authors.each do |author|
1545+
assert_predicate author.posts_with_extension_and_instance, :loaded?
15481546
end
1549-
assert_match(/Preloading instance dependent scopes is not supported/, e.message)
15501547
end
15511548

15521549
test "preloading readonly association" do

activerecord/test/cases/associations_test.rb

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,97 @@ def test_preload_grouped_queries_of_through_records
461461
end
462462
end
463463

464+
def test_preload_with_instance_dependent_scope
465+
david = authors(:david)
466+
david2 = Author.create!(name: "David")
467+
bob = authors(:bob)
468+
post = Post.create!(
469+
author: david,
470+
title: "test post",
471+
body: "this post is about David"
472+
)
473+
post2 = Post.create!(
474+
author: david,
475+
title: "test post 2",
476+
body: "this post is also about David"
477+
)
478+
479+
assert_queries(2) do
480+
preloader = ActiveRecord::Associations::Preloader.new(records: [david, david2, bob], associations: :posts_mentioning_author)
481+
preloader.call
482+
end
483+
484+
assert_predicate david.posts_mentioning_author, :loaded?
485+
assert_predicate david2.posts_mentioning_author, :loaded?
486+
assert_predicate bob.posts_mentioning_author, :loaded?
487+
488+
assert_equal [post, post2].sort, david.posts_mentioning_author.sort
489+
assert_equal [], david2.posts_mentioning_author
490+
assert_equal [], bob.posts_mentioning_author
491+
end
492+
493+
def test_preload_with_instance_dependent_through_scope
494+
david = authors(:david)
495+
david2 = Author.create!(name: "David")
496+
bob = authors(:bob)
497+
comment1 = david.posts.first.comments.create!(
498+
body: "Great post david!"
499+
)
500+
comment2 = david.posts.first.comments.create!(
501+
body: "I don't agree david"
502+
)
503+
504+
assert_queries(2) do
505+
preloader = ActiveRecord::Associations::Preloader.new(records: [david, david2, bob], associations: :comments_mentioning_author)
506+
preloader.call
507+
end
508+
509+
assert_predicate david.comments_mentioning_author, :loaded?
510+
assert_predicate david2.comments_mentioning_author, :loaded?
511+
assert_predicate bob.comments_mentioning_author, :loaded?
512+
513+
assert_equal [comment1, comment2].sort, david.comments_mentioning_author.sort
514+
assert_equal [], david2.comments_mentioning_author
515+
assert_equal [], bob.comments_mentioning_author
516+
end
517+
518+
def test_preload_with_through_instance_dependent_scope
519+
david = authors(:david)
520+
david2 = Author.create!(name: "David")
521+
bob = authors(:bob)
522+
post = Post.create!(
523+
author: david,
524+
title: "test post",
525+
body: "this post is about David"
526+
)
527+
Post.create!(
528+
author: david,
529+
title: "test post 2",
530+
body: "this post is also about David"
531+
)
532+
post3 = Post.create!(
533+
author: bob,
534+
title: "test post 3",
535+
body: "this post is about Bob"
536+
)
537+
comment1 = post.comments.create!(body: "hi!")
538+
comment2 = post.comments.create!(body: "hello!")
539+
comment3 = post3.comments.create!(body: "HI BOB!")
540+
541+
assert_queries(3) do
542+
preloader = ActiveRecord::Associations::Preloader.new(records: [david, david2, bob], associations: :comments_on_posts_mentioning_author)
543+
preloader.call
544+
end
545+
546+
assert_predicate david.comments_on_posts_mentioning_author, :loaded?
547+
assert_predicate david2.comments_on_posts_mentioning_author, :loaded?
548+
assert_predicate bob.comments_on_posts_mentioning_author, :loaded?
549+
550+
assert_equal [comment1, comment2].sort, david.comments_on_posts_mentioning_author.sort
551+
assert_equal [], david2.comments_on_posts_mentioning_author
552+
assert_equal [comment3], bob.comments_on_posts_mentioning_author
553+
end
554+
464555
def test_some_already_loaded_associations
465556
item_discount = Discount.create(amount: 5)
466557
shipping_discount = Discount.create(amount: 20)

activerecord/test/models/author.rb

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,10 @@ def ratings
205205
has_many :posts_with_default_include, class_name: "PostWithDefaultInclude"
206206
has_many :comments_on_posts_with_default_include, through: :posts_with_default_include, source: :comments
207207

208-
has_many :posts_with_signature, ->(record) { where("posts.title LIKE ?", "%by #{record.name.downcase}%") }, class_name: "Post"
209-
has_many :posts_mentioning_author, ->(record = nil) { where("posts.body LIKE ?", "%#{record&.name&.downcase}%") }, class_name: "Post"
208+
has_many :posts_with_signature, ->(record) { where(arel_table[:title].matches("%by #{record.name.downcase}%")) }, class_name: "Post"
209+
has_many :posts_mentioning_author, ->(record = nil) { where(arel_table[:body].matches("%#{record&.name&.downcase}%")) }, class_name: "Post"
210+
has_many :comments_on_posts_mentioning_author, through: :posts_mentioning_author, source: :comments
211+
has_many :comments_mentioning_author, ->(record) { where(arel_table[:body].matches("%#{record.name.downcase}%")) }, through: :posts, source: :comments
210212

211213
has_one :recent_post, -> { order(id: :desc) }, class_name: "Post"
212214
has_one :recent_response, through: :recent_post, source: :comments

0 commit comments

Comments
 (0)