diff options
author | Ryuta Kamizono <kamipo@gmail.com> | 2017-07-24 09:50:11 +0900 |
---|---|---|
committer | Ryuta Kamizono <kamipo@gmail.com> | 2019-01-18 16:01:07 +0900 |
commit | 31ffbf8d5056137717da3f11d28c4fbd7fbc8f07 (patch) | |
tree | 3b8d6a2d9329a91178d358f0e45c7ec6b13362ee /activerecord/lib | |
parent | 5b6daff5b6d5439e07c058718069f54b34970f93 (diff) | |
download | rails-31ffbf8d5056137717da3f11d28c4fbd7fbc8f07.tar.gz rails-31ffbf8d5056137717da3f11d28c4fbd7fbc8f07.tar.bz2 rails-31ffbf8d5056137717da3f11d28c4fbd7fbc8f07.zip |
All of queries should return correct result even if including large number
Currently several queries cannot return correct result due to incorrect
`RangeError` handling.
First example:
```ruby
assert_equal true, Topic.where(id: [1, 9223372036854775808]).exists?
assert_equal true, Topic.where.not(id: 9223372036854775808).exists?
```
The first example is obviously to be true, but currently it returns
false.
Second example:
```ruby
assert_equal topics(:first), Topic.where(id: 1..9223372036854775808).find(1)
```
The second example also should return the object, but currently it
raises `RecordNotFound`.
It can be seen from the examples, the queries including large number
assuming empty result is not always correct.
Therefore, This change handles `RangeError` to generate executable SQL
instead of raising `RangeError` to users to always return correct
result. By this change, it is no longer raised `RangeError` to users.
Diffstat (limited to 'activerecord/lib')
-rw-r--r-- | activerecord/lib/active_record/relation/finder_methods.rb | 10 | ||||
-rw-r--r-- | activerecord/lib/arel/predications.rb | 24 | ||||
-rw-r--r-- | activerecord/lib/arel/visitors/to_sql.rb | 4 |
3 files changed, 22 insertions, 16 deletions
diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index fd84f9c46b..8f1065c1e7 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -79,17 +79,12 @@ module ActiveRecord # Post.find_by "published_at < ?", 2.weeks.ago def find_by(arg, *args) where(arg, *args).take - rescue ::RangeError - nil end # Like #find_by, except that if no record is found, raises # an ActiveRecord::RecordNotFound error. def find_by!(arg, *args) where(arg, *args).take! - rescue ::RangeError - raise RecordNotFound.new("Couldn't find #{@klass.name} with an out of range value", - @klass.name, @klass.primary_key) end # Gives a record (or N records if a parameter is supplied) without any implied @@ -322,8 +317,6 @@ module ActiveRecord relation = construct_relation_for_exists(conditions) skip_query_cache_if_necessary { connection.select_value(relation.arel, "#{name} Exists") } ? true : false - rescue ::RangeError - false end # This method is called whenever no records are found with either a single @@ -434,9 +427,6 @@ module ActiveRecord else find_some(ids) end - rescue ::RangeError - error_message = "Couldn't find #{model_name} with an out of range ID" - raise RecordNotFound.new(error_message, model_name, primary_key, ids) end def find_one(id) diff --git a/activerecord/lib/arel/predications.rb b/activerecord/lib/arel/predications.rb index 2a62c53aa3..7dafde4952 100644 --- a/activerecord/lib/arel/predications.rb +++ b/activerecord/lib/arel/predications.rb @@ -35,15 +35,17 @@ module Arel # :nodoc: all end def between(other) - if infinity?(other.begin) - if other.end.nil? || infinity?(other.end) + if unboundable?(other.begin) == 1 || unboundable?(other.end) == -1 + self.in([]) + elsif open_ended?(other.begin) + if other.end.nil? || open_ended?(other.end) not_in([]) elsif other.exclude_end? lt(other.end) else lteq(other.end) end - elsif other.end.nil? || infinity?(other.end) + elsif other.end.nil? || open_ended?(other.end) gteq(other.begin) elsif other.exclude_end? gteq(other.begin).and(lt(other.end)) @@ -81,15 +83,17 @@ Passing a range to `#in` is deprecated. Call `#between`, instead. end def not_between(other) - if infinity?(other.begin) - if other.end.nil? || infinity?(other.end) + if unboundable?(other.begin) == 1 || unboundable?(other.end) == -1 + not_in([]) + elsif open_ended?(other.begin) + if other.end.nil? || open_ended?(other.end) self.in([]) elsif other.exclude_end? gteq(other.end) else gt(other.end) end - elsif other.end.nil? || infinity?(other.end) + elsif other.end.nil? || open_ended?(other.end) lt(other.begin) else left = lt(other.begin) @@ -241,5 +245,13 @@ Passing a range to `#not_in` is deprecated. Call `#not_between`, instead. def infinity?(value) value.respond_to?(:infinite?) && value.infinite? end + + def unboundable?(value) + value.respond_to?(:unboundable?) && value.unboundable? + end + + def open_ended?(value) + infinity?(value) || unboundable?(value) + end end end diff --git a/activerecord/lib/arel/visitors/to_sql.rb b/activerecord/lib/arel/visitors/to_sql.rb index c08403eea9..d0dec63860 100644 --- a/activerecord/lib/arel/visitors/to_sql.rb +++ b/activerecord/lib/arel/visitors/to_sql.rb @@ -629,6 +629,8 @@ module Arel # :nodoc: all def visit_Arel_Nodes_Equality(o, collector) right = o.right + return collector << "1=0" if unboundable?(right) + collector = visit o.left, collector if right.nil? @@ -662,6 +664,8 @@ module Arel # :nodoc: all def visit_Arel_Nodes_NotEqual(o, collector) right = o.right + return collector << "1=1" if unboundable?(right) + collector = visit o.left, collector if right.nil? |