Merge pull request #34303 from kamipo/lazy_checking_boundable

Lazy checking whether or not values in IN clause are boundable
rails · Oct 24, 2018 · d3e6465 · d3e6465
2 parents eba2ba8 + ce40073
commit d3e6465
Show file tree

Hide file tree

Showing 6 changed files with 18 additions and 9 deletions.
diff --git a/activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb b/activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb
@@ -10,8 +10,17 @@ def accept(*)
         super
       end
 
-      def visit_Arel_Nodes_In(*)
+      def visit_Arel_Nodes_In(o, collector)
         @preparable = false
+
+        if Array === o.right && !o.right.empty?
+          o.right.delete_if do |bind|
+            if Arel::Nodes::BindParam === bind && Relation::QueryAttribute === bind.value
+              !bind.value.boundable?
+            end
+          end
+        end
+
         super
       end
 

diff --git a/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb
@@ -22,9 +22,8 @@ def call(attribute, value)
           when 1 then predicate_builder.build(attribute, values.first)
           else
             values.map! do |v|
-              bind = predicate_builder.build_bind_attribute(attribute.name, v)
-              bind if bind.value.boundable?
-            end.compact!
+              predicate_builder.build_bind_attribute(attribute.name, v)
+            end
             values.empty? ? NullPredicate : attribute.in(values)
           end
 

diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb
@@ -34,7 +34,7 @@ class EagerLoadingTooManyIdsTest < ActiveRecord::TestCase
   fixtures :citations
 
   def test_preloading_too_many_ids
-    assert_equal Citation.count, Citation.preload(:citations).to_a.size
+    assert_equal Citation.count, Citation.preload(:reference_of).to_a.size
   end
 
   def test_eager_loading_too_may_ids

diff --git a/activerecord/test/cases/bind_parameter_test.rb b/activerecord/test/cases/bind_parameter_test.rb
@@ -36,7 +36,7 @@ def teardown
 
       def test_too_many_binds
         bind_params_length = @connection.send(:bind_params_length)
-        topics = Topic.where(id: (1 .. bind_params_length + 1).to_a)
+        topics = Topic.where(id: (1 .. bind_params_length).to_a << 2**63)
         assert_equal Topic.count, topics.count
       end
 

diff --git a/activerecord/test/fixtures/citations.yml b/activerecord/test/fixtures/citations.yml
@@ -1,4 +1,5 @@
 <% 65536.times do |i| %>
 fixture_no_<%= i %>:
   id: <%= i %>
+  book2_id: <%= i*i %>
 <% end %>
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
@@ -93,7 +93,7 @@
     t.integer :pirate_id
   end
 
-  create_table :books, force: true do |t|
+  create_table :books, id: :integer, force: true do |t|
     t.references :author
     t.string :format
     t.column :name, :string
@@ -158,8 +158,8 @@
   end
 
   create_table :citations, force: true do |t|
-    t.column :book1_id, :integer
-    t.column :book2_id, :integer
+    t.references :book1
+    t.references :book2
     t.references :citation
   end