Skip to content

Commit

Permalink
Use apply_join_dependency instead of meaningless named `find_with_a…
Browse files Browse the repository at this point in the history
…ssociations`

`find_with_associations` is meaningless name in this point since it just
contain `construct_join_dependency` and `apply_join_dependency`, does
not contain finding anything.
If `apply_join_dependency` returns `relation` and `join_dependency` then
`find_with_associations` is no longer needed.
  • Loading branch information
kamipo committed Jan 10, 2018
1 parent b4f64cb commit 412db71
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 26 deletions.
4 changes: 2 additions & 2 deletions activerecord/lib/active_record/relation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ def to_sql
relation = self

if eager_loading?
find_with_associations { |rel, _| relation = rel }
apply_join_dependency { |rel, _| relation = rel }
end

conn = klass.connection
Expand Down Expand Up @@ -533,7 +533,7 @@ def exec_queries(&block)
skip_query_cache_if_necessary do
@records =
if eager_loading?
find_with_associations do |relation, join_dependency|
apply_join_dependency do |relation, join_dependency|
if ActiveRecord::NullRelation === relation
[]
else
Expand Down
35 changes: 11 additions & 24 deletions activerecord/lib/active_record/relation/finder_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -358,24 +358,6 @@ def offset_index
offset_value || 0
end

def find_with_associations
# NOTE: the JoinDependency constructed here needs to know about
# any joins already present in `self`, so pass them in
#
# failing to do so means that in cases like activerecord/test/cases/associations/inner_join_association_test.rb:136
# incorrect SQL is generated. In that case, the join dependency for
# SpecialCategorizations is constructed without knowledge of the
# preexisting join in joins_values to categorizations (by way of
# the `has_many :through` for categories).
#
join_dependency = construct_join_dependency

relation = apply_join_dependency(join_dependency)
relation._select!(join_dependency.aliases.columns)

yield relation, join_dependency
end

def construct_relation_for_exists(conditions)
relation = except(:select, :distinct, :order)._select!(ONE_AS_ONE).limit!(1)

Expand All @@ -396,18 +378,23 @@ def construct_join_dependency(eager_loading: true)
)
end

def apply_join_dependency(join_dependency = nil, eager_loading: true)
join_dependency ||= construct_join_dependency(eager_loading: eager_loading)
def apply_join_dependency(eager_loading: true)
join_dependency = construct_join_dependency(eager_loading: eager_loading)
relation = except(:includes, :eager_load, :preload).joins!(join_dependency)

if !eager_loading || using_limitable_reflections?(join_dependency.reflections)
relation
else
if eager_loading && !using_limitable_reflections?(join_dependency.reflections)
if has_limit_or_offset?
limited_ids = limited_ids_for(relation)
limited_ids.empty? ? relation.none! : relation.where!(primary_key => limited_ids)
end
relation.except(:limit, :offset)
relation.limit_value = relation.offset_value = nil
end

if block_given?
relation._select!(join_dependency.aliases.columns)
yield relation, join_dependency
else
relation
end
end

Expand Down

0 comments on commit 412db71

Please sign in to comment.