mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
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.
This commit is contained in:
parent
dc67615920
commit
ce40073c9c
6 changed files with 18 additions and 9 deletions
|
@ -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
|
||||
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
1
activerecord/test/fixtures/citations.yml
vendored
1
activerecord/test/fixtures/citations.yml
vendored
|
@ -1,4 +1,5 @@
|
|||
<% 65536.times do |i| %>
|
||||
fixture_no_<%= i %>:
|
||||
id: <%= i %>
|
||||
book2_id: <%= i*i %>
|
||||
<% end %>
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
Loading…
Reference in a new issue