diff options
Diffstat (limited to 'activerecord')
| -rwxr-xr-x | activerecord/lib/active_record/associations.rb | 97 | ||||
| -rwxr-xr-x | activerecord/lib/active_record/base.rb | 43 | ||||
| -rw-r--r-- | activerecord/lib/active_record/calculations.rb | 19 | ||||
| -rw-r--r-- | activerecord/test/associations/ar_joins_test.rb | 158 | ||||
| -rw-r--r-- | activerecord/test/associations/inner_join_association_test.rb | 88 |
5 files changed, 128 insertions, 277 deletions
diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index fcc041beca..1013c75643 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -486,63 +486,7 @@ module ActiveRecord # # When eager loaded, conditions are interpolated in the context of the model class, not the model instance. Conditions are lazily interpolated # before the actual model exists. - # - # == Adding Joins For Associations to Queries Using the :joins option - # - # ActiveRecord::Base#find provides a :joins option, which takes either a string or values accepted by the :include option. - # if the value is a string, the it should contain a SQL fragment containing a join clause. - # - # Non-string values of :joins will add an automatic join clause to the query in the same way that the :include option does but with two critical - # differences: - # - # 1. A normal (inner) join will be performed instead of the outer join generated by :include. - # this means that only objects which have objects attached to the association will be included in the result. - # For example, suppose we have the following tables (in yaml format): - # - # Authors - # fred: - # id: 1 - # name: Fred - # steve: - # id: 2 - # name: Steve - # - # Contributions - # only: - # id: 1 - # author_id: 1 - # description: Atta Boy Letter for Steve - # date: 2007-10-27 14:09:54 - # - # and corresponding AR Classes - # - # class Author: < ActiveRecord::Base - # has_many :contributions - # end - # - # class Contribution < ActiveRecord::Base - # belongs_to :author - # end - # - # The query Author.find(:all) will return both authors, but Author.find(:all, :joins => :contributions) will - # only return authors who have at least one contribution, in this case only the first. - # This is only a degenerate case of the more typical use of :joins with a non-string value. - # For example to find authors who have at least one contribution before a certain date we can use: - # - # Author.find(:all, :joins => :contributions, :conditions => ["contributions.date <= ?", 1.week.ago.to_s(:db)]) - # - # 2. Only instances of the class to which the find is sent will be instantiated. ActiveRecord objects will not - # be instantiated for rows reached through the associations. - # - # The difference between using :joins vs :include to name associated records is that :joins allows associated tables to - # participate in selection criteria in the query without incurring the overhead of instantiating associated objects. - # This can be important when the number of associated objects in the database is large, and they will not be used, or - # only those associated with a paricular object or objects will be used after the query, making two queries more - # efficient than one. - # - # Note that while using a string value for :joins marks the result objects as read-only, the objects resulting - # from a call to find with a non-string :joins option value will be writable. - # + # # == Table Aliasing # # ActiveRecord uses table aliasing in the case that a table is referenced multiple times in a join. If a table is referenced only once, @@ -1177,13 +1121,7 @@ module ActiveRecord def find_with_associations(options = {}) catch :invalid_query do - if ar_joins = scope(:find, :ar_joins) - options = options.dup - options[:ar_joins] = ar_joins - end - includes = merge_includes(scope(:find, :include), options[:include]) - includes = merge_includes(includes, options[:ar_joins]) - join_dependency = JoinDependency.new(self, includes, options[:joins], options[:ar_joins]) + join_dependency = JoinDependency.new(self, merge_includes(scope(:find, :include), options[:include]), options[:joins]) rows = select_all_rows(options, join_dependency) return join_dependency.instantiate(rows) end @@ -1437,9 +1375,8 @@ module ActiveRecord class JoinDependency # :nodoc: attr_reader :joins, :reflections, :table_aliases - def initialize(base, associations, joins, ar_joins = nil) + def initialize(base, associations, joins) @joins = [JoinBase.new(base, joins)] - @ar_joins = ar_joins @associations = associations @reflections = [] @base_records_hash = {} @@ -1463,9 +1400,9 @@ module ActiveRecord unless @base_records_hash[primary_id] @base_records_in_order << (@base_records_hash[primary_id] = join_base.instantiate(row)) end - construct(@base_records_hash[primary_id], @associations, join_associations.dup, row) unless @ar_joins + construct(@base_records_hash[primary_id], @associations, join_associations.dup, row) end - remove_duplicate_results!(join_base.active_record, @base_records_in_order, @associations) unless @ar_joins + remove_duplicate_results!(join_base.active_record, @base_records_in_order, @associations) return @base_records_in_order end @@ -1507,7 +1444,7 @@ module ActiveRecord reflection = parent.reflections[associations.to_s.intern] or raise ConfigurationError, "Association named '#{ associations }' was not found; perhaps you misspelled it?" @reflections << reflection - @joins << (@ar_joins ? ARJoinAssociation : JoinAssociation).new(reflection, self, parent) + @joins << build_join_association(reflection, parent) when Array associations.each do |association| build(association, parent) @@ -1522,6 +1459,11 @@ module ActiveRecord end end + # overridden in InnerJoinDependency subclass + def build_join_association(reflection, parent) + JoinAssociation.new(reflection, self, parent) + end + def construct(parent, associations, joins, row) case associations when Symbol, String @@ -1786,21 +1728,30 @@ module ActiveRecord def interpolate_sql(sql) instance_eval("%@#{sql.gsub('@', '\@')}@") - end + end + + private - private def join_type "LEFT OUTER JOIN" end - end - class ARJoinAssociation < JoinAssociation + end + + class InnerJoinDependency < JoinDependency # :nodoc: + protected + def build_join_association(reflection, parent) + InnerJoinAssociation.new(reflection, self, parent) + end + + class InnerJoinAssociation < JoinAssociation private def join_type "INNER JOIN" end end end + end end end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index d7f0c4812b..a7d2b01f73 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -380,11 +380,10 @@ module ActiveRecord #:nodoc: # * <tt>:group</tt>: An attribute name by which the result should be grouped. Uses the GROUP BY SQL-clause. # * <tt>:limit</tt>: An integer determining the limit on the number of rows that should be returned. # * <tt>:offset</tt>: An integer determining the offset from where the rows should be fetched. So at 5, it would skip rows 0 through 4. - # * <tt>:joins</tt>: Either an SQL fragment for additional joins like "LEFT JOIN comments ON comments.post_id = id". (Rarely needed). - # or names associations in the same form used for the :include option. - # If the value is a string, then the records will be returned read-only since they will have attributes that do not correspond to the table's columns. + # * <tt>:joins</tt>: An SQL fragment for additional joins like "LEFT JOIN comments ON comments.post_id = id" (Rarely needed). + # Accepts named associations in the form of :include, which will perform an INNER JOIN on the associated table(s). + # The records will be returned read-only since they will have attributes that do not correspond to the table's columns. # Pass :readonly => false to override. - # See adding joins for associations under Association. # * <tt>:include</tt>: Names associations that should be loaded alongside using LEFT OUTER JOINs. The symbols named refer # to already defined associations. See eager loading under Associations. # * <tt>:select</tt>: By default, this is * as in SELECT * FROM, but can be changed if you for example want to do a join, but not @@ -430,17 +429,8 @@ module ActiveRecord #:nodoc: # end def find(*args) options = args.extract_options! - # Note: we extract any :joins option with a non-string value from the options, and turn it into - # an internal option :ar_joins. This allows code called from her to find the ar_joins, and - # it bypasses marking the result as read_only. - # A normal string join marks the result as read-only because it contains attributes from joined tables - # which are not in the base table and therefore prevent the result from being saved. - # In the case of an ar_join, the JoinDependency created to instantiate the results eliminates these - # bogus attributes. See JoinDependency#instantiate, and JoinBase#instantiate in associations.rb. - options, ar_joins = *extract_ar_join_from_options(options) validate_find_options(options) set_readonly_option!(options) - options[:ar_joins] = ar_joins if ar_joins case args.first when :first then find_initial(options) @@ -1038,17 +1028,8 @@ module ActiveRecord #:nodoc: find_every(options).first end - # If options contains :joins, with a non-string value - # remove it from options - # return the updated or unchanged options, and the ar_join value or nil - def extract_ar_join_from_options(options) - new_options = options.dup - join_option = new_options.delete(:joins) - (join_option && !join_option.kind_of?(String)) ? [new_options, join_option] : [options, nil] - end - def find_every(options) - records = scoped?(:find, :include) || options[:include] || scoped?(:find, :ar_joins) || (options[:ar_joins]) ? + records = scoped?(:find, :include) || options[:include] ? find_with_associations(options) : find_by_sql(construct_finder_sql(options)) @@ -1246,7 +1227,13 @@ module ActiveRecord #:nodoc: def add_joins!(sql, options, scope = :auto) scope = scope(:find) if :auto == scope join = (scope && scope[:joins]) || options[:joins] - sql << " #{join} " if join + case join + when Symbol, Hash, Array + join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, join, nil) + sql << " #{join_dependency.join_associations.collect{|join| join.association_join }.join} " + else + sql << " #{join} " + end end # Adds a sanitized version of +conditions+ to the +sql+ string. Note that the passed-in +sql+ string is changed. @@ -1472,13 +1459,7 @@ module ActiveRecord #:nodoc: if f = method_scoping[:find] f.assert_valid_keys(VALID_FIND_OPTIONS) - # see note about :joins and :ar_joins in ActiveRecord::Base#find - f, ar_joins = *extract_ar_join_from_options(f) set_readonly_option! f - if ar_joins - f[:ar_joins] = ar_joins - method_scoping[:find] = f - end end # Merge scopings @@ -1491,7 +1472,7 @@ module ActiveRecord #:nodoc: merge = hash[method][key] && params[key] # merge if both scopes have the same key if key == :conditions && merge hash[method][key] = [params[key], hash[method][key]].collect{ |sql| "( %s )" % sanitize_sql(sql) }.join(" AND ") - elsif ([:include, :ar_joins].include?(key)) && merge + elsif key == :include && merge hash[method][key] = merge_includes(hash[method][key], params[key]).uniq else hash[method][key] = hash[method][key] || params[key] diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb index 9620ce1b20..7f8af9421e 100644 --- a/activerecord/lib/active_record/calculations.rb +++ b/activerecord/lib/active_record/calculations.rb @@ -15,11 +15,8 @@ module ActiveRecord # The third approach, count using options, accepts an option hash as the only parameter. The options are: # # * <tt>:conditions</tt>: An SQL fragment like "administrator = 1" or [ "user_name = ?", username ]. See conditions in the intro. - # * <tt>:joins</tt>: Either an SQL fragment for additional joins like "LEFT JOIN comments ON comments.post_id = id". (Rarely needed). - # or names associations in the same form used for the :include option. - # If the value is a string, then the records will be returned read-only since they will have attributes that do not correspond to the table's columns. - # Pass :readonly => false to override. - # See adding joins for associations under Association. + # * <tt>:joins</tt>: An SQL fragment for additional joins like "LEFT JOIN comments ON comments.post_id = id". (Rarely needed). + # The records will be returned read-only since they will have attributes that do not correspond to the table's columns. # * <tt>:include</tt>: Named associations that should be loaded alongside using LEFT OUTER JOINs. The symbols named refer # to already defined associations. When using named associations count returns the number DISTINCT items for the model you're counting. # See eager loading under Associations. @@ -112,9 +109,7 @@ module ActiveRecord # Person.minimum(:age, :conditions => ['last_name != ?', 'Drake']) # Selects the minimum age for everyone with a last name other than 'Drake' # Person.minimum(:age, :having => 'min(age) > 17', :group => :last_name) # Selects the minimum age for any family without any minors def calculate(operation, column_name, options = {}) - options, ar_joins = *extract_ar_join_from_options(options) validate_calculation_options(operation, options) - options[:ar_joins] = ar_joins if ar_joins column_name = options[:select] if options[:select] column_name = '*' if column_name == :all column = column_for column_name @@ -154,14 +149,8 @@ module ActiveRecord operation = operation.to_s.downcase options = options.symbolize_keys - scope = scope(:find) - if scope && scope[:ar_joins] - scope = scope.dup - options = options.dup - options[:ar_joins] = scope.delete(:ar_joins) - end + scope = scope(:find) merged_includes = merge_includes(scope ? scope[:include] : [], options[:include]) - merged_includes = merge_includes(merged_includes, options[:ar_joins]) aggregate_alias = column_alias_for(operation, column_name) if operation == 'count' @@ -184,7 +173,7 @@ module ActiveRecord sql << " FROM (SELECT DISTINCT #{column_name}" if use_workaround sql << " FROM #{connection.quote_table_name(table_name)} " if merged_includes.any? - join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, merged_includes, options[:joins], options[:ar_joins]) + join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, merged_includes, options[:joins]) sql << join_dependency.join_associations.collect{|join| join.association_join }.join end add_joins!(sql, options, scope) diff --git a/activerecord/test/associations/ar_joins_test.rb b/activerecord/test/associations/ar_joins_test.rb index e6a9180fae..e69de29bb2 100644 --- a/activerecord/test/associations/ar_joins_test.rb +++ b/activerecord/test/associations/ar_joins_test.rb @@ -1,158 +0,0 @@ -require 'abstract_unit' -require 'fixtures/post' -require 'fixtures/comment' -require 'fixtures/author' -require 'fixtures/category' -require 'fixtures/categorization' -require 'fixtures/company' -require 'fixtures/topic' -require 'fixtures/reply' -require 'fixtures/developer' -require 'fixtures/project' - -class ArJoinsTest < Test::Unit::TestCase - fixtures :authors, :posts, :comments, :categories, :categories_posts, :people, - :developers, :projects, :developers_projects, - :categorizations, :companies, :accounts, :topics - - def test_ar_joins - authors = Author.find(:all, :joins => :posts, :conditions => ['posts.type = ?', "Post"]) - assert_not_equal(0 , authors.length) - authors.each do |author| - assert !(author.send(:instance_variables).include? "@posts") - assert(!author.readonly?, "non-string join value produced read-only result.") - end - end - - def test_ar_joins_with_cascaded_two_levels - authors = Author.find(:all, :joins=>{:posts=>:comments}) - assert_equal(2, authors.length) - authors.each do |author| - assert !(author.send(:instance_variables).include? "@posts") - assert(!author.readonly?, "non-string join value produced read-only result.") - end - authors = Author.find(:all, :joins=>{:posts=>:comments}, :conditions => ["comments.body = ?", "go crazy" ]) - assert_equal(1, authors.length) - authors.each do |author| - assert !(author.send(:instance_variables).include? "@posts") - assert(!author.readonly?, "non-string join value produced read-only result.") - end - end - - - def test_ar_joins_with_complex_conditions - authors = Author.find(:all, :joins=>{:posts=>[:comments, :categories]}, - :conditions => ["categories.name = ? AND posts.title = ?", "General", "So I was thinking"] - ) - assert_equal(1, authors.length) - authors.each do |author| - assert !(author.send(:instance_variables).include? "@posts") - assert(!author.readonly?, "non-string join value produced read-only result.") - end - assert_equal("David", authors.first.name) - end - - def test_ar_join_with_has_many_and_limit_and_scoped_and_explicit_conditions - Post.with_scope(:find => { :conditions => "1=1" }) do - posts = authors(:david).posts.find(:all, - :joins => :comments, - :conditions => "comments.body like 'Normal%' OR comments.#{QUOTED_TYPE}= 'SpecialComment'", - :limit => 2 - ) - assert_equal 2, posts.size - - count = Post.count( - :joins => [ :comments, :author ], - :conditions => "authors.name = 'David' AND (comments.body like 'Normal%' OR comments.#{QUOTED_TYPE}= 'SpecialComment')", - :limit => 2 - ) - assert_equal count, posts.size - end - end - - def test_ar_join_with_scoped_order_using_association_limiting_without_explicit_scope - posts_with_explicit_order = Post.find(:all, :conditions => 'comments.id is not null', :joins => :comments, :order => 'posts.id DESC', :limit => 2) - posts_with_scoped_order = Post.with_scope(:find => {:order => 'posts.id DESC'}) do - Post.find(:all, :conditions => 'comments.id is not null', :joins => :comments, :limit => 2) - end - assert_equal posts_with_explicit_order, posts_with_scoped_order - end - - def test_scoped_find_include - # with the include, will retrieve only developers for the given project - scoped_developers = Developer.with_scope(:find => { :joins => :projects }) do - Developer.find(:all, :conditions => 'projects.id = 2') - end - assert scoped_developers.include?(developers(:david)) - assert !scoped_developers.include?(developers(:jamis)) - assert_equal 1, scoped_developers.size - end - - - def test_nested_scoped_find_ar_join - Developer.with_scope(:find => { :joins => :projects }) do - Developer.with_scope(:find => { :conditions => "projects.id = 2" }) do - assert_equal('David', Developer.find(:first).name) - end - end - end - - def test_nested_scoped_find_merged_ar_join - # :include's remain unique and don't "double up" when merging - Developer.with_scope(:find => { :joins => :projects, :conditions => "projects.id = 2" }) do - Developer.with_scope(:find => { :joins => :projects }) do - assert_equal 1, Developer.instance_eval('current_scoped_methods')[:find][:ar_joins].length - assert_equal('David', Developer.find(:first).name) - end - end - # the nested scope doesn't remove the first :include - Developer.with_scope(:find => { :joins => :projects, :conditions => "projects.id = 2" }) do - Developer.with_scope(:find => { :joins => [] }) do - assert_equal 1, Developer.instance_eval('current_scoped_methods')[:find][:ar_joins].length - assert_equal('David', Developer.find(:first).name) - end - end - # mixing array and symbol include's will merge correctly - Developer.with_scope(:find => { :joins => [:projects], :conditions => "projects.id = 2" }) do - Developer.with_scope(:find => { :joins => :projects }) do - assert_equal 1, Developer.instance_eval('current_scoped_methods')[:find][:ar_joins].length - assert_equal('David', Developer.find(:first).name) - end - end - end - - def test_nested_scoped_find_replace_include - Developer.with_scope(:find => { :joins => :projects }) do - Developer.with_exclusive_scope(:find => { :joins => [] }) do - assert_equal 0, Developer.instance_eval('current_scoped_methods')[:find][:ar_joins].length - end - end - end - -# -# Calculations -# - def test_count_with_ar_joins - assert_equal(2, Author.count(:joins => :posts, :conditions => ['posts.type = ?', "Post"])) - assert_equal(1, Author.count(:joins => :posts, :conditions => ['posts.type = ?', "SpecialPost"])) - end - - def test_should_get_maximum_of_field_with_joins - assert_equal 50, Account.maximum(:credit_limit, :joins=> :firm, :conditions => "companies.name != 'Summit'") - end - - def test_should_get_maximum_of_field_with_scoped_include - Account.with_scope :find => { :joins => :firm, :conditions => "companies.name != 'Summit'" } do - assert_equal 50, Account.maximum(:credit_limit) - end - end - - def test_should_not_modify_options_when_using_ar_joins_on_count - options = {:conditions => 'companies.id > 1', :joins => :firm} - options_copy = options.dup - - Account.count(:all, options) - assert_equal options_copy, options - end - -end diff --git a/activerecord/test/associations/inner_join_association_test.rb b/activerecord/test/associations/inner_join_association_test.rb new file mode 100644 index 0000000000..983f269e87 --- /dev/null +++ b/activerecord/test/associations/inner_join_association_test.rb @@ -0,0 +1,88 @@ +require 'abstract_unit' +require 'fixtures/post' +require 'fixtures/comment' +require 'fixtures/author' +require 'fixtures/category' +require 'fixtures/categorization' + +class InnerJoinAssociationTest < Test::Unit::TestCase + fixtures :authors, :posts, :comments, :categories, :categories_posts, :categorizations + + def test_construct_finder_sql_creates_inner_joins + sql = Author.send(:construct_finder_sql, :joins => :posts) + assert_match /INNER JOIN posts ON posts.author_id = authors.id/, sql + end + + def test_construct_finder_sql_cascades_inner_joins + sql = Author.send(:construct_finder_sql, :joins => {:posts => :comments}) + assert_match /INNER JOIN posts ON posts.author_id = authors.id/, sql + assert_match /INNER JOIN comments ON comments.post_id = posts.id/, sql + end + + def test_construct_finder_sql_inner_joins_through_associations + sql = Author.send(:construct_finder_sql, :joins => :categorized_posts) + assert_match /INNER JOIN categorizations.*INNER JOIN posts/, sql + end + + def test_construct_finder_sql_applies_association_conditions + sql = Author.send(:construct_finder_sql, :joins => :categories_like_general, :conditions => "TERMINATING_MARKER") + assert_match /INNER JOIN categories ON.*AND.*'General'.*TERMINATING_MARKER/, sql + end + + def test_construct_finder_sql_unpacks_nested_joins + sql = Author.send(:construct_finder_sql, :joins => {:posts => [[:comments]]}) + assert_no_match /inner join.*inner join.*inner join/i, sql, "only two join clauses should be present" + assert_match /INNER JOIN posts ON posts.author_id = authors.id/, sql + assert_match /INNER JOIN comments ON comments.post_id = posts.id/, sql + end + + def test_construct_finder_sql_ignores_empty_joins_hash + sql = Author.send(:construct_finder_sql, :joins => {}) + assert_no_match /JOIN/i, sql + end + + def test_construct_finder_sql_ignores_empty_joins_array + sql = Author.send(:construct_finder_sql, :joins => []) + assert_no_match /JOIN/i, sql + end + + def test_find_with_implicit_inner_joins_honors_readonly_without_select + authors = Author.find(:all, :joins => :posts) + assert !authors.empty?, "expected authors to be non-empty" + assert authors.all? {|a| a.readonly? }, "expected all authors to be readonly" + end + + def test_find_with_implicit_inner_joins_honors_readonly_with_select + authors = Author.find(:all, :select => 'authors.*', :joins => :posts) + assert !authors.empty?, "expected authors to be non-empty" + assert authors.all? {|a| !a.readonly? }, "expected no authors to be readonly" + end + + def test_find_with_implicit_inner_joins_honors_readonly_false + authors = Author.find(:all, :joins => :posts, :readonly => false) + assert !authors.empty?, "expected authors to be non-empty" + assert authors.all? {|a| !a.readonly? }, "expected no authors to be readonly" + end + + def test_find_with_implicit_inner_joins_does_not_set_associations + authors = Author.find(:all, :select => 'authors.*', :joins => :posts) + assert !authors.empty?, "expected authors to be non-empty" + assert authors.all? {|a| !a.send(:instance_variables).include?("@posts")}, "expected no authors to have the @posts association loaded" + end + + def test_count_honors_implicit_inner_joins + real_count = Author.find(:all).sum{|a| a.posts.count } + assert_equal real_count, Author.count(:joins => :posts), "plain inner join count should match the number of referenced posts records" + end + + def test_calculate_honors_implicit_inner_joins + real_count = Author.find(:all).sum{|a| a.posts.count } + assert_equal real_count, Author.calculate(:count, 'authors.id', :joins => :posts), "plain inner join count should match the number of referenced posts records" + end + + def test_calculate_honors_implicit_inner_joins_and_distinct_and_conditions + real_count = Author.find(:all).select {|a| a.posts.any? {|p| p.title =~ /^Welcome/} }.length + authors_with_welcoming_post_titles = Author.calculate(:count, 'authors.id', :joins => :posts, :distinct => true, :conditions => "posts.title like 'Welcome%'") + assert_equal real_count, authors_with_welcoming_post_titles, "inner join and conditions should have only returned authors posting titles starting with 'Welcome'" + end +end |