diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 60e4a9c2ad..c6c2f00309 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Introduce preload query strategy for eager :includes. #9640 [Frederick Cheung, Aleksey Kondratenko] + * Support aggregations in finder conditions. #10572 [Ryan Kinderman] * Organize and clean up the Active Record test suite. #10742 [John Barnette] diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 36c1a2bd31..4d85c24584 100755 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -43,6 +43,7 @@ require 'active_record/validations' require 'active_record/callbacks' require 'active_record/reflection' require 'active_record/associations' +require 'active_record/association_preload' require 'active_record/aggregations' require 'active_record/transactions' require 'active_record/timestamp' @@ -63,6 +64,7 @@ ActiveRecord::Base.class_eval do include ActiveRecord::Observing include ActiveRecord::Timestamp include ActiveRecord::Associations + include ActiveRecord::AssociationPreload include ActiveRecord::Aggregations include ActiveRecord::Transactions include ActiveRecord::Reflection diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb new file mode 100644 index 0000000000..6fa49dfbe8 --- /dev/null +++ b/activerecord/lib/active_record/association_preload.rb @@ -0,0 +1,231 @@ +module ActiveRecord + module AssociationPreload #:nodoc: + def self.included(base) + base.extend(ClassMethods) + end + + module ClassMethods + + # Loads the named associations for the activerecord record (or records) given + # preload_options is passed only one level deep: don't pass to the child associations when associations is a Hash + protected + def preload_associations(records, associations, preload_options={}) + records = [records].flatten.compact + return if records.empty? + case associations + when Array then associations.each {|association| preload_associations(records, association, preload_options)} + when Symbol, String then preload_one_association(records, associations.to_sym, preload_options) + when Hash then + associations.each do |parent, child| + raise "parent must be an association name" unless parent.is_a?(String) || parent.is_a?(Symbol) + preload_associations(records, parent, preload_options) + reflection = reflections[parent] + parents = records.map {|record| record.send(reflection.name)}.flatten + unless parents.empty? + parents.first.class.preload_associations(parents, child) + end + end + end + end + + private + + def preload_one_association(records, association, preload_options={}) + reflection = reflections[association] + raise ConfigurationError, "Association named '#{ association }' was not found; perhaps you misspelled it?" unless reflection + + send(:"preload_#{reflection.macro}_association", records, reflection, preload_options) + end + + def add_preloaded_records_to_collection(parent_records, reflection_name, associated_record) + parent_records.each do |parent_record| + association_proxy = parent_record.send(reflection_name) + association_proxy.loaded + association_proxy.target.push(*[associated_record].flatten) + end + end + + def set_association_collection_records(id_to_record_map, reflection_name, associated_records, key) + associated_records.each do |associated_record| + mapped_records = id_to_record_map[associated_record[key].to_i] + add_preloaded_records_to_collection(mapped_records, reflection_name, associated_record) + end + end + + def set_association_single_records(id_to_record_map, reflection_name, associated_records, key) + associated_records.each do |associated_record| + mapped_records = id_to_record_map[associated_record[key].to_i] + mapped_records.each do |mapped_record| + mapped_record.send("set_#{reflection_name}_target", associated_record) + end + end + end + + def construct_id_map(records) + id_to_record_map = {} + ids = [] + records.each do |record| + ids << record.id + mapped_records = (id_to_record_map[record.id] ||= []) + mapped_records << record + end + ids.uniq! + return id_to_record_map, ids + end + + # FIXME: quoting + def preload_has_and_belongs_to_many_association(records, reflection, preload_options={}) + table_name = reflection.klass.table_name + id_to_record_map, ids = construct_id_map(records) + records.each {|record| record.send(reflection.name).loaded} + options = reflection.options + + conditions = "t0.#{reflection.primary_key_name} IN (?)" + conditions << append_conditions(options, preload_options) + + associated_records = reflection.klass.find(:all, :conditions => [conditions, ids], + :include => options[:include], + :joins => "INNER JOIN #{options[:join_table]} as t0 ON #{reflection.klass.table_name}.#{reflection.klass.primary_key} = t0.#{reflection.association_foreign_key}", + :select => "#{options[:select] || table_name+'.*'}, t0.#{reflection.primary_key_name} as _parent_record_id", + :order => options[:order]) + + set_association_collection_records(id_to_record_map, reflection.name, associated_records, '_parent_record_id') + end + + def preload_has_one_association(records, reflection, preload_options={}) + id_to_record_map, ids = construct_id_map(records) + records.each {|record| record.send("set_#{reflection.name}_target", nil)} + + set_association_single_records(id_to_record_map, reflection.name, find_associated_records(ids, reflection, preload_options), + reflection.primary_key_name) + end + + def preload_has_many_association(records, reflection, preload_options={}) + id_to_record_map, ids = construct_id_map(records) + records.each {|record| record.send(reflection.name).loaded} + options = reflection.options + + if options[:through] + through_records = preload_through_records(records, reflection, options[:through]) + through_reflection = reflections[options[:through]] + through_primary_key = through_reflection.primary_key_name + unless through_records.empty? + source = reflection.source_reflection.name + through_records.first.class.preload_associations(through_records, source) + through_records.compact.each do |through_record| + add_preloaded_records_to_collection(id_to_record_map[through_record[through_primary_key].to_i], + reflection.name, through_record.send(source)) + end + end + else + set_association_collection_records(id_to_record_map, reflection.name, find_associated_records(ids, reflection, preload_options), + reflection.primary_key_name) + end + end + + def preload_through_records(records, reflection, through_association) + through_reflection = reflections[through_association] + through_primary_key = through_reflection.primary_key_name + + if reflection.options[:source_type] + interface = reflection.source_reflection.options[:foreign_type] + preload_options = {:conditions => ["#{interface} = ?", reflection.options[:source_type]]} + + records.first.class.preload_associations(records, through_association, preload_options) + + # Dont cache the association - we would only be caching a subset + through_records = [] + records.compact.each do |record| + proxy = record.send(through_association) + through_records << proxy.target + proxy.reset + end + through_records = through_records.flatten + else + records.first.class.preload_associations(records, through_association) + through_records = records.compact.map {|record| record.send(through_association)}.flatten + end + end + + # FIXME: quoting + def preload_belongs_to_association(records, reflection, preload_options={}) + options = reflection.options + primary_key_name = reflection.primary_key_name + + if options[:polymorphic] + polymorph_type = options[:foreign_type] + klasses_and_ids = {} + + # Construct a mapping from klass to a list of ids to load and a mapping of those ids back to their parent_records + records.each do |record| + klass = record.send(polymorph_type) + klass_id = record.send(primary_key_name) + + id_map = klasses_and_ids[klass] ||= {} + id_list_for_klass_id = (id_map[klass_id] ||= []) + id_list_for_klass_id << record + end + klasses_and_ids = klasses_and_ids.to_a + else + id_map = {} + records.each do |record| + mapped_records = (id_map[record.send(primary_key_name)] ||= []) + mapped_records << record + end + klasses_and_ids = [[reflection.klass.name, id_map]] + end + + klasses_and_ids.each do |klass_and_id| + klass_name, id_map = *klass_and_id + klass = klass_name.constantize + + table_name = klass.table_name + conditions = "#{table_name}.#{primary_key} IN (?)" + conditions << append_conditions(options, preload_options) + associated_records = klass.find(:all, :conditions => [conditions, id_map.keys.uniq], + :include => options[:include], + :select => options[:select], + :joins => options[:joins], + :order => options[:order]) + set_association_single_records(id_map, reflection.name, associated_records, 'id') + end + end + + # FIXME: quoting + def find_associated_records(ids, reflection, preload_options) + options = reflection.options + table_name = reflection.klass.table_name + + if interface = reflection.options[:as] + conditions = "#{reflection.klass.table_name}.#{interface}_id IN (?) and #{reflection.klass.table_name}.#{interface}_type = '#{self.base_class.name.demodulize}'" + else + foreign_key = reflection.primary_key_name + conditions = "#{reflection.klass.table_name}.#{foreign_key} IN (?)" + end + + conditions << append_conditions(options, preload_options) + + reflection.klass.find(:all, + :select => (options[:select] || "#{table_name}.*"), + :include => options[:include], + :conditions => [conditions, ids], + :joins => options[:joins], + :group => options[:group], + :order => options[:order]) + end + + + def interpolate_sql_for_preload(sql) + instance_eval("%@#{sql.gsub('@', '\@')}@") + end + + def append_conditions(options, preload_options) + sql = "" + sql << " AND (#{interpolate_sql_for_preload(sanitize_sql(options[:conditions]))})" if options[:conditions] + sql << " AND (#{sanitize_sql preload_options[:conditions]})" if preload_options[:conditions] + sql + end + + end + end +end diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 8434644325..4743d3be8f 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1365,11 +1365,15 @@ module ActiveRecord def include_eager_order?(options) order = options[:order] return false unless order - order.scan(/([\.\w]+)\.\w+/).flatten.any? do |order_table_name| + order.to_s.scan(/([\.\w]+)\.\w+/).flatten.any? do |order_table_name| order_table_name != table_name end end + def references_eager_loaded_tables?(options) + include_eager_order?(options) || include_eager_conditions?(options) + end + def using_limitable_reflections?(reflections) reflections.reject { |r| [ :belongs_to, :has_one ].include?(r.macro) }.length.zero? end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 2d2f2297ed..dfde2e6e1d 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1237,9 +1237,16 @@ module ActiveRecord #:nodoc: end def find_every(options) - records = scoped?(:find, :include) || options[:include] ? - find_with_associations(options) : - find_by_sql(construct_finder_sql(options)) + include_associations = merge_includes(scope(:find, :include), options[:include]) + + if include_associations.any? && references_eager_loaded_tables?(options) + records = find_with_associations(options) + else + records = find_by_sql(construct_finder_sql(options)) + if include_associations.any? + preload_associations(records, include_associations) + end + end records.each { |record| record.readonly! } if options[:readonly] diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 0e351816d3..9ceb507e91 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -1,5 +1,6 @@ require 'abstract_unit' require 'models/post' +require 'models/tagging' require 'models/comment' require 'models/author' require 'models/category' @@ -9,7 +10,7 @@ require 'models/reader' class EagerAssociationTest < ActiveSupport::TestCase fixtures :posts, :comments, :authors, :categories, :categories_posts, - :companies, :accounts, :tags, :people, :readers + :companies, :accounts, :tags, :taggings, :people, :readers def test_loading_with_one_association posts = Post.find(:all, :include => :comments) @@ -56,6 +57,13 @@ class EagerAssociationTest < ActiveSupport::TestCase assert posts.first.comments.include?(comments(:greetings)) end + def test_duplicate_middle_objects + comments = Comment.find :all, :conditions => 'post_id = 1', :include => [:post => :author] + assert_no_queries do + comments.each {|comment| comment.post.author.name} + end + end + def test_loading_from_an_association posts = authors(:david).posts.find(:all, :include => :comments, :order => "posts.id") assert_equal 2, posts.first.comments.size @@ -353,6 +361,17 @@ class EagerAssociationTest < ActiveSupport::TestCase assert_equal posts(:sti_post_and_comments, :sti_comments), Post.find(:all, :include => [:author, :comments], :conditions => "authors.name = 'David'", :order => 'UPPER(posts.title) DESC, posts.id', :limit => 2, :offset => 1) end + def test_preload_with_interpolation + assert_equal [comments(:greetings)], Post.find(posts(:welcome).id, :include => :comments_with_interpolated_conditions).comments_with_interpolated_conditions + end + + def test_polymorphic_type_condition + post = Post.find(posts(:thinking).id, :include => :taggings) + assert post.taggings.include?(taggings(:thinking_general)) + post = SpecialPost.find(posts(:thinking).id, :include => :taggings) + assert post.taggings.include?(taggings(:thinking_general)) + end + def test_eager_with_multiple_associations_with_same_table_has_many_and_habtm # Eager includes of has many and habtm associations aren't necessarily sorted in the same way def assert_equal_after_sort(item1, item2, item3 = nil) @@ -405,34 +424,40 @@ class EagerAssociationTest < ActiveSupport::TestCase def test_preconfigured_includes_with_belongs_to author = posts(:welcome).author_with_posts - assert_equal 5, author.posts.size + assert_no_queries {assert_equal 5, author.posts.size} end def test_preconfigured_includes_with_has_one comment = posts(:sti_comments).very_special_comment_with_post - assert_equal posts(:sti_comments), comment.post + assert_no_queries {assert_equal posts(:sti_comments), comment.post} end def test_preconfigured_includes_with_has_many posts = authors(:david).posts_with_comments one = posts.detect { |p| p.id == 1 } - assert_equal 5, posts.size - assert_equal 2, one.comments.size + assert_no_queries do + assert_equal 5, posts.size + assert_equal 2, one.comments.size + end end def test_preconfigured_includes_with_habtm posts = authors(:david).posts_with_categories one = posts.detect { |p| p.id == 1 } - assert_equal 5, posts.size - assert_equal 2, one.categories.size + assert_no_queries do + assert_equal 5, posts.size + assert_equal 2, one.categories.size + end end def test_preconfigured_includes_with_has_many_and_habtm posts = authors(:david).posts_with_comments_and_categories one = posts.detect { |p| p.id == 1 } - assert_equal 5, posts.size - assert_equal 2, one.comments.size - assert_equal 2, one.categories.size + assert_no_queries do + assert_equal 5, posts.size + assert_equal 2, one.comments.size + assert_equal 2, one.categories.size + end end def test_count_with_include diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 23ed9d1c5e..c8ae42d78c 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -317,7 +317,7 @@ class AssociationsJoinModelTest < ActiveSupport::TestCase assert_equal posts(:welcome, :thinking), tags(:general).taggables end assert_raise ActiveRecord::EagerLoadPolymorphicError do - assert_equal posts(:welcome, :thinking), tags(:general).taggings.find(:all, :include => :taggable) + assert_equal posts(:welcome, :thinking), tags(:general).taggings.find(:all, :include => :taggable, :conditions => 'bogus_table.column = 1') end end @@ -331,6 +331,7 @@ class AssociationsJoinModelTest < ActiveSupport::TestCase assert_no_queries do assert_equal desired, tag_with_include.tagged_posts end + assert_equal 4, tag_with_include.taggings.length end def test_has_many_through_has_many_find_all @@ -546,6 +547,62 @@ class AssociationsJoinModelTest < ActiveSupport::TestCase assert_equal comment_ids.sort.reverse, authors(:david).ordered_uniq_comments_desc.map(&:id) end + def test_polymorphic_has_many + expected = taggings(:welcome_general) + p = Post.find(posts(:welcome).id, :include => :taggings) + assert_no_queries {assert p.taggings.include?(expected)} + assert posts(:welcome).taggings.include?(taggings(:welcome_general)) + end + + def test_polymorphic_has_one + expected = posts(:welcome) + + tagging = Tagging.find(taggings(:welcome_general).id, :include => :taggable) + assert_no_queries { assert_equal expected, tagging.taggable} + end + + def test_polymorphic_belongs_to + p = Post.find(posts(:welcome).id, :include => {:taggings => :taggable}) + assert_no_queries {assert_equal posts(:welcome), p.taggings.first.taggable} + end + + def test_preload_polymorphic_has_many_through + posts = Post.find(:all, :order => 'posts.id') + posts_with_tags = Post.find(:all, :include => :tags, :order => 'posts.id') + assert_equal posts.length, posts_with_tags.length + posts.length.times do |i| + assert_equal posts[i].tags.length, assert_no_queries { posts_with_tags[i].tags.length } + end + end + + def test_preload_polymorph_many_types + taggings = Tagging.find :all, :include => :taggable, :conditions => ['taggable_type != ?', 'FakeModel'] + assert_no_queries do + taggings.first.taggable.id + taggings[1].taggable.id + end + + taggables = taggings.map(&:taggable) + assert taggables.include?(items(:dvd)) + assert taggables.include?(posts(:welcome)) + end + + def test_preload_polymorphic_has_many + posts = Post.find(:all, :order => 'posts.id') + posts_with_taggings = Post.find(:all, :include => :taggings, :order => 'posts.id') + assert_equal posts.length, posts_with_taggings.length + posts.length.times do |i| + assert_equal posts[i].taggings.length, assert_no_queries { posts_with_taggings[i].taggings.length } + end + end + + def test_belongs_to_shared_parent + comments = Comment.find(:all, :include => :post, :conditions => 'post_id = 1') + assert_no_queries do + assert_equal comments.first.post, comments[1].post + end + end + private # create dynamic Post models to allow different dependency options def find_post_with_dependency(post_id, association, association_name, dependency) diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index 451a17b3bf..b21f0cc9e2 100755 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -21,6 +21,11 @@ class AssociationsTest < ActiveSupport::TestCase fixtures :accounts, :companies, :developers, :projects, :developers_projects, :computers + def test_include_with_order_works + assert_nothing_raised {Account.find(:first, :order => 'id', :include => :firm)} + assert_nothing_raised {Account.find(:first, :order => :id, :include => :firm)} + end + def test_bad_collection_keys assert_raise(ArgumentError, 'ActiveRecord should have barked on bad collection keys') do Class.new(ActiveRecord::Base).has_many(:wheels, :name => 'wheels') diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 53dcbb74f3..176a0ddc1e 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -13,6 +13,9 @@ class Post < ActiveRecord::Base end end + has_many :comments_with_interpolated_conditions, :class_name => 'Comment', + :conditions => ['#{"#{aliased_table_name}." rescue ""}body = ?', 'Thank you for the welcome'] + has_one :very_special_comment has_one :very_special_comment_with_post, :class_name => "VerySpecialComment", :include => :post has_many :special_comments