From 9ac01fad193b27e517ea772e0d1e13e06f4ddf34 Mon Sep 17 00:00:00 2001 From: Emilio Tagua Date: Fri, 14 Aug 2009 12:33:05 -0300 Subject: Use ARel's joins when building a query for finding records with included associations. --- activerecord/lib/active_record/associations.rb | 106 ++++++++++++++----------- activerecord/lib/active_record/base.rb | 21 +++-- activerecord/lib/active_record/calculations.rb | 23 ++---- activerecord/lib/active_record/relation.rb | 12 ++- 4 files changed, 88 insertions(+), 74 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index baad8fc5fd..e4cd515f15 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1673,14 +1673,20 @@ module ActiveRecord ) end - def construct_finder_sql_with_included_associations(options, join_dependency) + def construct_finder_arel_with_included_associations(options, join_dependency) scope = scope(:find) relation = arel_table((scope && scope[:from]) || options[:from]) - joins = join_dependency.join_associations.collect{|join| join.association_join }.join - joins << construct_join(options[:joins], scope) - relation.join(joins) + for association in join_dependency.join_associations + if association.relation.is_a?(Array) + relation.join(association.relation.first, association.join_type).on(association.association_join.first) + relation.join(association.relation.last, association.join_type).on(association.association_join.last) + else + relation.join(association.relation, association.join_type).on(association.association_join) + end + end + relation.join(construct_join(options[:joins], scope)) relation.where(construct_conditions(options[:conditions], scope)) relation.where(construct_arel_limited_ids_condition(options, join_dependency)) if !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit]) @@ -1690,7 +1696,11 @@ module ActiveRecord relation.order(construct_order(options[:order], scope)) relation.take(construct_limit(options[:limit], scope)) if using_limitable_reflections?(join_dependency.reflections) - sanitize_sql(relation.to_sql) + relation + end + + def construct_finder_sql_with_included_associations(options, join_dependency) + sanitize_sql(construct_finder_arel_with_included_associations(options, join_dependency).to_sql) end def construct_arel_limited_ids_condition(options, join_dependency) @@ -1716,9 +1726,15 @@ module ActiveRecord relation = arel_table(options[:from]) - joins = join_dependency.join_associations.collect{|join| join.association_join }.join - joins << construct_join(options[:joins], scope) - relation.join(joins) + for association in join_dependency.join_associations + if association.relation.is_a?(Array) + relation.join(association.relation.first, association.join_type).on(association.association_join.first) + relation.join(association.relation.last, association.join_type).on(association.association_join.last) + else + relation.join(association.relation, association.join_type).on(association.association_join) + end + end + relation.join(construct_join(options[:joins], scope)) relation.where(construct_conditions(options[:conditions], scope)) relation.project(connection.distinct("#{connection.quote_table_name table_name}.#{primary_key}", construct_order(options[:order], scope(:find)).join(","))) @@ -1907,12 +1923,6 @@ module ActiveRecord end end - def join_for_table_name(table_name) - join = (@joins.select{|j|j.aliased_table_name == table_name.gsub(/^\"(.*)\"$/){$1} }.first) rescue nil - return join unless join.nil? - @joins.select{|j|j.is_a?(JoinAssociation) && j.aliased_join_table_name == table_name.gsub(/^\"(.*)\"$/){$1} }.first rescue nil - end - def joins_for_table_name(table_name) join = join_for_table_name(table_name) result = nil @@ -2088,19 +2098,18 @@ module ActiveRecord connection = reflection.active_record.connection join = case reflection.macro when :has_and_belongs_to_many - " #{join_type} %s ON %s.%s = %s.%s " % [ - table_alias_for(options[:join_table], aliased_join_table_name), + ["%s.%s = %s.%s " % [ connection.quote_table_name(aliased_join_table_name), options[:foreign_key] || reflection.active_record.to_s.foreign_key, connection.quote_table_name(parent.aliased_table_name), - reflection.active_record.primary_key] + - " #{join_type} %s ON %s.%s = %s.%s " % [ - table_name_and_alias, + reflection.active_record.primary_key], + "%s.%s = %s.%s " % [ connection.quote_table_name(aliased_table_name), klass.primary_key, connection.quote_table_name(aliased_join_table_name), options[:association_foreign_key] || klass.to_s.foreign_key ] + ] when :has_many, :has_one if reflection.options[:through] through_conditions = through_reflection.options[:conditions] ? "AND #{interpolate_sql(sanitize_sql(through_reflection.options[:conditions]))}" : '' @@ -2154,26 +2163,22 @@ module ActiveRecord end end - " #{join_type} %s ON (%s.%s = %s.%s%s%s%s) " % [ - table_alias_for(through_reflection.klass.table_name, aliased_join_table_name), + ["(%s.%s = %s.%s%s%s%s) " % [ connection.quote_table_name(parent.aliased_table_name), connection.quote_column_name(parent.primary_key), connection.quote_table_name(aliased_join_table_name), connection.quote_column_name(jt_foreign_key), - jt_as_extra, jt_source_extra, jt_sti_extra - ] + - " #{join_type} %s ON (%s.%s = %s.%s%s) " % [ - table_name_and_alias, + jt_as_extra, jt_source_extra, jt_sti_extra], + "(%s.%s = %s.%s%s) " % [ connection.quote_table_name(aliased_table_name), connection.quote_column_name(first_key), connection.quote_table_name(aliased_join_table_name), connection.quote_column_name(second_key), - as_extra + as_extra] ] elsif reflection.options[:as] && [:has_many, :has_one].include?(reflection.macro) - " #{join_type} %s ON %s.%s = %s.%s AND %s.%s = %s" % [ - table_name_and_alias, + "%s.%s = %s.%s AND %s.%s = %s" % [ connection.quote_table_name(aliased_table_name), "#{reflection.options[:as]}_id", connection.quote_table_name(parent.aliased_table_name), @@ -2184,8 +2189,7 @@ module ActiveRecord ] else foreign_key = options[:foreign_key] || reflection.active_record.name.foreign_key - " #{join_type} %s ON %s.%s = %s.%s " % [ - table_name_and_alias, + "%s.%s = %s.%s " % [ aliased_table_name, foreign_key, parent.aliased_table_name, @@ -2193,13 +2197,12 @@ module ActiveRecord ] end when :belongs_to - " #{join_type} %s ON %s.%s = %s.%s " % [ - table_name_and_alias, - connection.quote_table_name(aliased_table_name), - reflection.klass.primary_key, - connection.quote_table_name(parent.aliased_table_name), - options[:foreign_key] || reflection.primary_key_name - ] + "%s.%s = %s.%s " % [ + connection.quote_table_name(aliased_table_name), + reflection.klass.primary_key, + connection.quote_table_name(parent.aliased_table_name), + options[:foreign_key] || reflection.primary_key_name + ] else "" end @@ -2213,6 +2216,20 @@ module ActiveRecord join end + def relation + if reflection.macro == :has_and_belongs_to_many + [Arel::Table.new(table_alias_for(options[:join_table], aliased_join_table_name)), Arel::Table.new(table_name_and_alias)] + elsif reflection.options[:through] + [Arel::Table.new(table_alias_for(through_reflection.klass.table_name, aliased_join_table_name)), Arel::Table.new(table_name_and_alias)] + else + Arel::Table.new(table_name_and_alias) + end + end + + def join_type + Arel::OuterJoin + end + protected def aliased_table_name_for(name, suffix = nil) @@ -2238,7 +2255,7 @@ module ActiveRecord end def table_alias_for(table_name, table_alias) - "#{reflection.active_record.connection.quote_table_name(table_name)} #{table_alias if table_name != table_alias}".strip + "#{table_name} #{table_alias if table_name != table_alias}".strip end def table_name_and_alias @@ -2248,11 +2265,6 @@ module ActiveRecord def interpolate_sql(sql) instance_eval("%@#{sql.gsub('@', '\@')}@") end - - private - def join_type - "LEFT OUTER JOIN" - end end end @@ -2263,13 +2275,11 @@ module ActiveRecord end class InnerJoinAssociation < JoinAssociation - private - def join_type - "INNER JOIN" - end + def join_type + Arel::InnerJoin + end end end - end end end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index e1f4461965..a6832b47ef 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1741,8 +1741,7 @@ module ActiveRecord #:nodoc: if array_of_strings?(merged_joins) merged_joins.join(' ') + " " else - join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, merged_joins, nil) - " #{join_dependency.join_associations.collect { |assoc| assoc.association_join }.join} " + build_association_joins(merged_joins) end when String " #{merged_joins} " @@ -1801,10 +1800,7 @@ module ActiveRecord #:nodoc: if joins.any?{|j| j.is_a?(String) || array_of_strings?(j) } joins = joins.collect do |join| join = [join] if join.is_a?(String) - unless array_of_strings?(join) - join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, join, nil) - join = join_dependency.join_associations.collect { |assoc| assoc.association_join } - end + join = build_association_joins(join) unless array_of_strings?(join) join end joins.flatten.map{|j| j.strip}.uniq @@ -1813,6 +1809,19 @@ module ActiveRecord #:nodoc: end end + def build_association_joins(joins) + join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, joins, nil) + relation = arel_table + join_dependency.join_associations.map { |association| + if association.relation.is_a?(Array) + [Arel::InnerJoin.new(relation, association.relation.first, association.association_join.first).joins(relation), + Arel::InnerJoin.new(relation, association.relation.last, association.association_join.last).joins(relation)].join() + else + Arel::InnerJoin.new(relation, association.relation, association.association_join).joins(relation) + end + }.join(" ") + end + # Object#to_a is deprecated, though it does have the desired behavior def safe_to_array(o) case o diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb index 6a5f2222a2..ab501dac33 100644 --- a/activerecord/lib/active_record/calculations.rb +++ b/activerecord/lib/active_record/calculations.rb @@ -128,12 +128,6 @@ module ActiveRecord scope = scope(:find) merged_includes = merge_includes(scope ? scope[:include] : [], options[:include]) - joins = construct_join(options[:joins], scope) - - if merged_includes.any? - join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, merged_includes, joins) - joins << join_dependency.join_associations.collect{|join| join.association_join }.join - end if operation == "count" if merged_includes.any? @@ -148,17 +142,12 @@ module ActiveRecord end catch :invalid_query do - relation = arel_table((scope && scope[:from]) || options[:from]) - - relation.join(joins) - - relation.where(construct_conditions(options[:conditions], scope)) - relation.where(construct_arel_limited_ids_condition(options, join_dependency)) if join_dependency && !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit]) - - relation.order(options[:order]) - relation.take(options[:limit]) - relation.skip(options[:offset]) - + relation = if merged_includes.any? + join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, merged_includes, construct_join(options[:joins], scope)) + construct_finder_arel_with_included_associations(options, join_dependency) + else + construct_finder_arel(options) + end if options[:group] return execute_grouped_calculation(operation, column_name, options, relation) else diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index f89baa1a74..456de73250 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -1,7 +1,7 @@ module ActiveRecord class Relation delegate :delete, :to_sql, :to => :relation - CLAUSES_METHODS = ["project", "group", "order", "take", "skip"].freeze + CLAUSES_METHODS = ["project", "group", "order", "take", "skip", "on"].freeze attr_reader :relation, :klass def initialize(klass, table = nil) @@ -31,8 +31,14 @@ module ActiveRecord } end - def join(joins) - @relation = @relation.join(@klass.send(:construct_join, joins, nil)) if !joins.blank? + def join(joins, join_type = nil) + if !joins.blank? + if [String, Hash, Array, Symbol].include?(joins.class) + @relation = @relation.join(@klass.send(:construct_join, joins, nil)) + else + @relation = @relation.join(joins, join_type) + end + end self end -- cgit v1.2.3