Merge branch 'master' into nested_has_many_through

Conflicts:
	activerecord/CHANGELOG
	activerecord/lib/active_record/associations/class_methods/join_dependency.rb
	activerecord/lib/active_record/associations/class_methods/join_dependency/join_association.rb
	activerecord/lib/active_record/associations/has_many_through_association.rb

12 files changed, 85 insertions, 83 deletions

diff --git a/activerecord/lib/active_record/associations/alias_tracker.rb b/activerecord/lib/active_record/associations/alias_tracker.rb
index 64582188b6..6428fcec0a 100644
--- a/activerecord/lib/active_record/associations/alias_tracker.rb
+++ b/activerecord/lib/active_record/associations/alias_tracker.rb
@@ -5,11 +5,10 @@ module ActiveRecord
     # Keeps track of table aliases for ActiveRecord::Associations::ClassMethods::JoinDependency and
     # ActiveRecord::Associations::ThroughAssociationScope
     class AliasTracker # :nodoc:
-      # other_sql is some other sql which might conflict with the aliases we assign here. Therefore
-      # we store other_sql so that we can scan it before assigning a specific name.
-      def initialize(other_sql = nil)
-        @aliases   = Hash.new
-        @other_sql = other_sql.to_s.downcase
+      # table_joins is an array of arel joins which might conflict with the aliases we assign here
+      def initialize(table_joins = nil)
+        @aliases     = Hash.new
+        @table_joins = table_joins
       end
 
       def aliased_name_for(table_name, aliased_name = nil)
@@ -47,15 +46,24 @@ module ActiveRecord
         def initialize_count_for(name)
           @aliases[name] = 0
 
-          unless @other_sql.blank?
+          unless @table_joins.nil? || Arel::Table === @table_joins
             # quoted_name should be downcased as some database adapters (Oracle) return quoted name in uppercase
-            quoted_name = connection.quote_table_name(name.downcase).downcase
-
-            # Table names
-            @aliases[name] += @other_sql.scan(/join(?:\s+\w+)?\s+#{quoted_name}\son/).size
-
-            # Table aliases
-            @aliases[name] += @other_sql.scan(/join(?:\s+\w+)?\s+\S+\s+#{quoted_name}\son/).size
+            quoted_name = connection.quote_table_name(name).downcase
+
+            @aliases[name] += @table_joins.grep(Arel::Nodes::Join).map { |join|
+              right = join.right
+              case right
+              when Arel::Table
+                right.name.downcase == name ? 1 : 0
+              when String
+                # Table names + table aliases
+                right.downcase.scan(
+                  /join(?:\s+\w+)?\s+(\S+\s+)?#{quoted_name}\son/
+                ).size
+              else
+                0
+              end
+            }.sum
           end
 
           @aliases[name]
diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index ba9373ba6a..abb17a11c6 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -121,13 +121,13 @@ module ActiveRecord
       # Since << flattens its argument list and inserts each record, +push+ and +concat+ behave identically.
       def <<(*records)
         result = true
-        load_target unless @owner.persisted?
+        load_target if @owner.new_record?
 
         transaction do
           flatten_deeper(records).each do |record|
             raise_on_type_mismatch(record)
             add_record_to_target_with_callbacks(record) do |r|
-              result &&= insert_record(record) if @owner.persisted?
+              result &&= insert_record(record) unless @owner.new_record?
             end
           end
         end
@@ -286,12 +286,12 @@ module ActiveRecord
       # This method is abstract in the sense that it relies on
       # +count_records+, which is a method descendants have to provide.
       def size
-        if !@owner.persisted? || (loaded? && !@reflection.options[:uniq])
+        if @owner.new_record? || (loaded? && !@reflection.options[:uniq])
           @target.size
         elsif !loaded? && @reflection.options[:group]
           load_target.size
         elsif !loaded? && !@reflection.options[:uniq] && @target.is_a?(Array)
-          unsaved_records = @target.reject { |r| r.persisted? }
+          unsaved_records = @target.select { |r| r.new_record? }
           unsaved_records.size + count_records
         else
           count_records
@@ -355,7 +355,7 @@ module ActiveRecord
 
       def include?(record)
         return false unless record.is_a?(@reflection.klass)
-        return include_in_memory?(record) unless record.persisted?
+        return include_in_memory?(record) if record.new_record?
         load_target if @reflection.options[:finder_sql] && !loaded?
         loaded? ? @target.include?(record) : exists?(record)
       end
@@ -369,7 +369,7 @@ module ActiveRecord
         end
 
         def load_target
-          if @owner.persisted? || foreign_key_present
+          if !@owner.new_record? || foreign_key_present
             begin
               unless loaded?
                 if @target.is_a?(Array) && @target.any?
@@ -478,7 +478,7 @@ module ActiveRecord
       private
         def create_record(attrs)
           attrs.update(@reflection.options[:conditions]) if @reflection.options[:conditions].is_a?(Hash)
-          ensure_owner_is_not_new
+          ensure_owner_is_persisted!
 
           scoped_where = scoped.where_values_hash
           create_scope = scoped_where ? @scope[:create].merge(scoped_where) : @scope[:create]
@@ -508,7 +508,7 @@ module ActiveRecord
 
           transaction do
             records.each { |record| callback(:before_remove, record) }
-            old_records = records.select { |r| r.persisted? }
+            old_records = records.reject { |r| r.new_record? }
             yield(records, old_records)
             records.each { |record| callback(:after_remove, record) }
           end
@@ -532,15 +532,15 @@ module ActiveRecord
           @owner.class.send(full_callback_name.to_sym) || []
         end
 
-        def ensure_owner_is_not_new
+        def ensure_owner_is_persisted!
           unless @owner.persisted?
             raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved"
           end
         end
 
         def fetch_first_or_last_using_find?(args)
-          (args.first.kind_of?(Hash) && !args.first.empty?) || !(loaded? || !@owner.persisted? || @reflection.options[:finder_sql] ||
-                                         !@target.all? { |record| record.persisted? } || args.first.kind_of?(Integer))
+          (args.first.kind_of?(Hash) && !args.first.empty?) || !(loaded? || @owner.new_record? || @reflection.options[:finder_sql] ||
+            @target.any? { |record| record.new_record? } || args.first.kind_of?(Integer))
         end
 
         def include_in_memory?(record)
diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb
index 7cd04a1ad5..252ff7e7ea 100644
--- a/activerecord/lib/active_record/associations/association_proxy.rb
+++ b/activerecord/lib/active_record/associations/association_proxy.rb
@@ -252,7 +252,7 @@ module ActiveRecord
         def load_target
           return nil unless defined?(@loaded)
 
-          if !loaded? and (@owner.persisted? || foreign_key_present)
+          if !loaded? && (!@owner.new_record? || foreign_key_present)
             @target = find_target
           end
 
diff --git a/activerecord/lib/active_record/associations/class_methods/join_dependency.rb b/activerecord/lib/active_record/associations/class_methods/join_dependency.rb
index ce20420aad..78b634a26c 100644
--- a/activerecord/lib/active_record/associations/class_methods/join_dependency.rb
+++ b/activerecord/lib/active_record/associations/class_methods/join_dependency.rb
@@ -6,13 +6,15 @@ module ActiveRecord
   module Associations
     module ClassMethods
       class JoinDependency # :nodoc:
-        attr_reader :join_parts, :reflections, :alias_tracker
+        attr_reader :join_parts, :reflections, :alias_tracker, :active_record
 
         def initialize(base, associations, joins)
-          @join_parts            = [JoinBase.new(base, joins)]
-          @associations          = {}
-          @reflections           = []
-          @alias_tracker         = AliasTracker.new(joins)
+          @active_record = base
+          @table_joins   = joins
+          @join_parts    = [JoinBase.new(base)]
+          @associations  = {}
+          @reflections   = []
+          @alias_tracker = AliasTracker.new(joins)
           @alias_tracker.aliased_name_for(base.table_name) # Updates the count for base.table_name to 1
           build(associations)
         end
@@ -49,11 +51,11 @@ module ActiveRecord
           records = rows.map { |model|
             primary_id = model[primary_key]
             parent = parents[primary_id] ||= join_base.instantiate(model)
-            construct(parent, @associations, join_associations.dup, model)
+            construct(parent, @associations, join_associations, model)
             parent
           }.uniq
 
-          remove_duplicate_results!(join_base.active_record, records, @associations)
+          remove_duplicate_results!(active_record, records, @associations)
           records
         end
 
@@ -122,7 +124,7 @@ module ActiveRecord
               build(association, parent, join_type)
             end
           when Hash
-            associations.keys.sort{|a,b|a.to_s<=>b.to_s}.each do |name|
+            associations.keys.sort_by { |a| a.to_s }.each do |name|
               join_association = build(name, parent, join_type)
               build(associations[name], join_association, join_type)
             end
diff --git a/activerecord/lib/active_record/associations/class_methods/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/class_methods/join_dependency/join_association.rb
index ed2053d3df..5cc96a7aef 100644
--- a/activerecord/lib/active_record/associations/class_methods/join_dependency/join_association.rb
+++ b/activerecord/lib/active_record/associations/class_methods/join_dependency/join_association.rb
@@ -21,7 +21,7 @@ module ActiveRecord
           attr_reader :aliased_prefix
 
           delegate :options, :through_reflection, :source_reflection, :through_reflection_chain, :to => :reflection
-          delegate :table, :table_name, :to => :parent, :prefix => true
+          delegate :table, :table_name, :to => :parent, :prefix => :parent
           delegate :alias_tracker, :to => :join_dependency
 
           def initialize(reflection, join_dependency, parent = nil)
@@ -50,7 +50,7 @@ module ActiveRecord
 
           def find_parent_in(other_join_dependency)
             other_join_dependency.join_parts.detect do |join_part|
-              self.parent == join_part
+              parent == join_part
             end
           end
 
@@ -129,7 +129,15 @@ module ActiveRecord
               conditions << reflection_conditions(index, table)
               conditions << sti_conditions(reflection, table)
 
-              relation = relation.join(table, join_type).on(*conditions.flatten.compact)
+              ands = relation.create_and(conditions.flatten.compact)
+
+              join = relation.create_join(
+                relation.froms.first,
+                table,
+                relation.create_on(ands),
+                join_type)
+
+              relation = relation.from(join)
 
               # The current table in this iteration becomes the foreign table in the next
               foreign_table = table
@@ -165,10 +173,6 @@ module ActiveRecord
             name
           end
 
-          def interpolate_sql(sql)
-            instance_eval("%@#{sql.gsub('@', '\@')}@", __FILE__, __LINE__)
-          end
-
           private
 
           # Generate aliases and Arel::Table instances for each of the tables which we will
@@ -181,10 +185,7 @@ module ActiveRecord
                 table_alias_for(reflection, reflection != self.reflection)
               )
 
-              table = Arel::Table.new(
-                reflection.table_name, :engine => arel_engine,
-                :as => aliased_table_name, :columns => reflection.klass.columns
-              )
+              table = Arel::Table.new(reflection.table_name, :as => aliased_table_name)
 
               # For habtm, we have two Arel::Table instances related to a single reflection, so
               # we just store them as a pair in the array.
@@ -199,10 +200,7 @@ module ActiveRecord
                   table_alias_for(reflection, true)
                 )
 
-                join_table = Arel::Table.new(
-                  join_table_name, :engine => arel_engine,
-                  :as => aliased_join_table_name
-                )
+                join_table = Arel::Table.new(join_table_name, :as => aliased_join_table_name)
 
                 [table, join_table]
               else
@@ -220,24 +218,23 @@ module ActiveRecord
 
           def reflection_conditions(index, table)
             @reflection.through_conditions.reverse[index].map do |condition|
-              Arel.sql(interpolate_sql(sanitize_sql(
-                condition,
-                table.table_alias || table.name
-              )))
+              Arel.sql(sanitize_sql(condition, table.table_alias || table.name))
             end
           end
 
+          def sanitize_sql(condition, table_name)
+            active_record.send(:sanitize_sql, condition, table_name)
+          end
+
           def sti_conditions(reflection, table)
             unless reflection.klass.descends_from_active_record?
-              sti_column = table[reflection.klass.inheritance_column]
-
-              condition = sti_column.eq(reflection.klass.sti_name)
-
-              reflection.klass.descendants.each do |subclass|
-                condition = condition.or(sti_column.eq(subclass.sti_name))
-              end
+              sti_column    = table[reflection.klass.inheritance_column]
+              sti_condition = sti_column.eq(reflection.klass.sti_name)
+              subclasses    = reflection.klass.descendants
 
-              condition
+              subclasses.inject(sti_condition) { |attr,subclass|
+                attr.or(sti_column.eq(subclass.sti_name))
+              }
             end
           end
 
diff --git a/activerecord/lib/active_record/associations/class_methods/join_dependency/join_base.rb b/activerecord/lib/active_record/associations/class_methods/join_dependency/join_base.rb
index ed05003f66..67567f06df 100644
--- a/activerecord/lib/active_record/associations/class_methods/join_dependency/join_base.rb
+++ b/activerecord/lib/active_record/associations/class_methods/join_dependency/join_base.rb
@@ -3,14 +3,6 @@ module ActiveRecord
     module ClassMethods
       class JoinDependency # :nodoc:
         class JoinBase < JoinPart # :nodoc:
-          # Extra joins provided when the JoinDependency was created
-          attr_reader :table_joins
-
-          def initialize(active_record, joins = nil)
-            super(active_record)
-            @table_joins = joins
-          end
-
           def ==(other)
             other.class == self.class &&
               other.active_record == active_record
@@ -21,7 +13,7 @@ module ActiveRecord
           end
 
           def table
-            Arel::Table.new(table_name, :engine => arel_engine, :columns => active_record.columns)
+            Arel::Table.new(table_name, arel_engine)
           end
 
           def aliased_table_name
diff --git a/activerecord/lib/active_record/associations/class_methods/join_dependency/join_part.rb b/activerecord/lib/active_record/associations/class_methods/join_dependency/join_part.rb
index 0b093b65e9..cd16ae5a8b 100644
--- a/activerecord/lib/active_record/associations/class_methods/join_dependency/join_part.rb
+++ b/activerecord/lib/active_record/associations/class_methods/join_dependency/join_part.rb
@@ -14,7 +14,7 @@ module ActiveRecord
           # association.
           attr_reader :active_record
 
-          delegate :table_name, :column_names, :primary_key, :reflections, :sanitize_sql, :arel_engine, :to => :active_record
+          delegate :table_name, :column_names, :primary_key, :reflections, :arel_engine, :to => :active_record
 
           def initialize(active_record)
             @active_record = active_record
diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
index 2c72fd0004..e2ce9aefcf 100644
--- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
@@ -34,7 +34,7 @@ module ActiveRecord
         end
 
         def insert_record(record, force = true, validate = true)
-          unless record.persisted?
+          if record.new_record?
             if force
               record.save!
             else
@@ -49,7 +49,7 @@ module ActiveRecord
             timestamps = record_timestamp_columns(record)
             timezone   = record.send(:current_time_from_proper_timezone) if timestamps.any?
 
-            attributes = Hash[columns.map do |column|
+            attributes = columns.map do |column|
               name = column.name
               value = case name.to_s
                 when @reflection.primary_key_name.to_s
@@ -62,9 +62,10 @@ module ActiveRecord
                   @owner.send(:quote_value, record[name], column) if record.has_attribute?(name)
               end
               [relation[name], value] unless value.nil?
-            end]
+            end
 
-            relation.insert(attributes)
+            stmt = relation.compile_insert Hash[attributes]
+            @owner.connection.insert stmt.to_sql
           end
 
           true
@@ -75,9 +76,10 @@ module ActiveRecord
             records.each { |record| @owner.connection.delete(interpolate_sql(sql, record)) }
           else
             relation = Arel::Table.new(@reflection.options[:join_table])
-            relation.where(relation[@reflection.primary_key_name].eq(@owner.id).
+            stmt = relation.where(relation[@reflection.primary_key_name].eq(@owner.id).
               and(relation[@reflection.association_foreign_key].in(records.map { |x| x.id }.compact))
-            ).delete
+            ).compile_delete
+            @owner.connection.delete stmt.to_sql
           end
         end
 
@@ -113,7 +115,7 @@ module ActiveRecord
       private
         def create_record(attributes, &block)
           # Can't use Base.create because the foreign key may be a protected attribute.
-          ensure_owner_is_not_new
+          ensure_owner_is_persisted!
           if attributes.is_a?(Array)
             attributes.collect { |attr| create(attr) }
           else
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index bbf62796bb..851b10e300 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -67,9 +67,10 @@ module ActiveRecord
               @reflection.klass.delete(records.map { |record| record.id })
             else
               relation = Arel::Table.new(@reflection.table_name)
-              relation.where(relation[@reflection.primary_key_name].eq(@owner.id).
+              stmt = relation.where(relation[@reflection.primary_key_name].eq(@owner.id).
                   and(relation[@reflection.klass.primary_key].in(records.map { |r| r.id }))
-              ).update(relation[@reflection.primary_key_name] => nil)
+              ).compile_update(relation[@reflection.primary_key_name] => nil)
+              @owner.connection.update stmt.to_sql
 
               @owner.class.update_counters(@owner.id, cached_counter_attribute_name => -records.size) if has_cached_counter?
           end
diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb
index c1fc16b0ed..0b00132ad9 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -42,7 +42,7 @@ module ActiveRecord
       protected
         def create_record(attrs, force = true)
           ensure_not_nested
-          ensure_owner_is_not_new
+          ensure_owner_is_persisted!
 
           transaction do
             object = @reflection.klass.new(attrs)
@@ -67,7 +67,7 @@ module ActiveRecord
         def insert_record(record, force = true, validate = true)
           ensure_not_nested
 
-          unless record.persisted?
+          if record.new_record?
             if force
               record.save!
             else
diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb
index 0ccf07f15e..d581939f04 100644
--- a/activerecord/lib/active_record/associations/has_one_association.rb
+++ b/activerecord/lib/active_record/associations/has_one_association.rb
@@ -56,7 +56,7 @@ module ActiveRecord
         set_inverse_instance(obj, @owner)
         @loaded = true
 
-        unless !@owner.persisted? or obj.nil? or dont_save
+        unless !@owner.persisted? || obj.nil? || dont_save
           return (obj.save ? self : false)
         else
           return (obj.nil? ? nil : self)
diff --git a/activerecord/lib/active_record/associations/has_one_through_association.rb b/activerecord/lib/active_record/associations/has_one_through_association.rb
index cfd4637e8e..e9dc32efd3 100644
--- a/activerecord/lib/active_record/associations/has_one_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_one_through_association.rb
@@ -23,7 +23,7 @@ module ActiveRecord
         if current_object
            new_value ? current_object.update_attributes(construct_join_attributes(new_value)) : current_object.destroy
         elsif new_value
-          unless @owner.persisted?
+          if @owner.new_record?
             self.target = new_value
             through_association = @owner.send(:association_instance_get, @reflection.through_reflection.name)
             through_association.build(construct_join_attributes(new_value))