From 6cf44a1bd64ba10497742d70ad78fe68faa16e99 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Sat, 30 Oct 2010 06:11:06 -0700
Subject: no need to to_i, sqlite does that for us

---
 activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index a4d7d12298..0844e5d7d7 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
-- 
cgit v1.2.3


From 2e07260f36f3e192e5f0b51b287700b419fd1019 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Sat, 30 Oct 2010 06:12:50 -0700
Subject: columns are always strings

---
 activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index 0844e5d7d7..394daef473 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -317,7 +317,7 @@ module ActiveRecord
           exec(sql, name, binds).map do |row|
             record = {}
             row.each do |key, value|
-              record[key.sub(/^"?\w+"?\./, '')] = value if key.is_a?(String)
+              record[key.sub(/^"?\w+"?\./, '')] = value
             end
             record
           end
-- 
cgit v1.2.3


From 2a47e7ef105559c4c931efff8fd14c454a21cf7a Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Sat, 30 Oct 2010 06:16:54 -0700
Subject: only do string substitution on column names once, remove intermediate
 data structures

---
 .../lib/active_record/connection_adapters/sqlite_adapter.rb | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index 394daef473..fc4d84a4f2 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -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
-            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)
-- 
cgit v1.2.3


From b82fab25f999dd6245c23a22f948048eef2d5d9a Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Thu, 21 Oct 2010 17:35:07 +0100
Subject: Refactoring: replace the mix of variables like @finder_sql,
 @counter_sql, etc with just a single scope hash (created on initialization of
 the proxy). This is now used consistently across all associations. Therefore,
 all you have to do to ensure finding/counting etc is done correctly is
 implement the scope correctly.

---
 .../associations/association_collection.rb         | 66 ++++++++++------------
 .../associations/association_proxy.rb              | 19 +++++++
 .../associations/belongs_to_association.rb         | 20 ++++---
 .../belongs_to_polymorphic_association.rb          | 22 ++++----
 .../has_and_belongs_to_many_association.rb         | 37 ++++++------
 .../associations/has_many_association.rb           | 56 ++++++++----------
 .../associations/has_many_through_association.rb   | 16 +-----
 .../associations/has_one_association.rb            | 39 ++++++-------
 .../associations/has_one_through_association.rb    |  2 +-
 .../associations/through_association_scope.rb      | 24 ++++----
 .../lib/active_record/autosave_association.rb      |  4 +-
 11 files changed, 146 insertions(+), 159 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index cb2d9e0a79..896e18af01 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -19,11 +19,6 @@ module ActiveRecord
     # If you need to work on all current children, new and existing records,
     # +load_target+ and the +loaded+ flag are your friends.
     class AssociationCollection < AssociationProxy #:nodoc:
-      def initialize(owner, reflection)
-        super
-        construct_sql
-      end
-
       delegate :group, :order, :limit, :joins, :where, :preload, :eager_load, :includes, :from, :lock, :readonly, :having, :to => :scoped
 
       def select(select = nil)
@@ -36,7 +31,7 @@ module ActiveRecord
       end
 
       def scoped
-        with_scope(construct_scope) { @reflection.klass.scoped }
+        with_scope(@scope) { @reflection.klass.scoped }
       end
 
       def find(*args)
@@ -58,9 +53,7 @@ module ActiveRecord
           merge_options_from_reflection!(options)
           construct_find_options!(options)
 
-          find_scope = construct_scope[:find].slice(:conditions, :order)
-
-          with_scope(:find => find_scope) do
+          with_scope(:find => @scope[:find].slice(:conditions, :order)) do
             relation = @reflection.klass.send(:construct_finder_arel, options, @reflection.klass.send(:current_scoped_methods))
 
             case args.first
@@ -178,17 +171,18 @@ module ActiveRecord
         end
       end
 
-      # Count all records using SQL. If the +:counter_sql+ option is set for the association, it will
-      # be used for the query. If no +:counter_sql+ was supplied, but +:finder_sql+ was set, the
-      # descendant's +construct_sql+ method will have set :counter_sql automatically.
-      # Otherwise, construct options and pass them with scope to the target class's +count+.
+      # Count all records using SQL. If the +:counter_sql+ or +:finder_sql+ option is set for the
+      # association, it will be used for the query. Otherwise, construct options and pass them with
+      # scope to the target class's +count+.
       def count(column_name = nil, options = {})
         column_name, options = nil, column_name if column_name.is_a?(Hash)
 
-        if @reflection.options[:counter_sql] && !options.blank?
-          raise ArgumentError, "If finder_sql/counter_sql is used then options cannot be passed"
-        elsif @reflection.options[:counter_sql]
-          @reflection.klass.count_by_sql(@counter_sql)
+        if @reflection.options[:counter_sql] || @reflection.options[:finder_sql]
+          unless options.blank?
+            raise ArgumentError, "If finder_sql/counter_sql is used then options cannot be passed"
+          end
+          
+          @reflection.klass.count_by_sql(custom_counter_sql)
         else
 
           if @reflection.options[:uniq]
@@ -197,7 +191,7 @@ module ActiveRecord
             options.merge!(:distinct => true)
           end
 
-          value = @reflection.klass.send(:with_scope, construct_scope) { @reflection.klass.count(column_name, options) }
+          value = @reflection.klass.send(:with_scope, @scope) { @reflection.klass.count(column_name, options) }
 
           limit  = @reflection.options[:limit]
           offset = @reflection.options[:offset]
@@ -377,18 +371,6 @@ module ActiveRecord
         def construct_find_options!(options)
         end
 
-        def construct_counter_sql
-          if @reflection.options[:counter_sql]
-            @counter_sql = interpolate_sql(@reflection.options[:counter_sql])
-          elsif @reflection.options[:finder_sql]
-            # replace the SELECT clause with COUNT(*), preserving any hints within /* ... */
-            @reflection.options[:counter_sql] = @reflection.options[:finder_sql].sub(/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im) { "SELECT #{$1}COUNT(*) FROM" }
-            @counter_sql = interpolate_sql(@reflection.options[:counter_sql])
-          else
-            @counter_sql = @finder_sql
-          end
-        end
-
         def load_target
           if !@owner.new_record? || foreign_key_present
             begin
@@ -434,9 +416,9 @@ module ActiveRecord
           elsif @reflection.klass.scopes[method]
             @_named_scopes_cache ||= {}
             @_named_scopes_cache[method] ||= {}
-            @_named_scopes_cache[method][args] ||= with_scope(construct_scope) { @reflection.klass.send(method, *args) }
+            @_named_scopes_cache[method][args] ||= with_scope(@scope) { @reflection.klass.send(method, *args) }
           else
-            with_scope(construct_scope) do
+            with_scope(@scope) do
               if block_given?
                 @reflection.klass.send(method, *args) { |*block_args| yield(*block_args) }
               else
@@ -446,9 +428,19 @@ module ActiveRecord
           end
         end
 
-        # overloaded in derived Association classes to provide useful scoping depending on association type.
-        def construct_scope
-          {}
+        def custom_counter_sql
+          if @reflection.options[:counter_sql]
+            counter_sql = @reflection.options[:counter_sql]
+          else
+            # replace the SELECT clause with COUNT(*), preserving any hints within /* ... */
+            counter_sql = @reflection.options[:finder_sql].sub(/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im) { "SELECT #{$1}COUNT(*) FROM" }
+          end
+          
+          interpolate_sql(counter_sql)
+        end
+        
+        def custom_finder_sql
+          interpolate_sql(@reflection.options[:finder_sql])
         end
 
         def reset_target!
@@ -462,7 +454,7 @@ module ActiveRecord
         def find_target
           records =
             if @reflection.options[:finder_sql]
-              @reflection.klass.find_by_sql(@finder_sql)
+              @reflection.klass.find_by_sql(custom_finder_sql)
             else
               find(:all)
             end
@@ -494,7 +486,7 @@ module ActiveRecord
           ensure_owner_is_not_new
 
           scoped_where = scoped.where_values_hash
-          create_scope = scoped_where ? construct_scope[:create].merge(scoped_where) : construct_scope[:create]
+          create_scope = scoped_where ? @scope[:create].merge(scoped_where) : @scope[:create]
           record = @reflection.klass.send(:with_scope, :create => create_scope) do
             @reflection.build_association(attrs)
           end
diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb
index f333f4d603..0c12c3737d 100644
--- a/activerecord/lib/active_record/associations/association_proxy.rb
+++ b/activerecord/lib/active_record/associations/association_proxy.rb
@@ -61,6 +61,7 @@ module ActiveRecord
         reflection.check_validity!
         Array.wrap(reflection.options[:extend]).each { |ext| proxy_extend(ext) }
         reset
+        construct_scope
       end
 
       # Returns the owner of the proxy.
@@ -203,6 +204,24 @@ module ActiveRecord
           @reflection.klass.send :with_scope, *args, &block
         end
 
+        # Construct the scope used for find/create queries on the target
+        def construct_scope
+          @scope = {
+            :find   => construct_find_scope,
+            :create => construct_create_scope
+          }
+        end
+        
+        # Implemented by subclasses
+        def construct_find_scope
+          raise NotImplementedError
+        end
+        
+        # Implemented by (some) subclasses
+        def construct_create_scope
+          {}
+        end
+
       private
         # Forwards any missing method call to the \target.
         def method_missing(method, *args)
diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb
index 2eb56e5cd3..34b6cd5576 100644
--- a/activerecord/lib/active_record/associations/belongs_to_association.rb
+++ b/activerecord/lib/active_record/associations/belongs_to_association.rb
@@ -50,19 +50,21 @@ module ActiveRecord
                           "find"
                         end
 
-          options = @reflection.options.dup
-          (options.keys - [:select, :include, :readonly]).each do |key|
-            options.delete key
-          end
-          options[:conditions] = conditions
+          options = @reflection.options.dup.slice(:select, :include, :readonly)
 
-          the_target = @reflection.klass.send(find_method,
-            @owner[@reflection.primary_key_name],
-            options
-          ) if @owner[@reflection.primary_key_name]
+          the_target = with_scope(:find => @scope[:find]) do
+            @reflection.klass.send(find_method,
+              @owner[@reflection.primary_key_name],
+              options
+            ) if @owner[@reflection.primary_key_name]
+          end
           set_inverse_instance(the_target, @owner)
           the_target
         end
+        
+        def construct_find_scope
+          { :conditions => conditions }
+        end
 
         def foreign_key_present
           !@owner[@reflection.primary_key_name].nil?
diff --git a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb
index e429806b0c..a0df860623 100644
--- a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb
+++ b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb
@@ -44,20 +44,20 @@ module ActiveRecord
           end
         end
 
+        def construct_find_scope
+          { :conditions => conditions }
+        end
+
         def find_target
           return nil if association_class.nil?
 
-          target =
-            if @reflection.options[:conditions]
-              association_class.find(
-                @owner[@reflection.primary_key_name],
-                :select     => @reflection.options[:select],
-                :conditions => conditions,
-                :include    => @reflection.options[:include]
-              )
-            else
-              association_class.find(@owner[@reflection.primary_key_name], :select => @reflection.options[:select], :include => @reflection.options[:include])
-            end
+          target = association_class.send(:with_scope, :find => @scope[:find]) do
+            association_class.find(
+              @owner[@reflection.primary_key_name],
+              :select  => @reflection.options[:select],
+              :include => @reflection.options[:include]
+            )
+          end
           set_inverse_instance(target, @owner)
           target
         end
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 eb65234dfb..1fc9aba5cf 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
@@ -24,7 +24,7 @@ module ActiveRecord
 
       protected
         def construct_find_options!(options)
-          options[:joins]      = Arel::SqlLiteral.new @join_sql
+          options[:joins]      = Arel::SqlLiteral.new(@scope[:find][:joins])
           options[:readonly]   = finding_with_ambiguous_select?(options[:select] || @reflection.options[:select])
           options[:select]   ||= (@reflection.options[:select] || Arel::SqlLiteral.new('*'))
         end
@@ -80,27 +80,26 @@ module ActiveRecord
             ).delete
           end
         end
+        
+        def construct_joins
+          "INNER JOIN #{@owner.connection.quote_table_name @reflection.options[:join_table]} ON #{@reflection.quoted_table_name}.#{@reflection.klass.primary_key} = #{@owner.connection.quote_table_name @reflection.options[:join_table]}.#{@reflection.association_foreign_key}"
+        end
 
-        def construct_sql
-          if @reflection.options[:finder_sql]
-            @finder_sql = interpolate_sql(@reflection.options[:finder_sql])
-          else
-            @finder_sql = "#{@owner.connection.quote_table_name @reflection.options[:join_table]}.#{@reflection.primary_key_name} = #{owner_quoted_id} "
-            @finder_sql << " AND (#{conditions})" if conditions
-          end
-
-          @join_sql = "INNER JOIN #{@owner.connection.quote_table_name @reflection.options[:join_table]} ON #{@reflection.quoted_table_name}.#{@reflection.klass.primary_key} = #{@owner.connection.quote_table_name @reflection.options[:join_table]}.#{@reflection.association_foreign_key}"
-
-          construct_counter_sql
+        def construct_conditions
+          sql = "#{@owner.connection.quote_table_name @reflection.options[:join_table]}.#{@reflection.primary_key_name} = #{owner_quoted_id} "
+          sql << " AND (#{conditions})" if conditions
+          sql
         end
 
-        def construct_scope
-          { :find => {  :conditions => @finder_sql,
-                        :joins => @join_sql,
-                        :readonly => false,
-                        :order => @reflection.options[:order],
-                        :include => @reflection.options[:include],
-                        :limit => @reflection.options[:limit] } }
+        def construct_find_scope
+          {
+            :conditions => construct_conditions,
+            :joins      => construct_joins,
+            :readonly   => false,
+            :order      => @reflection.options[:order],
+            :include    => @reflection.options[:include],
+            :limit      => @reflection.options[:limit]
+          }
         end
 
         # Join tables with additional columns on top of the two foreign keys must be considered
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index 978fc74560..830a82980d 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -6,14 +6,10 @@ module ActiveRecord
     # If the association has a <tt>:through</tt> option further specialization
     # is provided by its child HasManyThroughAssociation.
     class HasManyAssociation < AssociationCollection #:nodoc:
-      def initialize(owner, reflection)
-        @finder_sql = nil
-        super
-      end
       protected
         def owner_quoted_id
           if @reflection.options[:primary_key]
-            quote_value(@owner.send(@reflection.options[:primary_key]))
+            @owner.class.quote_value(@owner.send(@reflection.options[:primary_key]))
           else
             @owner.quoted_id
           end
@@ -35,10 +31,10 @@ module ActiveRecord
         def count_records
           count = if has_cached_counter?
             @owner.send(:read_attribute, cached_counter_attribute_name)
-          elsif @reflection.options[:counter_sql]
-            @reflection.klass.count_by_sql(@counter_sql)
+          elsif @reflection.options[:counter_sql] || @reflection.options[:finder_sql]
+            @reflection.klass.count_by_sql(custom_counter_sql)
           else
-            @reflection.klass.count(:conditions => @counter_sql, :include => @reflection.options[:include])
+            @reflection.klass.count(@scope[:find].slice(:conditions, :joins, :include))
           end
 
           # If there's nothing in the database and @target has no new records
@@ -87,36 +83,32 @@ module ActiveRecord
           false
         end
 
-        def construct_sql
-          case
-            when @reflection.options[:finder_sql]
-              @finder_sql = interpolate_sql(@reflection.options[:finder_sql])
-
-            when @reflection.options[:as]
-              @finder_sql =
-                "#{@reflection.quoted_table_name}.#{@reflection.options[:as]}_id = #{owner_quoted_id} AND " +
-                "#{@reflection.quoted_table_name}.#{@reflection.options[:as]}_type = #{@owner.class.quote_value(@owner.class.base_class.name.to_s)}"
-              @finder_sql << " AND (#{conditions})" if conditions
-
-            else
-              @finder_sql = "#{@reflection.quoted_table_name}.#{@reflection.primary_key_name} = #{owner_quoted_id}"
-              @finder_sql << " AND (#{conditions})" if conditions
+        def construct_conditions
+          if @reflection.options[:as]
+            sql =
+              "#{@reflection.quoted_table_name}.#{@reflection.options[:as]}_id = #{owner_quoted_id} AND " +
+              "#{@reflection.quoted_table_name}.#{@reflection.options[:as]}_type = #{@owner.class.quote_value(@owner.class.base_class.name.to_s)}"
+          else
+            sql = "#{@reflection.quoted_table_name}.#{@reflection.primary_key_name} = #{owner_quoted_id}"
           end
+          sql << " AND (#{conditions})" if conditions
+          sql
+        end
 
-          construct_counter_sql
+        def construct_find_scope
+          {
+            :conditions => construct_conditions,
+            :readonly   => false,
+            :order      => @reflection.options[:order],
+            :limit      => @reflection.options[:limit],
+            :include    => @reflection.options[:include]
+          }
         end
 
-        def construct_scope
+        def construct_create_scope
           create_scoping = {}
           set_belongs_to_association_for(create_scoping)
-          {
-            :find => { :conditions => @finder_sql,
-                       :readonly => false,
-                       :order => @reflection.options[:order],
-                       :limit => @reflection.options[:limit],
-                       :include => @reflection.options[:include]},
-            :create => create_scoping
-          }
+          create_scoping
         end
 
         def we_can_set_the_inverse_on_this?(record)
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 97883d8393..437c8b1fd6 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -82,21 +82,7 @@ module ActiveRecord
 
         def find_target
           return [] unless target_reflection_has_associated_record?
-          with_scope(construct_scope) { @reflection.klass.find(:all) }
-        end
-
-        def construct_sql
-          case
-            when @reflection.options[:finder_sql]
-              @finder_sql = interpolate_sql(@reflection.options[:finder_sql])
-
-              @finder_sql = "#{@reflection.quoted_table_name}.#{@reflection.primary_key_name} = #{owner_quoted_id}"
-              @finder_sql << " AND (#{conditions})" if conditions
-            else
-              @finder_sql = construct_conditions
-          end
-
-          construct_counter_sql
+          with_scope(@scope) { @reflection.klass.find(:all) }
         end
 
         def has_cached_counter?
diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb
index a6e6bfa356..17901387e9 100644
--- a/activerecord/lib/active_record/associations/has_one_association.rb
+++ b/activerecord/lib/active_record/associations/has_one_association.rb
@@ -2,11 +2,6 @@ module ActiveRecord
   # = Active Record Belongs To Has One Association
   module Associations
     class HasOneAssociation < AssociationProxy #:nodoc:
-      def initialize(owner, reflection)
-        super
-        construct_sql
-      end
-
       def create(attrs = {}, replace_existing = true)
         new_record(replace_existing) do |reflection|
           attrs = merge_with_conditions(attrs)
@@ -79,33 +74,31 @@ module ActiveRecord
 
       private
         def find_target
-          options = @reflection.options.dup
-          (options.keys - [:select, :order, :include, :readonly]).each do |key|
-            options.delete key
-          end
-          options[:conditions] = @finder_sql
+          options = @reflection.options.dup.slice(:select, :order, :include, :readonly)
 
-          the_target = @reflection.klass.find(:first, options)
+          the_target = with_scope(:find => @scope[:find]) do
+            @reflection.klass.find(:first, options)
+          end
           set_inverse_instance(the_target, @owner)
           the_target
         end
 
-        def construct_sql
-          case
-            when @reflection.options[:as]
-              @finder_sql =
-                "#{@reflection.quoted_table_name}.#{@reflection.options[:as]}_id = #{owner_quoted_id} AND " +
-                "#{@reflection.quoted_table_name}.#{@reflection.options[:as]}_type = #{@owner.class.quote_value(@owner.class.base_class.name.to_s)}"
-            else
-              @finder_sql = "#{@reflection.quoted_table_name}.#{@reflection.primary_key_name} = #{owner_quoted_id}"
+        def construct_find_scope
+          if @reflection.options[:as]
+            sql =
+              "#{@reflection.quoted_table_name}.#{@reflection.options[:as]}_id = #{owner_quoted_id} AND " +
+              "#{@reflection.quoted_table_name}.#{@reflection.options[:as]}_type = #{@owner.class.quote_value(@owner.class.base_class.name.to_s)}"
+          else
+            sql = "#{@reflection.quoted_table_name}.#{@reflection.primary_key_name} = #{owner_quoted_id}"
           end
-          @finder_sql << " AND (#{conditions})" if conditions
+          sql << " AND (#{conditions})" if conditions
+          { :conditions => sql }
         end
 
-        def construct_scope
+        def construct_create_scope
           create_scoping = {}
           set_belongs_to_association_for(create_scoping)
-          { :create => create_scoping }
+          create_scoping
         end
 
         def new_record(replace_existing)
@@ -113,7 +106,7 @@ module ActiveRecord
           # instance. Otherwise, if the target has not previously been loaded
           # elsewhere, the instance we create will get orphaned.
           load_target if replace_existing
-          record = @reflection.klass.send(:with_scope, :create => construct_scope[:create]) do
+          record = @reflection.klass.send(:with_scope, :create => @scope[:create]) do
             yield @reflection
           end
 
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 fba0a2bfcc..7f28abf464 100644
--- a/activerecord/lib/active_record/associations/has_one_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_one_through_association.rb
@@ -33,7 +33,7 @@ module ActiveRecord
 
     private
       def find_target
-        with_scope(construct_scope) { @reflection.klass.find(:first) }
+        with_scope(@scope) { @reflection.klass.find(:first) }
       end
     end
   end
diff --git a/activerecord/lib/active_record/associations/through_association_scope.rb b/activerecord/lib/active_record/associations/through_association_scope.rb
index cabb33c4a8..bd8e304e99 100644
--- a/activerecord/lib/active_record/associations/through_association_scope.rb
+++ b/activerecord/lib/active_record/associations/through_association_scope.rb
@@ -5,16 +5,20 @@ module ActiveRecord
 
       protected
 
-      def construct_scope
-        { :create => construct_owner_attributes(@reflection),
-          :find   => { :conditions  => construct_conditions,
-                       :joins       => construct_joins,
-                       :include     => @reflection.options[:include] || @reflection.source_reflection.options[:include],
-                       :select      => construct_select,
-                       :order       => @reflection.options[:order],
-                       :limit       => @reflection.options[:limit],
-                       :readonly    => @reflection.options[:readonly],
-           } }
+      def construct_find_scope
+        {
+          :conditions => construct_conditions,
+          :joins      => construct_joins,
+          :include    => @reflection.options[:include] || @reflection.source_reflection.options[:include],
+          :select     => construct_select,
+          :order      => @reflection.options[:order],
+          :limit      => @reflection.options[:limit],
+          :readonly   => @reflection.options[:readonly]
+        }
+      end
+      
+      def construct_create_scope
+        construct_owner_attributes(@reflection)
       end
 
       # Build SQL conditions from attributes, qualified by table name.
diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index 4a2c078e91..0b89a49896 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -322,8 +322,8 @@ module ActiveRecord
           end
         end
 
-        # reconstruct the SQL queries now that we know the owner's id
-        association.send(:construct_sql) if association.respond_to?(:construct_sql)
+        # reconstruct the scope now that we know the owner's id
+        association.send(:construct_scope) if association.respond_to?(:construct_scope)
       end
     end
 
-- 
cgit v1.2.3


From 0bb85ed9ffa9808926b46e8f7e59cab5b85ac19f Mon Sep 17 00:00:00 2001
From: Ernie Miller <ernie@metautonomo.us>
Date: Wed, 20 Oct 2010 22:54:43 -0400
Subject: Fix issues when including the same association multiple times and
 mixing joins/includes together.

---
 activerecord/lib/active_record/associations.rb     | 48 +++++++++++++++++-----
 .../associations/cascaded_eager_loading_test.rb    | 26 ++++++++++++
 2 files changed, 63 insertions(+), 11 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index 59d328f207..b8cd2aa31e 100644
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -1837,7 +1837,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 = []
@@ -1917,30 +1917,57 @@ 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)
+              case name_or_reflection
+              when Symbol, String
+                join_associations.detect {|j| (j.reflection.name == name_or_reflection.to_s.intern) && (j.parent == parent)}
+              else
+                join_associations.detect {|j| (j.reflection == name_or_reflection) && (j.parent == parent)}
+              end
+            end
+
             def remove_uniq_by_reflection(reflection, records)
               if reflection && reflection.collection?
                 records.each { |record| record.send(reflection.name).target.uniq! }
@@ -2013,7 +2040,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
@@ -2092,8 +2119,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
@@ -2114,7 +2140,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/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
index c7c32da9f3..271bb92ee8 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])
 
-- 
cgit v1.2.3


From 67a3a702951dae905b6270d652dbd14853b01c26 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Sat, 30 Oct 2010 08:45:40 -0700
Subject: refactoring find_join_association

---
 activerecord/lib/active_record/associations.rb | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index b8cd2aa31e..dc466eafc2 100644
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -1960,12 +1960,13 @@ module ActiveRecord
             end
 
             def find_join_association(name_or_reflection, parent)
-              case name_or_reflection
-              when Symbol, String
-                join_associations.detect {|j| (j.reflection.name == name_or_reflection.to_s.intern) && (j.parent == parent)}
-              else
-                join_associations.detect {|j| (j.reflection == name_or_reflection) && (j.parent == 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)
-- 
cgit v1.2.3


From cc9742920ccaf8e985fbe5239edb966949eb91c3 Mon Sep 17 00:00:00 2001
From: Denis Odorcic <denis.odorcic@gmail.com>
Date: Thu, 21 Oct 2010 00:23:05 -0400
Subject: Convert :primary_key in association to a string before comparing to
 column names, so that for example :primary_key => :another_pk works as well
 [#5605 state:resolved]

---
 activerecord/lib/active_record/association_preload.rb              | 2 +-
 .../test/cases/associations/belongs_to_associations_test.rb        | 7 +++++++
 activerecord/test/models/company.rb                                | 1 +
 3 files changed, 9 insertions(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb
index c2a71487dc..911a5155fd 100644
--- a/activerecord/lib/active_record/association_preload.rb
+++ b/activerecord/lib/active_record/association_preload.rb
@@ -320,7 +320,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/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/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
-- 
cgit v1.2.3


From 7d5762d2c2147f8240614997682e5e535a3e4f33 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Sat, 30 Oct 2010 12:28:49 -0700
Subject: no need to merge where values if no new where values have been added

---
 .../lib/active_record/relation/spawn_methods.rb    | 24 ++++++++++++----------
 1 file changed, 13 insertions(+), 11 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb
index 648a02f1cc..d761bd3ea6 100644
--- a/activerecord/lib/active_record/relation/spawn_methods.rb
+++ b/activerecord/lib/active_record/relation/spawn_methods.rb
@@ -28,17 +28,19 @@ 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 = {}
+        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
+      end
 
       merged_relation.where_values = merged_wheres
 
-- 
cgit v1.2.3


From cbca12f9086826dd7243c7c847deea89bbe026b1 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Sat, 30 Oct 2010 12:26:38 -0700
Subject: adding tests for #5234 and #5184. Tests were from Akira Matsuda. 
 Thanks Akira!

---
 activerecord/test/cases/method_scoping_test.rb | 6 ++++++
 activerecord/test/cases/relations_test.rb      | 5 +++++
 2 files changed, 11 insertions(+)

(limited to 'activerecord')

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 d2ccc1480a..f4f3dc4d5a 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
 
-- 
cgit v1.2.3


From 296467fcc40bb3c6a4f42dedc267eb4f313843a9 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Sat, 30 Oct 2010 13:25:49 -0700
Subject: only returning where values for the corresponding relation, also
 filtering where value hash based on table name [#5234 state:resolved] [#5184
 state:resolved]

---
 activerecord/lib/active_record/relation.rb               | 9 +++++++--
 activerecord/lib/active_record/relation/spawn_methods.rb | 9 +++++----
 2 files changed, 12 insertions(+), 6 deletions(-)

(limited to 'activerecord')

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 d761bd3ea6..a61a3bd41c 100644
--- a/activerecord/lib/active_record/relation/spawn_methods.rb
+++ b/activerecord/lib/active_record/relation/spawn_methods.rb
@@ -30,13 +30,14 @@ module ActiveRecord
 
       unless @where_values.empty?
         # Remove duplicates, last one wins.
-        seen = {}
+        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
-            nuke       = seen[name]
-            seen[name] = true
+            name              = w.left.name
+            table             = w.left.relation.name
+            nuke              = seen[table][name]
+            seen[table][name] = true
           end
           nuke
         }.reverse
-- 
cgit v1.2.3