Merge branch 'master' into nested_has_many_through

Conflicts:
	activerecord/lib/active_record/associations/has_many_association.rb
	activerecord/lib/active_record/associations/through_association_scope.rb

10 files changed, 113 insertions, 34 deletions

diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb
index bafd59fc02..c3ccb93ffd 100644
--- a/activerecord/lib/active_record/association_preload.rb
+++ b/activerecord/lib/active_record/association_preload.rb
@@ -338,7 +338,7 @@ module ActiveRecord
           klass = klass_name.constantize
 
           table_name = klass.quoted_table_name
-          primary_key = reflection.options[:primary_key] || klass.primary_key
+          primary_key = (reflection.options[:primary_key] || klass.primary_key).to_s
           column_type = klass.columns.detect{|c| c.name == primary_key}.type
 
           ids = _id_map.keys.map do |id|
diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index ee2e2765bc..37dbff4061 100644
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -1880,7 +1880,7 @@ module ActiveRecord
 
           def initialize(base, associations, joins)
             @join_parts            = [JoinBase.new(base, joins)]
-            @associations          = associations
+            @associations          = {}
             @reflections           = []
             @base_records_hash     = {}
             @base_records_in_order = []
@@ -1949,30 +1949,58 @@ module ActiveRecord
 
           protected
 
+            def cache_joined_association(association)
+              associations = []
+              parent = association.parent
+              while parent != join_base
+                associations.unshift(parent.reflection.name)
+                parent = parent.parent
+              end
+              ref = @associations
+              associations.each do |key|
+                ref = ref[key]
+              end
+              ref[association.reflection.name] ||= {}
+            end
+
             def build(associations, parent = nil, join_type = Arel::InnerJoin)
               parent ||= join_parts.last
               case associations
                 when Symbol, String
                   reflection = parent.reflections[associations.to_s.intern] or
                   raise ConfigurationError, "Association named '#{ associations }' was not found; perhaps you misspelled it?"
-                  @reflections << reflection
-                  join_association = build_join_association(reflection, parent)
-                  join_association.join_type = join_type
-                  @join_parts << join_association
+                  unless join_association = find_join_association(reflection, parent)
+                    @reflections << reflection
+                    join_association = build_join_association(reflection, parent)
+                    join_association.join_type = join_type
+                    @join_parts << join_association
+                    cache_joined_association(join_association)
+                  end
+                  join_association
                 when Array
                   associations.each do |association|
                     build(association, parent, join_type)
                   end
                 when Hash
                   associations.keys.sort{|a,b|a.to_s<=>b.to_s}.each do |name|
-                    build(name, parent, join_type)
-                    build(associations[name], nil, join_type)
+                    join_association = build(name, parent, join_type)
+                    build(associations[name], join_association, join_type)
                   end
                 else
                   raise ConfigurationError, associations.inspect
               end
             end
 
+            def find_join_association(name_or_reflection, parent)
+              if String === name_or_reflection
+                name_or_reflection = name_or_reflection.to_sym
+              end
+
+              join_associations.detect { |j|
+                j.reflection == name_or_reflection && j.parent == parent
+              }
+            end
+
             def remove_uniq_by_reflection(reflection, records)
               if reflection && reflection.collection?
                 records.each { |record| record.send(reflection.name).target.uniq! }
@@ -2045,7 +2073,7 @@ module ActiveRecord
             end
 
           # A JoinPart represents a part of a JoinDependency. It is an abstract class, inherited
-          # by JoinBase and JoinAssociation. A JoinBase represents the Active Record which 
+          # by JoinBase and JoinAssociation. A JoinBase represents the Active Record which
           # everything else is being joined onto. A JoinAssociation represents an association which
           # is joining to the base. A JoinAssociation may result in more than one actual join
           # operations (for example a has_and_belongs_to_many JoinAssociation would result in
@@ -2124,8 +2152,7 @@ module ActiveRecord
 
             def ==(other)
               other.class == self.class &&
-              other.active_record == active_record &&
-              other.table_joins == table_joins
+              other.active_record == active_record
             end
 
             def aliased_prefix
@@ -2146,7 +2173,7 @@ module ActiveRecord
             attr_reader :reflection
 
             # The JoinDependency object which this JoinAssociation exists within. This is mainly
-            # relevant for generating aliases which do not conflict with other joins which are 
+            # relevant for generating aliases which do not conflict with other joins which are
             # part of the query.
             attr_reader :join_dependency
 
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index a4d7d12298..fc4d84a4f2 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -226,7 +226,7 @@ module ActiveRecord
           IndexDefinition.new(
             table_name,
             row['name'],
-            row['unique'].to_i != 0,
+            row['unique'] != 0,
             exec("PRAGMA index_info('#{row['name']}')").map { |col|
               col['name']
             })
@@ -235,7 +235,7 @@ module ActiveRecord
 
       def primary_key(table_name) #:nodoc:
         column = table_structure(table_name).find { |field|
-          field['pk'].to_i == 1
+          field['pk'] == 1
         }
         column && column['name']
       end
@@ -314,13 +314,12 @@ module ActiveRecord
 
       protected
         def select(sql, name = nil, binds = []) #:nodoc:
-          exec(sql, name, binds).map do |row|
-            record = {}
-            row.each do |key, value|
-              record[key.sub(/^"?\w+"?\./, '')] = value if key.is_a?(String)
-            end
-            record
-          end
+          result = exec(sql, name, binds)
+          columns = result.columns.map { |column|
+            column.sub(/^"?\w+"?\./, '')
+          }
+
+          result.rows.map { |row| Hash[columns.zip(row)] }
         end
 
         def table_structure(table_name)
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index 58f7b74198..3b22be78cb 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -319,8 +319,13 @@ module ActiveRecord
     end
 
     def where_values_hash
-      Hash[@where_values.find_all {|w| w.respond_to?(:operator) && w.operator == :== }.map {|where|
-        [where.operand1.name, where.operand2.respond_to?(:value) ? where.operand2.value : where.operand2]
+      Hash[@where_values.find_all { |w|
+        w.respond_to?(:operator) && w.operator == :== && w.left.relation.name == table_name
+      }.map { |where|
+        [
+          where.left.name,
+          where.right.respond_to?(:value) ? where.right.value : where.right
+        ]
       }]
     end
 
diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb
index 648a02f1cc..a61a3bd41c 100644
--- a/activerecord/lib/active_record/relation/spawn_methods.rb
+++ b/activerecord/lib/active_record/relation/spawn_methods.rb
@@ -28,17 +28,20 @@ module ActiveRecord
 
       merged_wheres = @where_values + r.where_values
 
-      # Remove duplicates, last one wins.
-      seen = {}
-      merged_wheres = merged_wheres.reverse.reject { |w|
-        nuke = false
-        if w.respond_to?(:operator) && w.operator == :==
-          name       = w.left.name
-          nuke       = seen[name]
-          seen[name] = true
-        end
-        nuke
-      }.reverse
+      unless @where_values.empty?
+        # Remove duplicates, last one wins.
+        seen = Hash.new { |h,table| h[table] = {} }
+        merged_wheres = merged_wheres.reverse.reject { |w|
+          nuke = false
+          if w.respond_to?(:operator) && w.operator == :==
+            name              = w.left.name
+            table             = w.left.relation.name
+            nuke              = seen[table][name]
+            seen[table][name] = true
+          end
+          nuke
+        }.reverse
+      end
 
       merged_relation.where_values = merged_wheres
 
diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb
index cbaa4990f7..0fa4328826 100644
--- a/activerecord/test/cases/associations/belongs_to_associations_test.rb
+++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb
@@ -81,6 +81,13 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase
     assert_not_nil citibank_result.instance_variable_get("@firm_with_primary_key")
   end
 
+  def test_eager_loading_with_primary_key_as_symbol
+    Firm.create("name" => "Apple")
+    Client.create("name" => "Citibank", :firm_name => "Apple")
+    citibank_result = Client.find(:first, :conditions => {:name => "Citibank"}, :include => :firm_with_primary_key_symbols)
+    assert_not_nil citibank_result.instance_variable_get("@firm_with_primary_key_symbols")
+  end
+
   def test_no_unexpected_aliasing
     first_firm = companies(:first_firm)
     another_firm = companies(:another_firm)
diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
index 5b24d49a7d..81e99036af 100644
--- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
+++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
@@ -3,6 +3,7 @@ require 'models/post'
 require 'models/comment'
 require 'models/author'
 require 'models/categorization'
+require 'models/category'
 require 'models/company'
 require 'models/topic'
 require 'models/reply'
@@ -45,6 +46,31 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase
     assert_equal people(:michael), Person.eager_load(:primary_contact => :primary_contact).where('primary_contacts_people_2.first_name = ?', 'Susan').order('people.id').first
   end
 
+  def test_cascaded_eager_association_loading_with_join_for_count
+    categories = Category.joins(:categorizations).includes([{:posts=>:comments}, :authors])
+
+    assert_nothing_raised do
+      assert_equal 2, categories.count
+      assert_equal 2, categories.all.uniq.size # Must uniq since instantiating with inner joins will get dupes
+    end
+  end
+
+  def test_cascaded_eager_association_loading_with_duplicated_includes
+    categories = Category.includes(:categorizations).includes(:categorizations => :author).where("categorizations.id is not null")
+    assert_nothing_raised do
+      assert_equal 2, categories.count
+      assert_equal 2, categories.all.size
+    end
+  end
+
+  def test_cascaded_eager_association_loading_with_twice_includes_edge_cases
+    categories = Category.includes(:categorizations => :author).includes(:categorizations => :post).where("posts.id is not null")
+    assert_nothing_raised do
+      assert_equal 2, categories.count
+      assert_equal 2, categories.all.size
+    end
+  end
+
   def test_eager_association_loading_with_join_for_count
     authors = Author.joins(:special_posts).includes([:posts, :categorizations])
 
diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb
index f3d3d62830..0ffd0e2ab3 100644
--- a/activerecord/test/cases/method_scoping_test.rb
+++ b/activerecord/test/cases/method_scoping_test.rb
@@ -226,6 +226,12 @@ class MethodScopingTest < ActiveRecord::TestCase
     assert Post.find(1).comments.include?(new_comment)
   end
 
+  def test_scoped_create_with_join_and_merge
+    (Comment.where(:body => "but Who's Buying?").joins(:post) & Post.where(:body => 'Peace Sells...')).with_scope do
+      assert_equal({:body => "but Who's Buying?"}, Comment.scoped.scope_for_create)
+    end
+  end
+
   def test_immutable_scope
     options = { :conditions => "name = 'David'" }
     Developer.send(:with_scope, :find => options) do
diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index b47ef7592d..268cea27be 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -500,6 +500,11 @@ class RelationTest < ActiveRecord::TestCase
     end
   end
 
+  def test_relation_merging_with_joins
+    comments = Comment.joins(:post).where(:body => 'Thank you for the welcome') & Post.where(:body => 'Such a lovely day')
+    assert_equal 1, comments.count
+  end
+
   def test_count
     posts = Post.scoped
 
diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb
index be6dd71e3b..ee5f77b613 100644
--- a/activerecord/test/models/company.rb
+++ b/activerecord/test/models/company.rb
@@ -107,6 +107,7 @@ class Client < Company
   belongs_to :firm_with_other_name, :class_name => "Firm", :foreign_key => "client_of"
   belongs_to :firm_with_condition, :class_name => "Firm", :foreign_key => "client_of", :conditions => ["1 = ?", 1]
   belongs_to :firm_with_primary_key, :class_name => "Firm", :primary_key => "name", :foreign_key => "firm_name"
+  belongs_to :firm_with_primary_key_symbols, :class_name => "Firm", :primary_key => :name, :foreign_key => :firm_name
   belongs_to :readonly_firm, :class_name => "Firm", :foreign_key => "firm_id", :readonly => true
 
   # Record destruction so we can test whether firm.clients.clear has