From c65c4680f3f58ce7bc1e3440de1aba9a490b7c40 Mon Sep 17 00:00:00 2001 From: Ernie Miller Date: Wed, 20 Oct 2010 22:54:43 -0400 Subject: [PATCH] Fix issue with count when used with association joins and includes together. --- activerecord/lib/active_record/associations.rb | 97 ++++++++++---------- .../associations/cascaded_eager_loading_test.rb | 9 ++ 2 files changed, 57 insertions(+), 49 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index affa2fb..56e721c 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -2011,9 +2011,9 @@ module ActiveRecord association_proxy = record.send("set_#{join_part.reflection.name}_target", association) association_proxy.__send__(:set_inverse_instance, association, record) end - + # A JoinPart represents a part of a JoinDependency. It is an abstract class, inherited - # by JoinBase and JoinAssociation. A JoinBase represents the Active Record which + # by JoinBase and JoinAssociation. A JoinBase represents the Active Record which # everything else is being joined onto. A JoinAssociation represents an association which # is joining to the base. A JoinAssociation may result in more than one actual join # operations (for example a has_and_belongs_to_many JoinAssociation would result in @@ -2023,38 +2023,38 @@ module ActiveRecord # this is the actual base model, for a JoinAssociation this is the target model of the # association. attr_reader :active_record - + delegate :table_name, :column_names, :primary_key, :reflections, :sanitize_sql, :arel_engine, :to => :active_record - + def initialize(active_record) @active_record = active_record @cached_record = {} end - + def ==(other) raise NotImplementedError end - + # An Arel::Table for the active_record def table raise NotImplementedError end - + # The prefix to be used when aliasing columns in the active_record's table def aliased_prefix raise NotImplementedError end - + # The alias for the active_record's table def aliased_table_name raise NotImplementedError end - + # The alias for the primary key of the active_record's table def aliased_primary_key "#{aliased_prefix}_r0" end - + # An array of [column_name, alias] pairs for the table def column_names_with_alias unless defined?(@column_names_with_alias) @@ -2084,7 +2084,7 @@ module ActiveRecord class JoinBase < JoinPart # :nodoc: # Extra joins provided when the JoinDependency was created attr_reader :table_joins - + def initialize(active_record, joins = nil) super(active_record) @table_joins = joins @@ -2092,8 +2092,7 @@ module ActiveRecord def ==(other) other.class == self.class && - other.active_record == active_record && - other.table_joins == table_joins + other.active_record == active_record end def aliased_prefix @@ -2112,39 +2111,39 @@ module ActiveRecord class JoinAssociation < JoinPart # :nodoc: # The reflection of the association represented attr_reader :reflection - + # The JoinDependency object which this JoinAssociation exists within. This is mainly - # relevant for generating aliases which do not conflict with other joins which are + # relevant for generating aliases which do not conflict with other joins which are # part of the query. attr_reader :join_dependency - + # A JoinBase instance representing the active record we are joining onto. # (So in Author.has_many :posts, the Author would be that base record.) attr_reader :parent - + # What type of join will be generated, either Arel::InnerJoin (default) or Arel::OuterJoin attr_accessor :join_type - + # These implement abstract methods from the superclass attr_reader :aliased_prefix, :aliased_table_name - + delegate :options, :through_reflection, :source_reflection, :to => :reflection delegate :table, :table_name, :to => :parent, :prefix => true def initialize(reflection, join_dependency, parent = nil) reflection.check_validity! - + if reflection.options[:polymorphic] raise EagerLoadPolymorphicError.new(reflection) end super(reflection.klass) - + @reflection = reflection @join_dependency = join_dependency @parent = parent @join_type = Arel::InnerJoin - + # This must be done eagerly upon initialisation because the alias which is produced # depends on the state of the join dependency, but we want it to work the same way # every time. @@ -2162,7 +2161,7 @@ module ActiveRecord self.parent == join_part end end - + def join_to(relation) send("join_#{reflection.macro}_to", relation) end @@ -2171,14 +2170,14 @@ module ActiveRecord self.join_type = Arel::OuterJoin joining_relation.joins(self) end - + def table @table ||= Arel::Table.new( table_name, :as => aliased_table_name, :engine => arel_engine, :columns => active_record.columns ) end - + # More semantic name given we are talking about associations alias_method :target_table, :table @@ -2214,43 +2213,43 @@ module ActiveRecord end private - + def allocate_aliases @aliased_prefix = "t#{ join_dependency.join_parts.size }" @aliased_table_name = aliased_table_name_for(table_name) - + if reflection.macro == :has_and_belongs_to_many @aliased_join_table_name = aliased_table_name_for(reflection.options[:join_table], "_join") elsif [:has_many, :has_one].include?(reflection.macro) && reflection.options[:through] @aliased_join_table_name = aliased_table_name_for(reflection.through_reflection.klass.table_name, "_join") end end - + def process_conditions(conditions, table_name) Arel.sql(interpolate_sql(sanitize_sql(conditions, table_name))) end - + def join_target_table(relation, *conditions) relation = relation.join(target_table, join_type) - + # If the target table is an STI model then we must be sure to only include records of # its type and its sub-types. unless active_record.descends_from_active_record? sti_column = target_table[active_record.inheritance_column] - + sti_condition = sti_column.eq(active_record.sti_name) active_record.descendants.each do |subclass| sti_condition = sti_condition.or(sti_column.eq(subclass.sti_name)) end - + conditions << sti_condition end - + # If the reflection has conditions, add them if options[:conditions] conditions << process_conditions(options[:conditions], aliased_table_name) end - + relation = relation.on(*conditions) end @@ -2259,16 +2258,16 @@ module ActiveRecord options[:join_table], :engine => arel_engine, :as => @aliased_join_table_name ) - + fk = options[:foreign_key] || reflection.active_record.to_s.foreign_key klass_fk = options[:association_foreign_key] || reflection.klass.to_s.foreign_key - + relation = relation.join(join_table, join_type) relation = relation.on( join_table[fk]. eq(parent_table[reflection.active_record.primary_key]) ) - + join_target_table( relation, target_table[reflection.klass.primary_key]. @@ -2284,7 +2283,7 @@ module ActiveRecord else foreign_key = options[:foreign_key] || reflection.active_record.name.foreign_key primary_key = options[:primary_key] || parent.primary_key - + join_target_table( relation, target_table[foreign_key]. @@ -2293,20 +2292,20 @@ module ActiveRecord end end alias :join_has_one_to :join_has_many_to - + def join_has_many_through_to(relation) join_table = Arel::Table.new( through_reflection.klass.table_name, :engine => arel_engine, :as => @aliased_join_table_name ) - + jt_conditions = [] jt_foreign_key = first_key = second_key = nil if through_reflection.options[:as] # has_many :through against a polymorphic join as_key = through_reflection.options[:as].to_s jt_foreign_key = as_key + '_id' - + jt_conditions << join_table[as_key + '_type']. eq(parent.active_record.base_class.name) @@ -2331,10 +2330,10 @@ module ActiveRecord end when :belongs_to first_key = primary_key - + if reflection.options[:source_type] second_key = source_reflection.association_foreign_key - + jt_conditions << join_table[reflection.source_reflection.options[:foreign_type]]. eq(reflection.options[:source_type]) @@ -2342,23 +2341,23 @@ module ActiveRecord second_key = source_reflection.primary_key_name end end - + jt_conditions << parent_table[parent.primary_key]. eq(join_table[jt_foreign_key]) - + if through_reflection.options[:conditions] jt_conditions << process_conditions(through_reflection.options[:conditions], aliased_table_name) end - + relation = relation.join(join_table, join_type).on(*jt_conditions) - + join_target_table( relation, target_table[first_key].eq(join_table[second_key]) ) end - + def join_has_many_polymorphic_to(relation) join_target_table( relation, @@ -2372,7 +2371,7 @@ module ActiveRecord def join_belongs_to_to(relation) foreign_key = options[:foreign_key] || reflection.primary_key_name primary_key = options[:primary_key] || reflection.klass.primary_key - + join_target_table( relation, target_table[primary_key].eq(parent_table[foreign_key]) diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb index c7c32da..d1d34f3 100644 --- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb +++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb @@ -3,6 +3,7 @@ require 'models/post' require 'models/comment' require 'models/author' require 'models/categorization' +require 'models/category' require 'models/company' require 'models/topic' require 'models/reply' @@ -45,6 +46,14 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase assert_equal people(:michael), Person.eager_load(:primary_contact => :primary_contact).where('primary_contacts_people_2.first_name = ?', 'Susan').order('people.id').first end + def test_cascaded_eager_association_loading_with_join_for_count + categories = Category.joins(:categorizations).includes([{:posts=>:comments}, :authors]) + + assert_nothing_raised do + assert_equal 2, categories.count + end + end + def test_eager_association_loading_with_join_for_count authors = Author.joins(:special_posts).includes([:posts, :categorizations]) -- 1.7.2.2