Fix explain logging with binds

`binds` is an array of a query attribute since Active Record 5.0.

4 files changed, 28 insertions, 17 deletions

diff --git a/activerecord/lib/active_record/explain.rb b/activerecord/lib/active_record/explain.rb
index 727a9befc1..ac27e72f2b 100644
--- a/activerecord/lib/active_record/explain.rb
+++ b/activerecord/lib/active_record/explain.rb
@@ -16,15 +16,14 @@ module ActiveRecord
     # Makes the adapter execute EXPLAIN for the tuples of queries and bindings.
     # Returns a formatted string ready to be logged.
     def exec_explain(queries) # :nodoc:
-      str = queries.map do |sql, bind|
-        [].tap do |msg|
-          msg << "EXPLAIN for: #{sql}"
-          unless bind.empty?
-            bind_msg = bind.map {|col, val| [col.name, val]}.inspect
-            msg.last << " #{bind_msg}"
-          end
-          msg << connection.explain(sql, bind)
-        end.join("\n")
+      str = queries.map do |sql, binds|
+        msg = "EXPLAIN for: #{sql}"
+        unless binds.empty?
+          msg << " "
+          msg << binds.map { |attr| render_bind(attr) }.inspect
+        end
+        msg << "\n"
+        msg << connection.explain(sql, binds)
       end.join("\n")
 
       # Overriding inspect to be more human readable, especially in the console.
@@ -34,5 +33,17 @@ module ActiveRecord
 
       str
     end
+
+    private
+
+      def render_bind(attr)
+        value = if attr.type.binary? && attr.value
+          "<#{attr.value_for_database.to_s.bytesize} bytes of binary data>"
+        else
+          connection.type_cast(attr.value_for_database)
+        end
+
+        [attr.name, value]
+      end
   end
 end
diff --git a/activerecord/test/cases/adapters/postgresql/explain_test.rb b/activerecord/test/cases/adapters/postgresql/explain_test.rb
index 29bf2c15ea..b00f8f5706 100644
--- a/activerecord/test/cases/adapters/postgresql/explain_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/explain_test.rb
@@ -7,14 +7,14 @@ class PostgreSQLExplainTest < ActiveRecord::PostgreSQLTestCase
 
   def test_explain_for_one_query
     explain = Developer.where(id: 1).explain
-    assert_match %r(EXPLAIN for: SELECT "developers".* FROM "developers" WHERE "developers"."id" = \$?1), explain
+    assert_match %r(EXPLAIN for: SELECT "developers".* FROM "developers" WHERE "developers"."id" = (?:\$1 \[\["id", 1\]\]|1)), explain
     assert_match %(QUERY PLAN), explain
   end
 
   def test_explain_with_eager_loading
     explain = Developer.where(id: 1).includes(:audit_logs).explain
     assert_match %(QUERY PLAN), explain
-    assert_match %r(EXPLAIN for: SELECT "developers".* FROM "developers" WHERE "developers"."id" = \$?1), explain
+    assert_match %r(EXPLAIN for: SELECT "developers".* FROM "developers" WHERE "developers"."id" = (?:\$1 \[\["id", 1\]\]|1)), explain
     assert_match %(EXPLAIN for: SELECT "audit_logs".* FROM "audit_logs" WHERE "audit_logs"."developer_id" = 1), explain
   end
 end
diff --git a/activerecord/test/cases/adapters/sqlite3/explain_test.rb b/activerecord/test/cases/adapters/sqlite3/explain_test.rb
index a1a6e5f16a..4d25bd615d 100644
--- a/activerecord/test/cases/adapters/sqlite3/explain_test.rb
+++ b/activerecord/test/cases/adapters/sqlite3/explain_test.rb
@@ -7,13 +7,13 @@ class SQLite3ExplainTest < ActiveRecord::SQLite3TestCase
 
   def test_explain_for_one_query
     explain = Developer.where(id: 1).explain
-    assert_match %r(EXPLAIN for: SELECT "developers".* FROM "developers" WHERE "developers"."id" = (?:\?|1)), explain
+    assert_match %r(EXPLAIN for: SELECT "developers".* FROM "developers" WHERE "developers"."id" = (?:\? \[\["id", 1\]\]|1)), explain
     assert_match(/(SEARCH )?TABLE developers USING (INTEGER )?PRIMARY KEY/, explain)
   end
 
   def test_explain_with_eager_loading
     explain = Developer.where(id: 1).includes(:audit_logs).explain
-    assert_match %r(EXPLAIN for: SELECT "developers".* FROM "developers" WHERE "developers"."id" = (?:\?|1)), explain
+    assert_match %r(EXPLAIN for: SELECT "developers".* FROM "developers" WHERE "developers"."id" = (?:\? \[\["id", 1\]\]|1)), explain
     assert_match(/(SEARCH )?TABLE developers USING (INTEGER )?PRIMARY KEY/, explain)
     assert_match %(EXPLAIN for: SELECT "audit_logs".* FROM "audit_logs" WHERE "audit_logs"."developer_id" = 1), explain
     assert_match(/(SCAN )?TABLE audit_logs/, explain)
diff --git a/activerecord/test/cases/explain_test.rb b/activerecord/test/cases/explain_test.rb
index 64dfd86ce2..6409290e8d 100644
--- a/activerecord/test/cases/explain_test.rb
+++ b/activerecord/test/cases/explain_test.rb
@@ -46,11 +46,8 @@ if ActiveRecord::Base.connection.supports_explain?
     end
 
     def test_exec_explain_with_binds
-      object = Struct.new(:name)
-      cols = [object.new('wadus'), object.new('chaflan')]
-
       sqls    = %w(foo bar)
-      binds   = [[[cols[0], 1]], [[cols[1], 2]]]
+      binds   = [[bind_param('wadus', 1)], [bind_param('chaflan', 2)]]
       queries = sqls.zip(binds)
 
       stub_explain_for_query_plans(["query plan foo\n", "query plan bar\n"]) do
@@ -83,5 +80,8 @@ if ActiveRecord::Base.connection.supports_explain?
         end
       end
 
+      def bind_param(name, value)
+        ActiveRecord::Relation::QueryAttribute.new(name, value, ActiveRecord::Type::Value.new)
+      end
   end
 end