mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Fixed eager loading problems with single-table inheritance [Rick Olson] Added smarter table aliasing for eager associations for multiple self joins [Rick Olson] (closes #3580)
git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@3776 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
This commit is contained in:
parent
84b8920a11
commit
4f00c70580
10 changed files with 144 additions and 63 deletions
|
@ -1,5 +1,16 @@
|
|||
*SVN*
|
||||
|
||||
* Added smarter table aliasing for eager associations for multiple self joins #3580 [Rick Olson]
|
||||
|
||||
* The first time a table is referenced in a join, no alias is used.
|
||||
* After that, the parent table name and the reflection name are used.
|
||||
|
||||
Tree.find(:all, :include => :children) # LEFT OUTER JOIN trees AS tress_children ...
|
||||
|
||||
* Any additional join references get a numerical suffix like '_2', '_3', etc.
|
||||
|
||||
* Fixed eager loading problems with single-table inheritance #3580 [Rick Olson]. Post.find(:all, :include => :special_comments) now returns all posts, and any special comments that the posts may have. And made STI work with has_many :through and polymorphic belongs_to.
|
||||
|
||||
* Added cascading eager loading that allows for queries like Author.find(:all, :include=> { :posts=> :comments }), which will fetch all authors, their posts, and the comments belonging to those posts in a single query (using LEFT OUTER JOIN) #3913 [anna@wota.jp]. Examples:
|
||||
|
||||
# cascaded in two levels
|
||||
|
|
|
@ -932,7 +932,6 @@ module ActiveRecord
|
|||
sql << "#{options[:joins]} " if options[:joins]
|
||||
|
||||
add_conditions!(sql, options[:conditions])
|
||||
add_sti_conditions!(sql, join_dependency)
|
||||
add_limited_ids_condition!(sql, options, join_dependency) if !using_limitable_reflections?(join_dependency.reflections) && options[:limit]
|
||||
|
||||
add_limit!(sql, options) if using_limitable_reflections?(join_dependency.reflections)
|
||||
|
@ -950,7 +949,6 @@ module ActiveRecord
|
|||
sql << "#{options[:joins]} " if options[:joins]
|
||||
|
||||
add_conditions!(sql, options[:conditions])
|
||||
add_sti_conditions!(sql, join_dependency)
|
||||
add_limited_ids_condition!(sql, options, join_dependency) if !using_limitable_reflections?(join_dependency.reflections) && options[:limit]
|
||||
|
||||
sql << "ORDER BY #{options[:order]} " if options[:order]
|
||||
|
@ -1011,41 +1009,6 @@ module ActiveRecord
|
|||
reflections.reject { |r| [ :belongs_to, :has_one ].include?(r.macro) }.length.zero?
|
||||
end
|
||||
|
||||
def join_depended_type_condition (klass, join_dependency)
|
||||
aliased_table_name = join_dependency.aliased_table_names_for(klass.table_name).last || klass.table_name
|
||||
quoted_inheritance_column = connection.quote_column_name(klass.inheritance_column)
|
||||
type_condition = klass.subclasses.inject(sti_condition(klass, aliased_table_name, quoted_inheritance_column)) do |condition, subclass|
|
||||
condition << " OR #{sti_condition subclass, aliased_table_name, quoted_inheritance_column}"
|
||||
end
|
||||
|
||||
" (#{type_condition}) "
|
||||
end
|
||||
|
||||
def sti_condition(klass, table_name, inheritance_column)
|
||||
"(#{table_name}.#{inheritance_column} = '#{klass.name.demodulize}' OR #{table_name}.#{inheritance_column} IS NULL)"
|
||||
end
|
||||
|
||||
#def join_depended_type_condition (klass, join_dependency)
|
||||
# aliased_table_name = join_dependency.aliased_table_names_for(klass.table_name).first || klass.table_name
|
||||
# quoted_inheritance_column = connection.quote_column_name(klass.inheritance_column)
|
||||
# type_condition = klass.subclasses.inject("#{aliased_table_name}.#{quoted_inheritance_column} = '#{klass.name.demodulize}' ") do |condition, subclass|
|
||||
# condition << "OR #{aliased_table_name}.#{quoted_inheritance_column} = '#{subclass.name.demodulize}' "
|
||||
# end
|
||||
#
|
||||
# " (#{type_condition}) "
|
||||
#end
|
||||
|
||||
def add_sti_conditions!(sql, join_dependency)
|
||||
reflections = join_dependency.reflections
|
||||
sti_conditions = reflections.collect do |reflection|
|
||||
join_depended_type_condition(reflection.klass, join_dependency) unless reflection.klass.descends_from_active_record?
|
||||
end.compact
|
||||
|
||||
unless sti_conditions.empty?
|
||||
sql << condition_word(sql) + sti_conditions.join(" AND ")
|
||||
end
|
||||
end
|
||||
|
||||
def column_aliases(join_dependency)
|
||||
join_dependency.joins.collect{|join| join.column_names_with_alias.collect{|column_name, aliased_name|
|
||||
"#{join.aliased_table_name}.#{connection.quote_column_name column_name} AS #{aliased_name}"}}.flatten.join(", ")
|
||||
|
@ -1078,7 +1041,7 @@ module ActiveRecord
|
|||
end
|
||||
|
||||
class JoinDependency
|
||||
attr_reader :joins, :reflections
|
||||
attr_reader :joins, :reflections, :table_aliases
|
||||
|
||||
def initialize(base, associations)
|
||||
@joins = [JoinBase.new(base)]
|
||||
|
@ -1086,6 +1049,8 @@ module ActiveRecord
|
|||
@reflections = []
|
||||
@base_records_hash = {}
|
||||
@base_records_in_order = []
|
||||
@table_aliases = Hash.new { |aliases, table| aliases[table] = 0 }
|
||||
@table_aliases[base.table_name] = 1
|
||||
build(associations)
|
||||
end
|
||||
|
||||
|
@ -1228,26 +1193,65 @@ module ActiveRecord
|
|||
@parent = parent
|
||||
@reflection = reflection
|
||||
@aliased_prefix = "t#{ join_dependency.joins.size }"
|
||||
@aliased_table_name = join_dependency.aliased_table_names_for(table_name).empty? ? table_name : @aliased_prefix
|
||||
@aliased_table_name = table_name # start with the table name
|
||||
unless join_dependency.table_aliases[aliased_table_name].zero?
|
||||
# if the table name has been used, then use an alias
|
||||
# if the alias has been used, add a '_n' suffix to the end.
|
||||
@aliased_table_name = "#{parent.table_name}_#{reflection.name}_#{join_dependency.table_aliases[aliased_table_name]}".gsub(/_1$/, '')
|
||||
end
|
||||
join_dependency.table_aliases[aliased_table_name] += 1
|
||||
end
|
||||
|
||||
def association_join
|
||||
case reflection.macro
|
||||
join = case reflection.macro
|
||||
when :has_and_belongs_to_many
|
||||
join_table_name =
|
||||
" LEFT OUTER JOIN %s ON %s.%s = %s.%s " % [
|
||||
options[:join_table], options[:join_table],
|
||||
options[:foreign_key] || reflection.active_record.to_s.classify.foreign_key,
|
||||
reflection.active_record.table_name, reflection.active_record.primary_key] +
|
||||
" LEFT OUTER JOIN %s ON %s.%s = %s.%s " % [
|
||||
aliased_table_name, aliased_table_name, klass.primary_key,
|
||||
" LEFT OUTER JOIN %s AS %s ON %s.%s = %s.%s " % [
|
||||
table_name, aliased_table_name, aliased_table_name, klass.primary_key,
|
||||
options[:join_table], options[:association_foreign_key] || klass.table_name.classify.foreign_key
|
||||
]
|
||||
when :has_many, :has_one
|
||||
case
|
||||
when reflection.macro == :has_many && reflection.options[:through]
|
||||
through_reflection = parent.active_record.reflect_on_association(reflection.options[:through])
|
||||
aliased_through_table = "#{parent.table_name}_#{through_reflection.klass.table_name}"
|
||||
if through_reflection.options[:as] # has_many :through against a polymorphic join
|
||||
polymorphic_foreign_key = through_reflection.options[:as].to_s + '_id'
|
||||
polymorphic_foreign_type = through_reflection.options[:as].to_s + '_type'
|
||||
|
||||
" LEFT OUTER JOIN %s AS %s ON (%s.%s = %s.%s AND %s.%s = %s) " % [through_reflection.klass.table_name, aliased_through_table,
|
||||
aliased_through_table, polymorphic_foreign_key,
|
||||
parent.aliased_table_name, parent.primary_key,
|
||||
aliased_through_table, polymorphic_foreign_type, reflection.klass.quote(parent.active_record.base_class.name)] +
|
||||
" LEFT OUTER JOIN %s AS %s ON %s.%s = %s.%s " % [table_name, aliased_table_name,
|
||||
aliased_table_name, primary_key, aliased_through_table, options[:foreign_key] || reflection.klass.to_s.classify.foreign_key
|
||||
]
|
||||
else # has_many :through against a normal join
|
||||
" LEFT OUTER JOIN %s AS %s ON %s.%s = %s.%s " % [through_reflection.klass.table_name, aliased_through_table,
|
||||
aliased_through_table, through_reflection.options[:foreign_key] || through_reflection.active_record.to_s.classify.foreign_key,
|
||||
parent.aliased_table_name, parent.primary_key] +
|
||||
" LEFT OUTER JOIN %s AS %s ON %s.%s = %s.%s " % [table_name, aliased_table_name,
|
||||
aliased_table_name, primary_key, aliased_through_table, options[:foreign_key] || reflection.klass.to_s.classify.foreign_key
|
||||
]
|
||||
end
|
||||
|
||||
when reflection.macro == :has_many && reflection.options[:as]
|
||||
" LEFT OUTER JOIN %s AS %s ON %s.%s = %s.%s AND %s.%s = %s" % [table_name, aliased_table_name,
|
||||
aliased_table_name, "#{reflection.options[:as]}_id",
|
||||
parent.aliased_table_name, parent.primary_key,
|
||||
aliased_table_name, "#{reflection.options[:as]}_type",
|
||||
reflection.klass.quote(parent.active_record.base_class.name)
|
||||
]
|
||||
else
|
||||
" LEFT OUTER JOIN %s AS %s ON %s.%s = %s.%s " % [table_name, aliased_table_name,
|
||||
aliased_table_name, options[:foreign_key] || reflection.active_record.to_s.classify.foreign_key,
|
||||
parent.aliased_table_name, parent.primary_key
|
||||
]
|
||||
end
|
||||
when :belongs_to
|
||||
" LEFT OUTER JOIN %s AS %s ON %s.%s = %s.%s " % [table_name, aliased_table_name,
|
||||
aliased_table_name, reflection.klass.primary_key,
|
||||
|
@ -1256,6 +1260,11 @@ module ActiveRecord
|
|||
else
|
||||
""
|
||||
end
|
||||
join << %(AND %s."%s" = %s ) % [
|
||||
aliased_table_name,
|
||||
reflection.active_record.inheritance_column,
|
||||
reflection.klass.quote(klass.name)] unless join.blank? || reflection.klass.descends_from_active_record?
|
||||
join
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -29,6 +29,19 @@ class Test::Unit::TestCase #:nodoc:
|
|||
assert_equal expected.to_s, actual.to_s, message
|
||||
end
|
||||
end
|
||||
|
||||
def assert_no_queries
|
||||
ActiveRecord::Base.connection.class.class_eval do
|
||||
self.query_count = 0
|
||||
alias_method :execute, :execute_with_query_counting
|
||||
end
|
||||
yield
|
||||
ensure
|
||||
ActiveRecord::Base.connection.class.class_eval do
|
||||
alias_method :execute, :execute_without_query_counting
|
||||
end
|
||||
assert_equal 0, ActiveRecord::Base.connection.query_count, "1 or more queries were executed"
|
||||
end
|
||||
end
|
||||
|
||||
def current_adapter?(type)
|
||||
|
@ -36,5 +49,14 @@ def current_adapter?(type)
|
|||
ActiveRecord::Base.connection.instance_of?(ActiveRecord::ConnectionAdapters.const_get(type))
|
||||
end
|
||||
|
||||
ActiveRecord::Base.connection.class.class_eval do
|
||||
cattr_accessor :query_count
|
||||
alias_method :execute_without_query_counting, :execute
|
||||
def execute_with_query_counting(sql, name = nil)
|
||||
self.query_count += 1
|
||||
execute_without_query_counting(sql, name)
|
||||
end
|
||||
end
|
||||
|
||||
#ActiveRecord::Base.logger = Logger.new(STDOUT)
|
||||
#ActiveRecord::Base.colorize_logging = false
|
||||
|
|
|
@ -56,23 +56,33 @@ class CascadedEagerLoadingTest < Test::Unit::TestCase
|
|||
def test_eager_association_loading_with_acts_as_tree
|
||||
roots = TreeMixin.find(:all, :include=>"children", :conditions=>"mixins.parent_id IS NULL", :order=>"mixins.id")
|
||||
assert_equal [mixins(:tree_1), mixins(:tree2_1), mixins(:tree3_1)], roots
|
||||
assert_no_queries do
|
||||
assert_equal 2, roots[0].children.size
|
||||
assert_equal 0, roots[1].children.size
|
||||
assert_equal 0, roots[2].children.size
|
||||
end
|
||||
end
|
||||
|
||||
def test_eager_association_loading_with_cascaded_three_levels_by_ping_pong
|
||||
firms = Firm.find(:all, :include=>{:account=>{:firm=>:account}}, :order=>"companies.id")
|
||||
assert_equal 2, firms.size
|
||||
assert_equal firms.first.account, firms.first.account.firm.account
|
||||
assert_equal companies(:first_firm).account, firms.first.account.firm.account
|
||||
assert_equal companies(:first_firm).account.firm.account, firms.first.account.firm.account
|
||||
assert_equal companies(:first_firm).account, assert_no_queries { firms.first.account.firm.account }
|
||||
assert_equal companies(:first_firm).account.firm.account, assert_no_queries { firms.first.account.firm.account }
|
||||
end
|
||||
|
||||
def test_eager_association_loading_with_sti
|
||||
def test_eager_association_loading_with_has_many_sti
|
||||
topics = Topic.find(:all, :include => :replies, :order => 'topics.id')
|
||||
assert_equal [topics(:first), topics(:second)], topics
|
||||
assert_no_queries do
|
||||
assert_equal 1, topics[0].replies.size
|
||||
assert_equal 0, topics[1].replies.size
|
||||
end
|
||||
end
|
||||
|
||||
def test_eager_association_loading_with_belongs_to_sti
|
||||
replies = Reply.find(:all, :include => :topic, :order => 'topics.id')
|
||||
assert_equal [topics(:second)], replies
|
||||
assert_equal topics(:first), assert_no_queries { replies.first.topic }
|
||||
end
|
||||
end
|
||||
|
|
|
@ -94,13 +94,13 @@ class EagerAssociationTest < Test::Unit::TestCase
|
|||
def test_eager_association_loading_with_belongs_to_and_limit_and_multiple_associations
|
||||
posts = Post.find(:all, :include => [:author, :very_special_comment], :limit => 1, :order => 'posts.id')
|
||||
assert_equal 1, posts.length
|
||||
assert_equal [3], posts.collect { |p| p.id }
|
||||
assert_equal [1], posts.collect { |p| p.id }
|
||||
end
|
||||
|
||||
def test_eager_association_loading_with_belongs_to_and_limit_and_offset_and_multiple_associations
|
||||
posts = Post.find(:all, :include => [:author, :very_special_comment], :limit => 1, :offset => 1, :order => 'posts.id')
|
||||
assert_equal 1, posts.length
|
||||
assert_equal [4], posts.collect { |p| p.id }
|
||||
assert_equal [2], posts.collect { |p| p.id }
|
||||
end
|
||||
|
||||
def test_eager_with_has_many_through
|
||||
|
@ -108,8 +108,8 @@ class EagerAssociationTest < Test::Unit::TestCase
|
|||
posts_with_author = people(:michael).posts.find(:all, :include => :author )
|
||||
posts_with_comments_and_author = people(:michael).posts.find(:all, :include => [ :comments, :author ])
|
||||
assert_equal 2, posts_with_comments.inject(0) { |sum, post| sum += post.comments.size }
|
||||
assert_equal authors(:david), posts_with_author.first.author
|
||||
assert_equal authors(:david), posts_with_comments_and_author.first.author
|
||||
assert_equal authors(:david), assert_no_queries { posts_with_author.first.author }
|
||||
assert_equal authors(:david), assert_no_queries { posts_with_comments_and_author.first.author }
|
||||
end
|
||||
|
||||
def test_eager_with_has_many_and_limit
|
||||
|
|
|
@ -82,6 +82,33 @@ class AssociationsJoinModelTest < Test::Unit::TestCase
|
|||
assert_equal "2", categories(:sti_test).authors.first.post_id.to_s
|
||||
end
|
||||
|
||||
def test_include_has_many_through
|
||||
posts = Post.find(:all, :order => 'posts.id')
|
||||
posts_with_authors = Post.find(:all, :include => :authors, :order => 'posts.id')
|
||||
assert_equal posts.length, posts_with_authors.length
|
||||
posts.length.times do |i|
|
||||
assert_equal posts[i].authors.length, assert_no_queries { posts_with_authors[i].authors.length }
|
||||
end
|
||||
end
|
||||
|
||||
def test_include_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_include_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_has_many_find_all
|
||||
assert_equal [categories(:general)], authors(:david).categories.find(:all)
|
||||
end
|
||||
|
|
|
@ -567,7 +567,7 @@ class BasicsTest < Test::Unit::TestCase
|
|||
end
|
||||
|
||||
def test_equality
|
||||
assert_equal Topic.find(1), Topic.find(2).parent
|
||||
assert_equal Topic.find(1), Topic.find(2).topic
|
||||
end
|
||||
|
||||
def test_equality_of_new_records
|
||||
|
@ -575,7 +575,7 @@ class BasicsTest < Test::Unit::TestCase
|
|||
end
|
||||
|
||||
def test_hashing
|
||||
assert_equal [ Topic.find(1) ], [ Topic.find(2).parent ] & [ Topic.find(1) ]
|
||||
assert_equal [ Topic.find(1) ], [ Topic.find(2).topic ] & [ Topic.find(1) ]
|
||||
end
|
||||
|
||||
def test_destroy_new_record
|
||||
|
|
|
@ -1,6 +1,7 @@
|
|||
require 'abstract_unit'
|
||||
require 'fixtures/company'
|
||||
require 'fixtures/topic'
|
||||
require 'fixtures/reply'
|
||||
require 'fixtures/entrant'
|
||||
require 'fixtures/developer'
|
||||
require 'fixtures/post'
|
||||
|
@ -92,7 +93,7 @@ class FinderTest < Test::Unit::TestCase
|
|||
Topic.find(1).parent
|
||||
}
|
||||
|
||||
Topic.find(2).parent
|
||||
Topic.find(2).topic
|
||||
end
|
||||
|
||||
def test_find_only_some_columns
|
||||
|
|
2
activerecord/test/fixtures/topic.rb
vendored
2
activerecord/test/fixtures/topic.rb
vendored
|
@ -6,7 +6,7 @@ class Topic < ActiveRecord::Base
|
|||
before_destroy :destroy_children
|
||||
|
||||
def parent
|
||||
self.class.find(parent_id)
|
||||
Topic.find(parent_id)
|
||||
end
|
||||
|
||||
protected
|
||||
|
|
1
activerecord/test/fixtures/topics.yml
vendored
1
activerecord/test/fixtures/topics.yml
vendored
|
@ -19,3 +19,4 @@ second:
|
|||
approved: true
|
||||
replies_count: 2
|
||||
parent_id: 1
|
||||
type: Reply
|
||||
|
|
Loading…
Reference in a new issue