From 412db710dfa6ed84654068576b1841966d7f89b2 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Thu, 11 Jan 2018 04:28:28 +0900 Subject: [PATCH] Use `apply_join_dependency` instead of meaningless named `find_with_associations` `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. --- activerecord/lib/active_record/relation.rb | 4 +-- .../active_record/relation/finder_methods.rb | 35 ++++++------------- 2 files changed, 13 insertions(+), 26 deletions(-) diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 4df3864d07f..10be583ef41 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -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 @@ -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 diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 12ac89f5ad5..98992caa0c6 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -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) @@ -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