From b571c4f3f2811b5d3dc8b005707cf8c353abdf03 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Fri, 14 Sep 2018 06:57:20 +0900
Subject: [PATCH] Fallback to unprepared statement only when bind params limit
 is exceeded

This is a follow up and/or an alternative of #33844.

Unlike #33844, this would attempt to construct unprepared statement only
when bind params limit (mysql2 65535, pg 65535, sqlite3 249999) is
exceeded.

I only defined 65535 as the limit, not defined 249999 for sqlite3, since
it is an edge case, I'm not excited to add less worth extra code.
---
 .../connection_adapters/abstract/database_limits.rb         | 5 +++++
 .../connection_adapters/abstract/database_statements.rb     | 5 +++++
 .../lib/active_record/relation/predicate_builder.rb         | 6 +-----
 .../relation/predicate_builder/array_handler.rb             | 4 ++--
 activerecord/test/cases/bind_parameter_test.rb              | 6 ++++++
 5 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb
index ad148efcfe..1305216be2 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb
@@ -71,6 +71,11 @@ def joins_per_query
         256
       end
       deprecate :joins_per_query
+
+      private
+        def bind_params_length
+          65535
+        end
     end
   end
 end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
index fdc9ffa688..5b24a467ec 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
@@ -46,11 +46,16 @@ def cacheable_query(klass, arel) # :nodoc:
       def select_all(arel, name = nil, binds = [], preparable: nil)
         arel = arel_from_relation(arel)
         sql, binds = to_sql_and_binds(arel, binds)
+
         if !prepared_statements || (arel.is_a?(String) && preparable.nil?)
           preparable = false
+        elsif binds.length > bind_params_length
+          sql, binds = unprepared_statement { to_sql_and_binds(arel) }
+          preparable = false
         else
           preparable = visitor.preparable
         end
+
         if prepared_statements && preparable
           select_prepared(sql, name, binds)
         else
diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb
index cb70b8bcde..f734cd0ad8 100644
--- a/activerecord/lib/active_record/relation/predicate_builder.rb
+++ b/activerecord/lib/active_record/relation/predicate_builder.rb
@@ -57,14 +57,10 @@ def build(attribute, value)
     end
 
     def build_bind_attribute(column_name, value)
-      attr = build_query_attribute(column_name, value)
+      attr = Relation::QueryAttribute.new(column_name.to_s, value, table.type(column_name))
       Arel::Nodes::BindParam.new(attr)
     end
 
-    def build_query_attribute(column_name, value)
-      Relation::QueryAttribute.new(column_name.to_s, value, table.type(column_name))
-    end
-
     protected
       def expand_from_hash(attributes)
         return ["1=0"] if attributes.empty?
diff --git a/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb
index 0895b9fba6..fadb3c420d 100644
--- a/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb
+++ b/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb
@@ -22,8 +22,8 @@ def call(attribute, value)
           when 1 then predicate_builder.build(attribute, values.first)
           else
             values.map! do |v|
-              attr = predicate_builder.build_query_attribute(attribute.name, v)
-              attr.value_for_database if attr.boundable?
+              bind = predicate_builder.build_bind_attribute(attribute.name, v)
+              bind if bind.value.boundable?
             end.compact!
             values.empty? ? NullPredicate : attribute.in(values)
           end
diff --git a/activerecord/test/cases/bind_parameter_test.rb b/activerecord/test/cases/bind_parameter_test.rb
index 91cc49385c..9c1f7aaef2 100644
--- a/activerecord/test/cases/bind_parameter_test.rb
+++ b/activerecord/test/cases/bind_parameter_test.rb
@@ -34,6 +34,12 @@ def teardown
         ActiveSupport::Notifications.unsubscribe(@subscription)
       end
 
+      def test_too_many_binds
+        bind_params_length = @connection.send(:bind_params_length)
+        topics = Topic.where(id: (1 .. bind_params_length + 1).to_a)
+        assert_equal Topic.count, topics.count
+      end
+
       def test_bind_from_join_in_subquery
         subquery = Author.joins(:thinking_posts).where(name: "David")
         scope = Author.from(subquery, "authors").where(id: 1)
-- 
GitLab