aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRyuta Kamizono <kamipo@gmail.com>2018-10-24 10:31:41 +0900
committerRyuta Kamizono <kamipo@gmail.com>2018-10-24 11:26:49 +0900
commitce40073c9c321575e6b4f46dd5ac9b796a2637be (patch)
tree0e57b4f8c46ff591122cec58d248fe39125e3bbf
parentdc6761592009e9146552fc9d6299bf58a34e187a (diff)
downloadrails-ce40073c9c321575e6b4f46dd5ac9b796a2637be.tar.gz
rails-ce40073c9c321575e6b4f46dd5ac9b796a2637be.tar.bz2
rails-ce40073c9c321575e6b4f46dd5ac9b796a2637be.zip
Lazy checking whether or not values in IN clause are boundable
Since #33844, eager loading/preloading with too many and/or too large ids won't be broken by pre-checking whether the value is constructable or not. But the pre-checking caused the type to be evaluated at relation build time instead of at the query execution time, that is breaking an expectation for some apps. I've made the pre-cheking lazy as much as possible, that is no longer happend at relation build time.
-rw-r--r--activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb11
-rw-r--r--activerecord/lib/active_record/relation/predicate_builder/array_handler.rb5
-rw-r--r--activerecord/test/cases/associations/eager_test.rb2
-rw-r--r--activerecord/test/cases/bind_parameter_test.rb2
-rw-r--r--activerecord/test/fixtures/citations.yml1
-rw-r--r--activerecord/test/schema/schema.rb6
6 files changed, 18 insertions, 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
index 3dcb916d99..f158946c6d 100644
--- 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 @@ module ActiveRecord
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
index fadb3c420d..ee2ece1560 100644
--- 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 @@ module ActiveRecord
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
index 39034746c9..ed7715c1e2 100644
--- 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
index 9c1f7aaef2..fddc2781b8 100644
--- a/activerecord/test/cases/bind_parameter_test.rb
+++ b/activerecord/test/cases/bind_parameter_test.rb
@@ -36,7 +36,7 @@ if ActiveRecord::Base.connection.prepared_statements
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
index d31cb8efa1..396099621c 100644
--- 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
index 2aaf393009..4ef463fdad 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -93,7 +93,7 @@ ActiveRecord::Schema.define do
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 @@ ActiveRecord::Schema.define do
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