Skip to content

Commit

Permalink
Revert "Merge pull request rails#39022 from kamipo/perf_where_in"
Browse files Browse the repository at this point in the history
This reverts commit 9817d74, reversing
changes made to d326b02.

Just making this easier to merge our PR in. Otherwise there's tons of
conflicts and our PR is faster.
  • Loading branch information
eileencodes committed May 5, 2020
1 parent 9aa197d commit 200058b
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,20 @@ def call(attribute, value)
case values.length
when 0 then NullPredicate
when 1 then predicate_builder.build(attribute, values.first)
else attribute.in(values)
else
values.map! do |v|
predicate_builder.build_bind_attribute(attribute.name, v)
end
values.empty? ? NullPredicate : attribute.in(values)
end

if nils.empty?
return values_predicate if ranges.empty?
else
unless nils.empty?
values_predicate = values_predicate.or(predicate_builder.build(attribute, nil))
end

array_predicates = ranges.map! { |range| predicate_builder.build(attribute, range) }
array_predicates.inject(values_predicate, &:or)
array_predicates = ranges.map { |range| predicate_builder.build(attribute, range) }
array_predicates.unshift(values_predicate)
array_predicates.inject(&:or)
end

private
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/relation/where_clause.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def contradiction?
predicates.any? do |x|
case x
when Arel::Nodes::In
Arel::Nodes::CastedArray === x.right && x.right.value.empty?
Array === x.right && x.right.empty?
when Arel::Nodes::Equality
x.right.respond_to?(:unboundable?) && x.right.unboundable?
end
Expand Down
10 changes: 0 additions & 10 deletions activerecord/lib/arel/nodes/casted.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,6 @@ def eql?(other)
alias :== :eql?
end

class CastedArray < Casted # :nodoc:
def value_for_database
if attribute.able_to_type_cast?
value.map { |v| attribute.type_cast_for_database(v) }
else
value
end
end
end

class Quoted < Arel::Nodes::Unary # :nodoc:
alias :value_for_database :value
alias :value_before_type_cast :value
Expand Down
4 changes: 2 additions & 2 deletions activerecord/lib/arel/predications.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def eq_any(others)
end

def eq_all(others)
grouping_all :eq, others
grouping_all :eq, quoted_array(others)
end

def between(other)
Expand Down Expand Up @@ -238,7 +238,7 @@ def quoted_node(other)
end

def quoted_array(others)
Nodes::CastedArray.new(others, self)
others.map { |v| quoted_node(v) }
end

def infinity?(value)
Expand Down
1 change: 0 additions & 1 deletion activerecord/lib/arel/table.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ def eql?(other)

def type_cast_for_database(attribute_name, value)
type_caster.type_cast_for_database(attribute_name, value)
rescue ::RangeError
end

def able_to_type_cast?
Expand Down
20 changes: 12 additions & 8 deletions activerecord/lib/arel/visitors/to_sql.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,6 @@ def visit_Arel_Nodes_Exists(o, collector)
end
end

def visit_Arel_Nodes_CastedArray(o, collector)
collector << o.value_for_database.map! { |v| quote(v) }.join(", ")
end

def visit_Arel_Nodes_Casted(o, collector)
collector << quote(o.value_for_database).to_s
end
Expand Down Expand Up @@ -517,8 +513,12 @@ def visit_Arel_Nodes_In(o, collector)
collector.preparable = false
attr, values = o.left, o.right

if Arel::Nodes::CastedArray === values
return collector << "1=0" if values.value.empty?
if Array === values
unless values.empty?
values.delete_if { |value| unboundable?(value) }
end

return collector << "1=0" if values.empty?
end

visit(attr, collector) << " IN ("
Expand All @@ -529,8 +529,12 @@ def visit_Arel_Nodes_NotIn(o, collector)
collector.preparable = false
attr, values = o.left, o.right

if Arel::Nodes::CastedArray === values
return collector << "1=1" if values.value.empty?
if Array === values
unless values.empty?
values.delete_if { |value| unboundable?(value) }
end

return collector << "1=1" if values.empty?
end

visit(attr, collector) << " NOT IN ("
Expand Down
20 changes: 14 additions & 6 deletions activerecord/test/cases/arel/attributes/attribute_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -618,14 +618,14 @@ class AttributeTest < Arel::Spec
attribute = Attribute.new nil, nil
node = attribute.between(-::Float::INFINITY..::Float::INFINITY)

_(node).must_equal attribute.not_in([])
_(node).must_equal Nodes::NotIn.new(attribute, [])
end

it "can be constructed with a quoted infinite range" do
attribute = Attribute.new nil, nil
node = attribute.between(quoted_range(-::Float::INFINITY, ::Float::INFINITY, false))

_(node).must_equal attribute.not_in([])
_(node).must_equal Nodes::NotIn.new(attribute, [])
end

it "can be constructed with a range ending at Infinity" do
Expand Down Expand Up @@ -707,7 +707,11 @@ class AttributeTest < Arel::Spec

_(node).must_equal Nodes::In.new(
attribute,
Nodes::CastedArray.new([1, 2, 3], attribute)
[
Nodes::Casted.new(1, attribute),
Nodes::Casted.new(2, attribute),
Nodes::Casted.new(3, attribute),
]
)
end

Expand Down Expand Up @@ -827,14 +831,14 @@ class AttributeTest < Arel::Spec
attribute = Attribute.new nil, nil
node = attribute.not_between(-::Float::INFINITY..::Float::INFINITY)

_(node).must_equal attribute.in([])
_(node).must_equal Nodes::In.new(attribute, [])
end

it "can be constructed with a quoted infinite range" do
attribute = Attribute.new nil, nil
node = attribute.not_between(quoted_range(-::Float::INFINITY, ::Float::INFINITY, false))

_(node).must_equal attribute.in([])
_(node).must_equal Nodes::In.new(attribute, [])
end

it "can be constructed with a range ending at Infinity" do
Expand Down Expand Up @@ -930,7 +934,11 @@ class AttributeTest < Arel::Spec

_(node).must_equal Nodes::NotIn.new(
attribute,
Nodes::CastedArray.new([1, 2, 3], attribute)
[
Nodes::Casted.new(1, attribute),
Nodes::Casted.new(2, attribute),
Nodes::Casted.new(3, attribute),
]
)
end

Expand Down

0 comments on commit 200058b

Please sign in to comment.