Permalink
Browse files

Address shortcomings of changeset [8054] [protocool]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@8109 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
1 parent 31e2a2d commit 37adea6ff1fba85c29406bbe2af4831f4c6beeec @dhh dhh committed Nov 7, 2007
@@ -486,63 +486,7 @@ def clear_association_cache #:nodoc:
#
# 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 @@ def association_constructor_method(constructor, reflection, association_proxy_cl
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 @@ def create_extension_modules(association_id, block_extension, extensions)
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 @@ def instantiate(rows)
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 @@ def build(associations, parent = nil)
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 @@ def build(associations, parent = nil)
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 @@ def table_name_and_alias
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
@@ -380,11 +380,10 @@ class << self # Class methods
# * <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 @@ class << self # Class methods
# 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 @@ def find_initial(options)
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 @@ def add_lock!(sql, options, scope = :auto)
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 @@ def with_scope(method_scoping = {}, action = :merge, &block)
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 @@ def with_scope(method_scoping = {}, action = :merge, &block)
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]
@@ -15,11 +15,8 @@ module ClassMethods
# 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 @@ def sum(column_name, options = {})
# 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 @@ def construct_calculation_sql(operation, column_name, options) #:nodoc:
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 @@ def construct_calculation_sql(operation, column_name, options) #:nodoc:
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)
Oops, something went wrong.

0 comments on commit 37adea6

Please sign in to comment.