From 8233f8314ba00838e8ef6b2d95cdfb7d58c8dece Mon Sep 17 00:00:00 2001
From: Will Bryant <will.bryant@gmail.com>
Date: Wed, 24 Sep 2008 16:44:56 +1200
Subject: wrote a test showing eager loading's misbehavior (sanitizing against
 the wrong table) when the association has a :conditions hash

Signed-off-by: Michael Koziarski <michael@koziarski.com>
---
 activerecord/test/cases/associations/eager_test.rb | 7 +++++++
 1 file changed, 7 insertions(+)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb
index e78624a98d..3dd2cb028a 100644
--- a/activerecord/test/cases/associations/eager_test.rb
+++ b/activerecord/test/cases/associations/eager_test.rb
@@ -116,6 +116,13 @@ class EagerAssociationTest < ActiveRecord::TestCase
     assert_equal 2, posts.first.comments.size
   end
 
+  def test_loading_from_an_association_that_has_a_hash_of_conditions
+    assert_nothing_raised do
+      Author.find(:all, :include => :hello_posts_with_hash_conditions)
+    end
+    assert !Author.find(authors(:david).id, :include => :hello_posts_with_hash_conditions).hello_posts.empty?
+  end
+
   def test_loading_with_no_associations
     assert_nil Post.find(posts(:authorless).id, :include => :author).author
   end
-- 
cgit v1.2.3


From 35d731ef0aaf1b6103121bb2c2d21f5b17c26b3c Mon Sep 17 00:00:00 2001
From: Will Bryant <will.bryant@gmail.com>
Date: Wed, 24 Sep 2008 17:00:35 +1200
Subject: fix eager loading's :condition sanitizing expanding against the wrong
 table

Signed-off-by: Michael Koziarski <michael@koziarski.com>
---
 activerecord/lib/active_record/association_preload.rb | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb
index c60850fc77..284dc7dca0 100644
--- a/activerecord/lib/active_record/association_preload.rb
+++ b/activerecord/lib/active_record/association_preload.rb
@@ -96,7 +96,7 @@ module ActiveRecord
         options = reflection.options
 
         conditions = "t0.#{reflection.primary_key_name} #{in_or_equals_for_ids(ids)}"
-        conditions << append_conditions(options, preload_options)
+        conditions << append_conditions(reflection, preload_options)
 
         associated_records = reflection.klass.find(:all, :conditions => [conditions, ids],
         :include => options[:include],
@@ -233,7 +233,7 @@ module ActiveRecord
             end
           end
           conditions = "#{table_name}.#{connection.quote_column_name(primary_key)} #{in_or_equals_for_ids(ids)}"
-          conditions << append_conditions(options, preload_options)
+          conditions << append_conditions(reflection, preload_options)
           associated_records = klass.find(:all, :conditions => [conditions, ids],
                                           :include => options[:include],
                                           :select => options[:select],
@@ -254,7 +254,7 @@ module ActiveRecord
           conditions = "#{reflection.klass.quoted_table_name}.#{foreign_key} #{in_or_equals_for_ids(ids)}"
         end
 
-        conditions << append_conditions(options, preload_options)
+        conditions << append_conditions(reflection, preload_options)
 
         reflection.klass.find(:all,
                               :select => (preload_options[:select] || options[:select] || "#{table_name}.*"),
@@ -270,9 +270,9 @@ module ActiveRecord
         instance_eval("%@#{sql.gsub('@', '\@')}@")
       end
 
-      def append_conditions(options, preload_options)
+      def append_conditions(reflection, preload_options)
         sql = ""
-        sql << " AND (#{interpolate_sql_for_preload(sanitize_sql(options[:conditions]))})" if options[:conditions]
+        sql << " AND (#{interpolate_sql_for_preload(reflection.sanitized_conditions)})" if reflection.sanitized_conditions
         sql << " AND (#{sanitize_sql preload_options[:conditions]})" if preload_options[:conditions]
         sql
       end
-- 
cgit v1.2.3


From 8d337e9ec2e25007d557150dbe7557ab3c3bd05f Mon Sep 17 00:00:00 2001
From: Eloy Duran <eloy.de.enige@gmail.com>
Date: Fri, 3 Oct 2008 13:55:35 +0200
Subject: Dynamic finders should use the ActiveRecord::Base::find method
 instead of ::find_initial, :find_last, and ::find_all.

This is so when people override ActiveRecord::Base::find, the new ::find method will also be invoked by the dynamic finders.
Associations for instance do go through ::find, so this makes it more consistent.
Also removed the unnecessary deprecation silence blocks.

Signed-off-by: Michael Koziarski <michael@koziarski.com>

[#1162 state:committed]
---
 activerecord/lib/active_record/base.rb             |  8 +++----
 .../lib/active_record/dynamic_finder_match.rb      |  8 +++----
 activerecord/test/cases/finder_test.rb             | 27 ++++++++++++++++++----
 3 files changed, 30 insertions(+), 13 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 69ea155ace..3e8c573d09 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -1724,10 +1724,10 @@ module ActiveRecord #:nodoc:
 
                   #{'result = ' if bang}if options[:conditions]
                     with_scope(:find => finder_options) do
-                      ActiveSupport::Deprecation.silence { send(:#{finder}, options) }
+                      find(:#{finder}, options)
                     end
                   else
-                    ActiveSupport::Deprecation.silence { send(:#{finder}, options.merge(finder_options)) }
+                    find(:#{finder}, options.merge(finder_options))
                   end
                   #{'result || raise(RecordNotFound)' if bang}
                 end
@@ -1750,9 +1750,9 @@ module ActiveRecord #:nodoc:
                   options = { :conditions => find_attributes }
                   set_readonly_option!(options)
 
-                  record = find_initial(options)
+                  record = find(:first, options)
 
-                   if record.nil?
+                  if record.nil?
                     record = self.new { |r| r.send(:attributes=, attributes, guard_protected_attributes) }
                     #{'yield(record) if block_given?'}
                     #{'record.save' if instantiator == :create}
diff --git a/activerecord/lib/active_record/dynamic_finder_match.rb b/activerecord/lib/active_record/dynamic_finder_match.rb
index f4a5712981..8f9f05ce36 100644
--- a/activerecord/lib/active_record/dynamic_finder_match.rb
+++ b/activerecord/lib/active_record/dynamic_finder_match.rb
@@ -6,11 +6,11 @@ module ActiveRecord
     end
 
     def initialize(method)
-      @finder = :find_initial
+      @finder = :first
       case method.to_s
       when /^find_(all_by|last_by|by)_([_a-zA-Z]\w*)$/
-        @finder = :find_last if $1 == 'last_by'
-        @finder = :find_every if $1 == 'all_by'
+        @finder = :last if $1 == 'last_by'
+        @finder = :all if $1 == 'all_by'
         names = $2
       when /^find_by_([_a-zA-Z]\w*)\!$/
         @bang = true
@@ -31,7 +31,7 @@ module ActiveRecord
     end
 
     def instantiator?
-      @finder == :find_initial && !@instantiator.nil?
+      @finder == :first && !@instantiator.nil?
     end
 
     def bang?
diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
index 292b88edbc..853474916c 100644
--- a/activerecord/test/cases/finder_test.rb
+++ b/activerecord/test/cases/finder_test.rb
@@ -21,7 +21,7 @@ class DynamicFinderMatchTest < ActiveRecord::TestCase
     match = ActiveRecord::DynamicFinderMatch.match("find_by_age_and_sex_and_location")
     assert_not_nil match
     assert match.finder?
-    assert_equal :find_initial, match.finder
+    assert_equal :first, match.finder
     assert_equal %w(age sex location), match.attribute_names
   end
 
@@ -30,7 +30,7 @@ class DynamicFinderMatchTest < ActiveRecord::TestCase
     assert_not_nil match
     assert match.finder?
     assert match.bang?
-    assert_equal :find_initial, match.finder
+    assert_equal :first, match.finder
     assert_equal %w(age sex location), match.attribute_names
   end
 
@@ -38,7 +38,7 @@ class DynamicFinderMatchTest < ActiveRecord::TestCase
     match = ActiveRecord::DynamicFinderMatch.match("find_all_by_age_and_sex_and_location")
     assert_not_nil match
     assert match.finder?
-    assert_equal :find_every, match.finder
+    assert_equal :all, match.finder
     assert_equal %w(age sex location), match.attribute_names
   end
 
@@ -47,7 +47,7 @@ class DynamicFinderMatchTest < ActiveRecord::TestCase
     assert_not_nil match
     assert !match.finder?
     assert match.instantiator?
-    assert_equal :find_initial, match.finder
+    assert_equal :first, match.finder
     assert_equal :new, match.instantiator
     assert_equal %w(age sex location), match.attribute_names
   end
@@ -57,7 +57,7 @@ class DynamicFinderMatchTest < ActiveRecord::TestCase
     assert_not_nil match
     assert !match.finder?
     assert match.instantiator?
-    assert_equal :find_initial, match.finder
+    assert_equal :first, match.finder
     assert_equal :create, match.instantiator
     assert_equal %w(age sex location), match.attribute_names
   end
@@ -500,6 +500,23 @@ class FinderTest < ActiveRecord::TestCase
     assert_equal(2, Entrant.count_by_sql(["SELECT COUNT(*) FROM entrants WHERE id > ?", 1]))
   end
 
+  uses_mocha('test_dynamic_finder_should_go_through_the_find_class_method') do
+    def test_dynamic_finders_should_go_through_the_find_class_method
+      Topic.expects(:find).with(:first, :conditions => { :title => 'The First Topic!' })
+      Topic.find_by_title("The First Topic!")
+
+      Topic.expects(:find).with(:last, :conditions => { :title => 'The Last Topic!' })
+      Topic.find_last_by_title("The Last Topic!")
+
+      Topic.expects(:find).with(:all, :conditions => { :title => 'A Topic.' })
+      Topic.find_all_by_title("A Topic.")
+
+      Topic.expects(:find).with(:first, :conditions => { :title => 'Does not exist yet for sure!' }).times(2)
+      Topic.find_or_initialize_by_title('Does not exist yet for sure!')
+      Topic.find_or_create_by_title('Does not exist yet for sure!')
+    end
+  end
+
   def test_find_by_one_attribute
     assert_equal topics(:first), Topic.find_by_title("The First Topic")
     assert_nil Topic.find_by_title("The First Topic!")
-- 
cgit v1.2.3


From 7553a23c0a84424bdbc09cc81791f41bfebe1b25 Mon Sep 17 00:00:00 2001
From: Michael Koziarski <michael@koziarski.com>
Date: Fri, 3 Oct 2008 21:35:01 +0200
Subject: Remove AS for oracle compatibility

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

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb
index 80992dd34b..634236e959 100644
--- a/activerecord/lib/active_record/calculations.rb
+++ b/activerecord/lib/active_record/calculations.rb
@@ -217,7 +217,7 @@ module ActiveRecord
 
           sql << " ORDER BY #{options[:order]} "       if options[:order]
           add_limit!(sql, options, scope)
-          sql << ") AS #{aggregate_alias}_subquery" if use_workaround
+          sql << ") #{aggregate_alias}_subquery" if use_workaround
           sql
         end
 
-- 
cgit v1.2.3


From 1bc267d21679408b3624d1c697656ec250c01972 Mon Sep 17 00:00:00 2001
From: Luca Guidi <guidi.luca@gmail.com>
Date: Fri, 3 Oct 2008 16:08:17 +0200
Subject: Make sure recreate MySQL test database with the proper encoding and
 collation [#1165 state:resolved]

Signed-off-by: Michael Koziarski <michael@koziarski.com>
[#1165 state:committed]
---
 activerecord/lib/active_record/connection_adapters/mysql_adapter.rb | 4 ++--
 activerecord/test/cases/active_schema_test_mysql.rb                 | 5 +++++
 2 files changed, 7 insertions(+), 2 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index a26fd02b90..3aa27bfc99 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -371,9 +371,9 @@ module ActiveRecord
         end
       end
 
-      def recreate_database(name) #:nodoc:
+      def recreate_database(name, options = {}) #:nodoc:
         drop_database(name)
-        create_database(name)
+        create_database(name, options)
       end
 
       # Create a new MySQL database with optional <tt>:charset</tt> and <tt>:collation</tt>.
diff --git a/activerecord/test/cases/active_schema_test_mysql.rb b/activerecord/test/cases/active_schema_test_mysql.rb
index 2a42dc3517..9aff538ce9 100644
--- a/activerecord/test/cases/active_schema_test_mysql.rb
+++ b/activerecord/test/cases/active_schema_test_mysql.rb
@@ -25,6 +25,11 @@ class ActiveSchemaTest < ActiveRecord::TestCase
       assert_equal "CREATE DATABASE `aimonetti` DEFAULT CHARACTER SET `latin1`", create_database(:aimonetti, {:charset => 'latin1'})
       assert_equal "CREATE DATABASE `matt_aimonetti` DEFAULT CHARACTER SET `big5` COLLATE `big5_chinese_ci`", create_database(:matt_aimonetti, {:charset => :big5, :collation => :big5_chinese_ci})
     end
+
+    def test_recreate_mysql_database_with_encoding
+      create_database(:luca, {:charset => 'latin1'})
+      assert_equal "CREATE DATABASE `luca` DEFAULT CHARACTER SET `latin1`", recreate_database(:luca, {:charset => 'latin1'})
+    end
   end
 
   def test_add_column
-- 
cgit v1.2.3


From f2dee4915306e6bd1554e47640d201600ac39c41 Mon Sep 17 00:00:00 2001
From: Chris Kampmeier <chris@kampers.net>
Date: Sat, 4 Oct 2008 00:16:29 -0700
Subject: Fix typography and punctuation in ActiveRecord#find_by_sql

---
 activerecord/lib/active_record/base.rb | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index ac15eed408..6fb05b5551 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -610,8 +610,8 @@ module ActiveRecord #:nodoc:
 
       # Executes a custom SQL query against your database and returns all the results.  The results will
       # be returned as an array with columns requested encapsulated as attributes of the model you call
-      # this method from.  If you call +Product.find_by_sql+ then the results will be returned in a Product
-      # object with the attributes you specified in the SQL query.
+      # this method from.  If you call <tt>Product.find_by_sql</tt> then the results will be returned in
+      # a Product object with the attributes you specified in the SQL query.
       #
       # If you call a complicated SQL query which spans multiple tables the columns specified by the
       # SELECT will be attributes of the model, whether or not they are columns of the corresponding
@@ -620,7 +620,7 @@ module ActiveRecord #:nodoc:
       # The +sql+ parameter is a full SQL query as a string.  It will be called as is, there will be
       # no database agnostic conversions performed.  This should be a last resort because using, for example,
       # MySQL specific terms will lock you to using that particular database engine or require you to
-      # change your call if you switch engines
+      # change your call if you switch engines.
       #
       # ==== Examples
       #   # A simple SQL query spanning multiple tables
-- 
cgit v1.2.3


From 2fc835d646ec4c420639a2b5e8edd5fb204c0414 Mon Sep 17 00:00:00 2001
From: Mike Gunderloy <MikeG1@larkfarm.com>
Date: Sat, 4 Oct 2008 08:30:20 -0500
Subject: Remove erroneous warning about counter_cache column defaults.

---
 activerecord/lib/active_record/associations.rb | 2 --
 1 file changed, 2 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index 4d36216c34..d1a0b2f96a 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -955,8 +955,6 @@ module ActiveRecord
       #   destroyed. This requires that a column named <tt>#{table_name}_count</tt> (such as +comments_count+ for a belonging Comment class)
       #   is used on the associate class (such as a Post class). You can also specify a custom counter cache column by providing
       #   a column name instead of a +true+/+false+ value to this option (e.g., <tt>:counter_cache => :my_custom_counter</tt>.)
-      #   When creating a counter cache column, the database statement or migration must specify a default value of <tt>0</tt>, failing to do 
-      #   this results in a counter with +NULL+ value, which will never increment.
       #   Note: Specifying a counter cache will add it to that model's list of readonly attributes using +attr_readonly+.
       # [:include]
       #   Specify second-order associations that should be eager loaded when this object is loaded.
-- 
cgit v1.2.3


From 4cb3d27443fa30fde528f07658e76537f38a0661 Mon Sep 17 00:00:00 2001
From: Aliaksey Kandratsenka <alk@tut.by>
Date: Sat, 6 Sep 2008 11:22:01 +0300
Subject: don't quote decimal values for mysql. It doesn't make sense and
 breaks in newer versions of mysql

Signed-off-by: Michael Koziarski <michael@koziarski.com>
[#1168 state:committed]
---
 activerecord/lib/active_record/connection_adapters/mysql_adapter.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 3aa27bfc99..1e452ae88a 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -218,7 +218,7 @@ module ActiveRecord
           s = column.class.string_to_binary(value).unpack("H*")[0]
           "x'#{s}'"
         elsif value.kind_of?(BigDecimal)
-          "'#{value.to_s("F")}'"
+          value.to_s("F")
         else
           super
         end
-- 
cgit v1.2.3


From 21eb18a70c7a1f08e7e2dc1c5bc17d67e1d14c46 Mon Sep 17 00:00:00 2001
From: Aliaksey Kandratsenka <alk@tut.by>
Date: Thu, 2 Oct 2008 21:44:49 +0300
Subject: Fix race in ConnectionPool#checkout

After releasing monitor some connection(s) may appear in pool before monitor is re-aquired.
When this happens we'll wait for connection which is already available.

Signed-off-by: Michael Koziarski <michael@koziarski.com>
---
 .../abstract/connection_pool.rb                    | 29 +++++++++++-----------
 1 file changed, 14 insertions(+), 15 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 54a39db1eb..74381437ca 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -131,21 +131,20 @@ module ActiveRecord
       # Check-out a database connection from the pool.
       def checkout
         # Checkout an available connection
-        conn = @connection_mutex.synchronize do
-          if @checked_out.size < @connections.size
-            checkout_existing_connection
-          elsif @connections.size < @size
-            checkout_new_connection
-          end
-        end
-        return conn if conn
-
-        # No connections available; wait for one
         @connection_mutex.synchronize do
-          if @queue.wait(@timeout)
-            checkout_existing_connection
-          else
-            raise ConnectionTimeoutError, "could not obtain a database connection within #{@timeout} seconds.  The pool size is currently #{@size}, perhaps you need to increase it?"
+          loop do
+            conn = if @checked_out.size < @connections.size
+                     checkout_existing_connection
+                   elsif @connections.size < @size
+                     checkout_new_connection
+                   end
+            return conn if conn
+            # No connections available; wait for one
+            if @queue.wait(@timeout)
+              next
+            else
+              raise ConnectionTimeoutError, "could not obtain a database connection within #{@timeout} seconds.  The pool size is currently #{@size}, perhaps you need to increase it?"
+            end
           end
         end
       end
@@ -275,4 +274,4 @@ module ActiveRecord
       end
     end
   end
-end
\ No newline at end of file
+end
-- 
cgit v1.2.3


From 834361145a805b5077f962dc2e67a0a9ea882535 Mon Sep 17 00:00:00 2001
From: Aliaksey Kandratsenka <alk@tut.by>
Date: Thu, 2 Oct 2008 22:59:49 +0300
Subject: made ConnectionPool#checkout more robust by trying to loot dead
 threads when pool is empty

Signed-off-by: Michael Koziarski <michael@koziarski.com>

[#1169 state:committed]
---
 .../active_record/connection_adapters/abstract/connection_pool.rb   | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 74381437ca..b7d7384548 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -143,7 +143,11 @@ module ActiveRecord
             if @queue.wait(@timeout)
               next
             else
-              raise ConnectionTimeoutError, "could not obtain a database connection within #{@timeout} seconds.  The pool size is currently #{@size}, perhaps you need to increase it?"
+              # try looting dead threads
+              clear_stale_cached_connections!
+              if @size == @checked_out.size
+                raise ConnectionTimeoutError, "could not obtain a database connection within #{@timeout} seconds.  The pool size is currently #{@size}, perhaps you need to increase it?"
+              end
             end
           end
         end
-- 
cgit v1.2.3


From f550c8625739106e957e4260d3d592a9665fc943 Mon Sep 17 00:00:00 2001
From: Aliaksey Kandratsenka <alk@tut.by>
Date: Fri, 3 Oct 2008 13:40:51 +0300
Subject: Fix performance bug in AttibuteMethods#respond_to? in handling of
 private methods

We have hit dramatic increase in tests time after upgrading rails.
Profiling revealed this particular place. After this fix our test times returned
back to norm.

Signed-off-by: Michael Koziarski <michael@koziarski.com>
[#1173 state:committed]
---
 activerecord/lib/active_record/attribute_methods.rb | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb
index e5486738f0..1c753524de 100644
--- a/activerecord/lib/active_record/attribute_methods.rb
+++ b/activerecord/lib/active_record/attribute_methods.rb
@@ -342,7 +342,9 @@ module ActiveRecord
       method_name = method.to_s
       if super
         return true
-      elsif self.private_methods.include?(method_name) && !include_private_methods
+      elsif !include_private_methods && super(method, true)
+        # If we're here than we haven't found among non-private methods
+        # but found among all methods. Which means that given method is private.
         return false
       elsif !self.class.generated_methods?
         self.class.define_attribute_methods
-- 
cgit v1.2.3


From 7659fb6a2b638703a99a63033d947d19089a6b85 Mon Sep 17 00:00:00 2001
From: Lawrence Pit <lawrence.pit@gmail.com>
Date: Sat, 4 Oct 2008 15:31:04 +0100
Subject: Try reloading model on class mismatch [#229 state:resolved]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>
---
 .../active_record/associations/association_proxy.rb  |  2 +-
 activerecord/test/cases/reload_models_test.rb        | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)
 create mode 100644 activerecord/test/cases/reload_models_test.rb

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb
index acdcd14ec8..b617147f67 100644
--- a/activerecord/lib/active_record/associations/association_proxy.rb
+++ b/activerecord/lib/active_record/associations/association_proxy.rb
@@ -240,7 +240,7 @@ module ActiveRecord
         # the kind of the class of the associated objects. Meant to be used as
         # a sanity check when you are about to assign an associated record.
         def raise_on_type_mismatch(record)
-          unless record.is_a?(@reflection.klass)
+          unless record.is_a?(@reflection.klass) || record.is_a?(@reflection.class_name.constantize)
             message = "#{@reflection.class_name}(##{@reflection.klass.object_id}) expected, got #{record.class}(##{record.class.object_id})"
             raise ActiveRecord::AssociationTypeMismatch, message
           end
diff --git a/activerecord/test/cases/reload_models_test.rb b/activerecord/test/cases/reload_models_test.rb
new file mode 100644
index 0000000000..411b5f6afa
--- /dev/null
+++ b/activerecord/test/cases/reload_models_test.rb
@@ -0,0 +1,20 @@
+require "cases/helper"
+require 'models/owner'
+require 'models/pet'
+
+class ReloadModelsTest < ActiveRecord::TestCase
+  def test_has_one_with_reload
+    pet = Pet.find_by_name('parrot')
+    pet.owner = Owner.find_by_name('ashley')
+
+    # Reload the class Owner, simulating auto-reloading of model classes in a
+    # development environment. Note that meanwhile the class Pet is not
+    # reloaded, simulating a class that is present in a plugin.
+    Object.class_eval { remove_const :Owner }
+    Kernel.load(File.expand_path(File.join(File.dirname(__FILE__), "../models/owner.rb")))
+
+    pet = Pet.find_by_name('parrot')
+    pet.owner = Owner.find_by_name('ashley')
+    assert_equal pet.owner, Owner.find_by_name('ashley')
+  end
+end
\ No newline at end of file
-- 
cgit v1.2.3


From 95e1cf4812d4b964d7ab0fdf4bfa31177d27909c Mon Sep 17 00:00:00 2001
From: Zach Dennis <zach.dennis@gmail.com>
Date: Sat, 4 Oct 2008 15:42:36 +0100
Subject: Fix has_many :through when the source is a belongs_to association.
 [#323 state:resolved]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>
---
 .../lib/active_record/association_preload.rb       | 44 +++++++++++++++++-----
 .../associations/has_many_through_association.rb   | 11 ++++++
 activerecord/test/cases/associations/eager_test.rb |  9 +++++
 .../associations/has_many_associations_test.rb     |  1 +
 .../has_many_through_associations_test.rb          | 17 ++++++++-
 activerecord/test/models/post.rb                   |  2 +
 6 files changed, 73 insertions(+), 11 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb
index 284dc7dca0..0f96ee95d5 100644
--- a/activerecord/lib/active_record/association_preload.rb
+++ b/activerecord/lib/active_record/association_preload.rb
@@ -4,6 +4,28 @@ module ActiveRecord
       base.extend(ClassMethods)
     end
 
+    class HasManyAssociationStrategy
+      def initialize(through_reflection)
+        @through_reflection = through_reflection
+      end
+
+      def primary_key
+        if @through_reflection && @through_reflection.macro == :belongs_to
+          @through_reflection.klass.primary_key
+        else
+          @through_reflection.primary_key_name
+        end
+      end
+
+      def primary_key_name
+        if @through_reflection && @through_reflection.macro == :belongs_to
+          @through_reflection.primary_key_name
+        else
+          nil
+        end
+      end
+    end
+
     module ClassMethods
 
       # Loads the named associations for the activerecord record (or records) given
@@ -77,12 +99,13 @@ module ActiveRecord
         end
       end
 
-      def construct_id_map(records)
+      def construct_id_map(records, primary_key=nil)
         id_to_record_map = {}
         ids = []
         records.each do |record|
-          ids << record.id
-          mapped_records = (id_to_record_map[record.id.to_s] ||= [])
+          primary_key ||= record.class.primary_key
+          ids << record[primary_key]
+          mapped_records = (id_to_record_map[ids.last.to_s] ||= [])
           mapped_records << record
         end
         ids.uniq!
@@ -129,23 +152,24 @@ module ActiveRecord
       end
 
       def preload_has_many_association(records, reflection, preload_options={})
-        id_to_record_map, ids = construct_id_map(records)
-        records.each {|record| record.send(reflection.name).loaded}
         options = reflection.options
+        through_reflection = reflections[options[:through]]
+        strat = HasManyAssociationStrategy.new(through_reflection)
+        id_to_record_map, ids = construct_id_map(records, strat.primary_key_name)
+        records.each {|record| record.send(reflection.name).loaded}
 
         if options[:through]
           through_records = preload_through_records(records, reflection, options[:through])
           through_reflection = reflections[options[:through]]
-          through_primary_key = through_reflection.primary_key_name
           unless through_records.empty?
             source = reflection.source_reflection.name
-            #add conditions from reflection!
-            through_records.first.class.preload_associations(through_records, source, reflection.options)
+            through_records.first.class.preload_associations(through_records, source, options)
             through_records.each do |through_record|
-              add_preloaded_records_to_collection(id_to_record_map[through_record[through_primary_key].to_s],
-                                                 reflection.name, through_record.send(source))
+              through_record_id = through_record[strat.primary_key].to_s
+              add_preloaded_records_to_collection(id_to_record_map[through_record_id], reflection.name, through_record.send(source))
             end
           end
+
         else
           set_association_collection_records(id_to_record_map, reflection.name, find_associated_records(ids, reflection, preload_options),
                                              reflection.primary_key_name)
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 3171665e19..a0bb3a45b0 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -32,6 +32,14 @@ module ActiveRecord
       end
       
       protected
+        def target_reflection_has_associated_record?
+          if @reflection.through_reflection.macro == :belongs_to && @owner[@reflection.through_reflection.primary_key_name].blank?
+            false
+          else
+            true
+          end
+        end
+
         def construct_find_options!(options)
           options[:select]  = construct_select(options[:select])
           options[:from]  ||= construct_from
@@ -61,6 +69,7 @@ module ActiveRecord
         end
 
         def find_target
+          return [] unless target_reflection_has_associated_record?
           @reflection.klass.find(:all,
             :select     => construct_select,
             :conditions => construct_conditions,
@@ -102,6 +111,8 @@ module ActiveRecord
               "#{as}_type" => reflection.klass.quote_value(
                 @owner.class.base_class.name.to_s,
                 reflection.klass.columns_hash["#{as}_type"]) }
+          elsif reflection.macro == :belongs_to
+            { reflection.klass.primary_key => @owner[reflection.primary_key_name] }
           else
             { reflection.primary_key_name => owner_quoted_id }
           end
diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb
index 3dd2cb028a..7f42577ab0 100644
--- a/activerecord/test/cases/associations/eager_test.rb
+++ b/activerecord/test/cases/associations/eager_test.rb
@@ -275,6 +275,15 @@ class EagerAssociationTest < ActiveRecord::TestCase
     assert_equal authors(:david), assert_no_queries { posts_with_comments_and_author.first.author }
   end
 
+  def test_eager_with_has_many_through_a_belongs_to_association
+    author = authors(:mary)
+    post = Post.create!(:author => author, :title => "TITLE", :body => "BODY")
+    author.author_favorites.create(:favorite_author_id => 1)
+    author.author_favorites.create(:favorite_author_id => 2)
+    posts_with_author_favorites = author.posts.find(:all, :include => :author_favorites)
+    assert_no_queries { posts_with_author_favorites.first.author_favorites.first.author_id }
+  end
+
   def test_eager_with_has_many_through_an_sti_join_model
     author = Author.find(:first, :include => :special_post_comments, :order => 'authors.id')
     assert_equal [comments(:does_it_hurt)], assert_no_queries { author.special_post_comments }
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 315d77de07..1bc9c39c19 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -1081,3 +1081,4 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   end
 
 end
+
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index 12cce98c26..a07f4bcbdd 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -5,7 +5,7 @@ require 'models/reader'
 require 'models/comment'
 
 class HasManyThroughAssociationsTest < ActiveRecord::TestCase
-  fixtures :posts, :readers, :people, :comments
+  fixtures :posts, :readers, :people, :comments, :authors
 
   def test_associate_existing
     assert_queries(2) { posts(:thinking);people(:david) }
@@ -229,4 +229,19 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
       end
     end
   end
+
+  def test_has_many_association_through_a_belongs_to_association_where_the_association_doesnt_exist
+    author = authors(:mary)
+    post = Post.create!(:title => "TITLE", :body => "BODY")
+    assert_equal [], post.author_favorites
+  end
+
+  def test_has_many_association_through_a_belongs_to_association
+    author = authors(:mary)
+    post = Post.create!(:author => author, :title => "TITLE", :body => "BODY")
+    author.author_favorites.create(:favorite_author_id => 1)
+    author.author_favorites.create(:favorite_author_id => 2)
+    author.author_favorites.create(:favorite_author_id => 3)
+    assert_equal post.author.author_favorites, post.author_favorites
+  end
 end
diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb
index 3adbc0ce1f..6da37c31ff 100644
--- a/activerecord/test/models/post.rb
+++ b/activerecord/test/models/post.rb
@@ -22,6 +22,8 @@ class Post < ActiveRecord::Base
     end
   end
 
+  has_many :author_favorites, :through => :author
+
   has_many :comments_with_interpolated_conditions, :class_name => 'Comment',
       :conditions => ['#{"#{aliased_table_name}." rescue ""}body = ?', 'Thank you for the welcome']
 
-- 
cgit v1.2.3


From 25ca21ae21d49f06708357a5ce0670103ced2d58 Mon Sep 17 00:00:00 2001
From: Pratik Naik <pratiknaik@gmail.com>
Date: Sat, 4 Oct 2008 16:53:13 +0100
Subject: Introduce ActiveRecord::Reflection::ThroughReflection to simplify
 hm:t reflection logic

---
 activerecord/lib/active_record/reflection.rb | 119 +++++++++++++++------------
 activerecord/test/cases/reflection_test.rb   |   4 +
 2 files changed, 70 insertions(+), 53 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index a1b498eceb..77d03493dc 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -13,14 +13,15 @@ module ActiveRecord
       def create_reflection(macro, name, options, active_record)
         case macro
           when :has_many, :belongs_to, :has_one, :has_and_belongs_to_many
-            reflection = AssociationReflection.new(macro, name, options, active_record)
+            klass = options[:through] ? ThroughReflection : AssociationReflection
+            reflection = klass.new(macro, name, options, active_record)
           when :composed_of
             reflection = AggregateReflection.new(macro, name, options, active_record)
         end
         write_inheritable_hash :reflections, name => reflection
         reflection
       end
-      
+
       # Returns a hash containing all AssociationReflection objects for the current class
       # Example:
       #
@@ -30,7 +31,7 @@ module ActiveRecord
       def reflections
         read_inheritable_attribute(:reflections) || write_inheritable_attribute(:reflections, {})
       end
-       
+
       # Returns an array of AggregateReflection objects for all the aggregations in the class.
       def reflect_on_all_aggregations
         reflections.values.select { |reflection| reflection.is_a?(AggregateReflection) }
@@ -192,6 +193,49 @@ module ActiveRecord
         end
       end
 
+      def check_validity!
+      end
+
+      def through_reflection
+        false
+      end
+
+      def source_reflection
+        nil
+      end
+
+      private
+        def derive_class_name
+          class_name = name.to_s.camelize
+          class_name = class_name.singularize if [ :has_many, :has_and_belongs_to_many ].include?(macro)
+          class_name
+        end
+
+        def derive_primary_key_name
+          if macro == :belongs_to
+            "#{name}_id"
+          elsif options[:as]
+            "#{options[:as]}_id"
+          else
+            active_record.name.foreign_key
+          end
+        end
+    end
+
+    # Holds all the meta-data about a :through association as it was specified in the Active Record class.
+    class ThroughReflection < AssociationReflection #:nodoc:
+      # Gets the source of the through reflection.  It checks both a singularized and pluralized form for <tt>:belongs_to</tt> or <tt>:has_many</tt>.
+      # (The <tt>:tags</tt> association on Tagging below.)
+      #
+      #   class Post < ActiveRecord::Base
+      #     has_many :taggings
+      #     has_many :tags, :through => :taggings
+      #   end
+      #
+      def source_reflection
+        @source_reflection ||= source_reflection_names.collect { |name| through_reflection.klass.reflect_on_association(name) }.compact.first
+      end
+
       # Returns the AssociationReflection object specified in the <tt>:through</tt> option
       # of a HasManyThrough or HasOneThrough association. Example:
       #
@@ -204,7 +248,7 @@ module ActiveRecord
       #   taggings_reflection = tags_reflection.through_reflection
       #
       def through_reflection
-        @through_reflection ||= options[:through] ? active_record.reflect_on_association(options[:through]) : false
+        @through_reflection ||= active_record.reflect_on_association(options[:through])
       end
 
       # Gets an array of possible <tt>:through</tt> source reflection names:
@@ -215,63 +259,32 @@ module ActiveRecord
         @source_reflection_names ||= (options[:source] ? [options[:source]] : [name.to_s.singularize, name]).collect { |n| n.to_sym }
       end
 
-      # Gets the source of the through reflection.  It checks both a singularized and pluralized form for <tt>:belongs_to</tt> or <tt>:has_many</tt>.
-      # (The <tt>:tags</tt> association on Tagging below.)
-      # 
-      #   class Post < ActiveRecord::Base
-      #     has_many :taggings
-      #     has_many :tags, :through => :taggings
-      #   end
-      #
-      def source_reflection
-        return nil unless through_reflection
-        @source_reflection ||= source_reflection_names.collect { |name| through_reflection.klass.reflect_on_association(name) }.compact.first
-      end
-
       def check_validity!
-        if options[:through]
-          if through_reflection.nil?
-            raise HasManyThroughAssociationNotFoundError.new(active_record.name, self)
-          end
-          
-          if source_reflection.nil?
-            raise HasManyThroughSourceAssociationNotFoundError.new(self)
-          end
+        if through_reflection.nil?
+          raise HasManyThroughAssociationNotFoundError.new(active_record.name, self)
+        end
 
-          if options[:source_type] && source_reflection.options[:polymorphic].nil?
-            raise HasManyThroughAssociationPointlessSourceTypeError.new(active_record.name, self, source_reflection)
-          end
-          
-          if source_reflection.options[:polymorphic] && options[:source_type].nil?
-            raise HasManyThroughAssociationPolymorphicError.new(active_record.name, self, source_reflection)
-          end
-          
-          unless [:belongs_to, :has_many].include?(source_reflection.macro) && source_reflection.options[:through].nil?
-            raise HasManyThroughSourceAssociationMacroError.new(self)
-          end
+        if source_reflection.nil?
+          raise HasManyThroughSourceAssociationNotFoundError.new(self)
+        end
+
+        if options[:source_type] && source_reflection.options[:polymorphic].nil?
+          raise HasManyThroughAssociationPointlessSourceTypeError.new(active_record.name, self, source_reflection)
+        end
+
+        if source_reflection.options[:polymorphic] && options[:source_type].nil?
+          raise HasManyThroughAssociationPolymorphicError.new(active_record.name, self, source_reflection)
+        end
+
+        unless [:belongs_to, :has_many].include?(source_reflection.macro) && source_reflection.options[:through].nil?
+          raise HasManyThroughSourceAssociationMacroError.new(self)
         end
       end
 
       private
         def derive_class_name
           # get the class_name of the belongs_to association of the through reflection
-          if through_reflection
-            options[:source_type] || source_reflection.class_name
-          else
-            class_name = name.to_s.camelize
-            class_name = class_name.singularize if [ :has_many, :has_and_belongs_to_many ].include?(macro)
-            class_name
-          end
-        end
-
-        def derive_primary_key_name
-          if macro == :belongs_to
-            "#{name}_id"
-          elsif options[:as]
-            "#{options[:as]}_id"
-          else
-            active_record.name.foreign_key
-          end
+          options[:source_type] || source_reflection.class_name
         end
     end
   end
diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb
index e339ef41ab..e0ed3e5886 100644
--- a/activerecord/test/cases/reflection_test.rb
+++ b/activerecord/test/cases/reflection_test.rb
@@ -170,6 +170,10 @@ class ReflectionTest < ActiveRecord::TestCase
     assert_nothing_raised { Firm.reflections[:clients] == Object.new }
   end
 
+  def test_has_many_through_reflection
+    assert_kind_of ActiveRecord::Reflection::ThroughReflection, Subscriber.reflect_on_association(:books)
+  end
+
   private
     def assert_reflection(klass, association, options)
       assert reflection = klass.reflect_on_association(association)
-- 
cgit v1.2.3


From 4918e6de989a80bb2ae92183f1b4eb98c15b487f Mon Sep 17 00:00:00 2001
From: Pratik Naik <pratiknaik@gmail.com>
Date: Sat, 4 Oct 2008 17:41:27 +0100
Subject: Remove HasManyAssociationStrategy and move the logic to
 ActiveRecord::Reflection::ThroughReflection.

---
 .../lib/active_record/association_preload.rb       | 30 +++-------------------
 activerecord/lib/active_record/reflection.rb       | 18 ++++++++++++-
 2 files changed, 21 insertions(+), 27 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb
index 0f96ee95d5..c38b98258a 100644
--- a/activerecord/lib/active_record/association_preload.rb
+++ b/activerecord/lib/active_record/association_preload.rb
@@ -4,28 +4,6 @@ module ActiveRecord
       base.extend(ClassMethods)
     end
 
-    class HasManyAssociationStrategy
-      def initialize(through_reflection)
-        @through_reflection = through_reflection
-      end
-
-      def primary_key
-        if @through_reflection && @through_reflection.macro == :belongs_to
-          @through_reflection.klass.primary_key
-        else
-          @through_reflection.primary_key_name
-        end
-      end
-
-      def primary_key_name
-        if @through_reflection && @through_reflection.macro == :belongs_to
-          @through_reflection.primary_key_name
-        else
-          nil
-        end
-      end
-    end
-
     module ClassMethods
 
       # Loads the named associations for the activerecord record (or records) given
@@ -153,9 +131,9 @@ module ActiveRecord
 
       def preload_has_many_association(records, reflection, preload_options={})
         options = reflection.options
-        through_reflection = reflections[options[:through]]
-        strat = HasManyAssociationStrategy.new(through_reflection)
-        id_to_record_map, ids = construct_id_map(records, strat.primary_key_name)
+
+        primary_key_name = reflection.through_reflection_primary_key_name
+        id_to_record_map, ids = construct_id_map(records, primary_key_name)
         records.each {|record| record.send(reflection.name).loaded}
 
         if options[:through]
@@ -165,7 +143,7 @@ module ActiveRecord
             source = reflection.source_reflection.name
             through_records.first.class.preload_associations(through_records, source, options)
             through_records.each do |through_record|
-              through_record_id = through_record[strat.primary_key].to_s
+              through_record_id = through_record[reflection.through_reflection_primary_key].to_s
               add_preloaded_records_to_collection(id_to_record_map[through_record_id], reflection.name, through_record.send(source))
             end
           end
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index 77d03493dc..dbff4f24d6 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -117,6 +117,11 @@ module ActiveRecord
         @sanitized_conditions ||= klass.send(:sanitize_sql, options[:conditions]) if options[:conditions]
       end
 
+      # Returns +true+ if +self+ is a +belongs_to+ reflection.
+      def belongs_to?
+        macro == :belongs_to
+      end
+
       private
         def derive_class_name
           name.to_s.camelize
@@ -200,6 +205,9 @@ module ActiveRecord
         false
       end
 
+      def through_reflection_primary_key_name
+      end
+
       def source_reflection
         nil
       end
@@ -212,7 +220,7 @@ module ActiveRecord
         end
 
         def derive_primary_key_name
-          if macro == :belongs_to
+          if belongs_to?
             "#{name}_id"
           elsif options[:as]
             "#{options[:as]}_id"
@@ -281,6 +289,14 @@ module ActiveRecord
         end
       end
 
+      def through_reflection_primary_key
+        through_reflection.belongs_to? ? through_reflection.klass.primary_key : through_reflection.primary_key_name
+      end
+
+      def through_reflection_primary_key_name
+        through_reflection.primary_key_name if through_reflection.belongs_to?
+      end
+
       private
         def derive_class_name
           # get the class_name of the belongs_to association of the through reflection
-- 
cgit v1.2.3


From 9599948fbcd67c1c2c5fecc2dca148e998479e33 Mon Sep 17 00:00:00 2001
From: Pratik Naik <pratiknaik@gmail.com>
Date: Sat, 4 Oct 2008 20:03:42 +0100
Subject: Ensure Model.sum and Model.avg typecast appropriately. [#1066
 state:resolved]

Model.sum delegates typecasting to the column being summed. If that's not feasible, returns a string.
Model.avg always returns big decimal.
---
 activerecord/lib/active_record/calculations.rb | 10 +++++++---
 activerecord/test/cases/calculations_test.rb   |  6 +++---
 2 files changed, 10 insertions(+), 6 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb
index 634236e959..5e33cf1bd4 100644
--- a/activerecord/lib/active_record/calculations.rb
+++ b/activerecord/lib/active_record/calculations.rb
@@ -285,11 +285,15 @@ module ActiveRecord
           operation = operation.to_s.downcase
           case operation
             when 'count' then value.to_i
-            when 'sum'   then value =~ /\./ ? value.to_f : value.to_i
-            when 'avg'   then value && value.to_f
-            else column ? column.type_cast(value) : value
+            when 'sum'   then type_cast_using_column(value || '0', column)
+            when 'avg'   then value && value.to_d
+            else type_cast_using_column(value, column)
           end
         end
+
+        def type_cast_using_column(value, column)
+          column ? column.type_cast(value) : value
+        end
     end
   end
 end
diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb
index 754fd58f35..0fa61500c0 100644
--- a/activerecord/test/cases/calculations_test.rb
+++ b/activerecord/test/cases/calculations_test.rb
@@ -18,8 +18,8 @@ class CalculationsTest < ActiveRecord::TestCase
 
   def test_should_average_field
     value = Account.average(:credit_limit)
-    assert_kind_of Float, value
-    assert_in_delta 53.0, value, 0.001
+    assert_kind_of BigDecimal, value
+    assert_equal BigDecimal.new('53.0'), value
   end
 
   def test_should_return_nil_as_average
@@ -273,7 +273,7 @@ class CalculationsTest < ActiveRecord::TestCase
   end
 
   def test_should_sum_expression
-    assert_equal 636, Account.sum("2 * credit_limit")
+    assert_equal '636', Account.sum("2 * credit_limit")
   end
 
   def test_count_with_from_option
-- 
cgit v1.2.3