Merge pull request #28526 from kamipo/fix_log_subscriber_to_allow_legacy_binds

Fix `LogSubscriber` to allow legacy `binds`

activerecord/lib/active_record/connection_adapters/abstract/quoting.rb

@@ -152,16 +152,15 @@ def quoted_binary(value) # :nodoc:
         "'#{quote_string(value.to_s)}'"
       end
 
-      private
-
-        def type_casted_binds(binds)
-          if binds.first.is_a?(Array)
-            binds.map { |column, value| type_cast(value, column) }
-          else
-            binds.map { |attr| type_cast(attr.value_for_database) }
-          end
+      def type_casted_binds(binds) # :nodoc:
+        if binds.first.is_a?(Array)
+          binds.map { |column, value| type_cast(value, column) }
+        else
+          binds.map { |attr| type_cast(attr.value_for_database) }
         end
+      end
 
+      private
         def id_value_for_database(value)
           if primary_key = value.class.primary_key
             value.instance_variable_get(:@attributes)[primary_key].value_for_database

activerecord/lib/active_record/log_subscriber.rb

@@ -44,17 +44,17 @@ def sql(event)
     private
 
       def type_casted_binds(binds, casted_binds)
-        casted_binds || binds.map { |attr| type_cast attr.value_for_database }
+        casted_binds || ActiveRecord::Base.connection.type_casted_binds(binds)
       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>"
-        else
-          type_casted_value
+      def render_bind(attr, value)
+        if attr.is_a?(Array)
+          attr = attr.first
+        elsif attr.type.binary? && attr.value
+          value = "<#{attr.value_for_database.to_s.bytesize} bytes of binary data>"
         end
 
-        [attr.name, value]
+        [attr && attr.name, value]
       end
 
       def colorize_payload_name(name, payload_name)
@@ -89,10 +89,6 @@ def sql_color(sql)
       def logger
         ActiveRecord::Base.logger
       end
-
-      def type_cast(value)
-        ActiveRecord::Base.connection.type_cast(value)
-      end
   end
 end
 

activerecord/test/cases/bind_parameter_test.rb

@@ -3,36 +3,36 @@
 require "models/author"
 require "models/post"
 
-module ActiveRecord
-  class BindParameterTest < ActiveRecord::TestCase
-    fixtures :topics, :authors, :posts
-
-    class LogListener
-      attr_accessor :calls
-
-      def initialize
-        @calls = []
+if ActiveRecord::Base.connection.supports_statement_cache? &&
+   ActiveRecord::Base.connection.prepared_statements
+  module ActiveRecord
+    class BindParameterTest < ActiveRecord::TestCase
+      fixtures :topics, :authors, :posts
+
+      class LogListener
+        attr_accessor :calls
+
+        def initialize
+          @calls = []
+        end
+
+        def call(*args)
+          calls << args
+        end
       end
 
-      def call(*args)
-        calls << args
+      def setup
+        super
+        @connection = ActiveRecord::Base.connection
+        @subscriber = LogListener.new
+        @pk = Topic.columns_hash[Topic.primary_key]
+        @subscription = ActiveSupport::Notifications.subscribe("sql.active_record", @subscriber)
       end
-    end
 
-    def setup
-      super
-      @connection = ActiveRecord::Base.connection
-      @subscriber = LogListener.new
-      @pk = Topic.columns_hash[Topic.primary_key]
-      @subscription = ActiveSupport::Notifications.subscribe("sql.active_record", @subscriber)
-    end
-
-    teardown do
-      ActiveSupport::Notifications.unsubscribe(@subscription)
-    end
+      def teardown
+        ActiveSupport::Notifications.unsubscribe(@subscription)
+      end
 
-    if ActiveRecord::Base.connection.supports_statement_cache? &&
-       ActiveRecord::Base.connection.prepared_statements
       def test_bind_from_join_in_subquery
         subquery = Author.joins(:thinking_posts).where(name: "David")
         scope = Author.from(subquery, "authors").where(id: 1)
@@ -56,43 +56,48 @@ def test_find_one_uses_binds
         assert message, "expected a message with binds"
       end
 
-      def test_logs_bind_vars_after_type_cast
+      def test_logs_binds_after_type_cast
         binds = [Relation::QueryAttribute.new("id", "10", Type::Integer.new)]
-        type_casted_binds = binds.map { |attr| type_cast(attr.value_for_database) }
-        payload = {
-          name: "SQL",
-          sql: "select * from topics where id = ?",
-          binds: binds,
-          type_casted_binds: type_casted_binds
-        }
-        event  = ActiveSupport::Notifications::Event.new(
-          "foo",
-          Time.now,
-          Time.now,
-          123,
-          payload)
-
-        logger = Class.new(ActiveRecord::LogSubscriber) {
-          attr_reader :debugs
-          def initialize
-            super
-            @debugs = []
-          end
-
-          def debug(str)
-            @debugs << str
-          end
-        }.new
-
-        logger.sql event
-        assert_match([[@pk.name, 10]].inspect, logger.debugs.first)
+        assert_logs_binds(binds)
       end
 
-      private
-
-      def type_cast(value)
-        ActiveRecord::Base.connection.type_cast(value)
+      def test_logs_legacy_binds_after_type_cast
+        binds = [[@pk, "10"]]
+        assert_logs_binds(binds)
       end
+
+      private
+        def assert_logs_binds(binds)
+          payload = {
+            name: "SQL",
+            sql: "select * from topics where id = ?",
+            binds: binds,
+            type_casted_binds: @connection.type_casted_binds(binds)
+          }
+
+          event = ActiveSupport::Notifications::Event.new(
+            "foo",
+            Time.now,
+            Time.now,
+            123,
+            payload)
+
+          logger = Class.new(ActiveRecord::LogSubscriber) {
+            attr_reader :debugs
+
+            def initialize
+              super
+              @debugs = []
+            end
+
+            def debug(str)
+              @debugs << str
+            end
+          }.new
+
+          logger.sql(event)
+          assert_match([[@pk.name, 10]].inspect, logger.debugs.first)
+        end
     end
   end
 end