From 1345511081a7b4562281cf77d9e5aeeae2d3eb02 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Thu, 20 Oct 2016 09:21:16 -0700
Subject: Use old typecasting method if no type casted binds are passed in

Query cache doesn't type cast bind parameters since it isn't
actually querying the database, so it can't pass those values in.  Type
casting in the query cache method would cause the values to be type cast
twice in the case that there is a cache miss (since the methods it calls
will type cast *again*).  If logging is disabled, then adding the type
cast code to the query cache method will needlessly typecast the values
(since the only reason those values are type cast is for display in the
logs).

Fixes #26828.
---
 activerecord/lib/active_record/log_subscriber.rb | 11 +++++++-
 activerecord/test/cases/query_cache_test.rb      | 36 +++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/activerecord/lib/active_record/log_subscriber.rb b/activerecord/lib/active_record/log_subscriber.rb
index ca9143c57d..4b8d8d9105 100644
--- a/activerecord/lib/active_record/log_subscriber.rb
+++ b/activerecord/lib/active_record/log_subscriber.rb
@@ -29,7 +29,8 @@ module ActiveRecord
       binds = nil
 
       unless (payload[:binds] || []).empty?
-        binds = "  " + payload[:binds].zip(payload[:type_casted_binds]).map { |attr, value|
+        casted_params = type_casted_binds(payload[:binds], payload[:type_casted_binds])
+        binds = "  " + payload[:binds].zip(casted_params).map { |attr, value|
           render_bind(attr, value)
         }.inspect
       end
@@ -42,6 +43,10 @@ module ActiveRecord
 
     private
 
+      def type_casted_binds(binds, casted_binds)
+        casted_binds || binds.map { |attr| type_cast attr.value_for_database }
+      end
+
       def render_bind(attr, type_casted_value)
         value = if attr.type.binary? && attr.value
           "<#{attr.value_for_database.to_s.bytesize} bytes of binary data>"
@@ -84,6 +89,10 @@ module ActiveRecord
       def logger
         ActiveRecord::Base.logger
       end
+
+      def type_cast(value)
+        ActiveRecord::Base.connection.type_cast(value)
+      end
   end
 end
 
diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb
index 16cf2bd2d0..4cd258695d 100644
--- a/activerecord/test/cases/query_cache_test.rb
+++ b/activerecord/test/cases/query_cache_test.rb
@@ -10,9 +10,30 @@ class QueryCacheTest < ActiveRecord::TestCase
 
   fixtures :tasks, :topics, :categories, :posts, :categories_posts
 
-  teardown do
+  class ShouldNotHaveExceptionsLogger < ActiveRecord::LogSubscriber
+    attr_reader :logger
+
+    def initialize
+      super
+      @logger = ::Logger.new File::NULL
+      @exception = false
+    end
+
+    def exception?
+      @exception
+    end
+
+    def sql(event)
+      super
+    rescue
+      @exception = true
+    end
+  end
+
+  def teardown
     Task.connection.clear_query_cache
     ActiveRecord::Base.connection.disable_query_cache!
+    super
   end
 
   def test_exceptional_middleware_clears_and_disables_cache_on_error
@@ -121,6 +142,19 @@ class QueryCacheTest < ActiveRecord::TestCase
     end
   end
 
+  def test_cache_does_not_raise_exceptions
+    logger = ShouldNotHaveExceptionsLogger.new
+    subscriber = ActiveSupport::Notifications.subscribe "sql.active_record", logger
+
+    ActiveRecord::Base.cache do
+      assert_queries(1) { Task.find(1); Task.find(1) }
+    end
+
+    assert_not_predicate logger, :exception?
+  ensure
+    ActiveSupport::Notifications.unsubscribe subscriber
+  end
+
   def test_cache_is_flat
     Task.cache do
       assert_queries(1) { Topic.find(1); Topic.find(1); }
-- 
cgit v1.2.3