Merge pull request #30000 from kamipo/all_of_queries_should_return_correct_result

All of queries should return correct result even if including large number

activerecord/lib/active_record/associations/association.rb

@@ -190,9 +190,7 @@ def find_target
           end
 
           binds = AssociationScope.get_bind_values(owner, reflection.chain)
-          sc.execute(binds, conn) do |record|
-            set_inverse_instance(record)
-          end
+          sc.execute(binds, conn) { |record| set_inverse_instance(record) } || []
         end
 
         # The scope for this association.

activerecord/lib/active_record/associations/singular_association.rb

@@ -37,8 +37,6 @@ def scope_for_create
 
         def find_target
           super.first
-        rescue ::RangeError
-          nil
         end
 
         def replace(record)

activerecord/lib/active_record/core.rb

@@ -169,15 +169,12 @@ def find(*ids) # :nodoc:
           where(key => params.bind).limit(1)
         }
 
-        record = statement.execute([id], connection).first
+        record = statement.execute([id], connection)&.first
         unless record
           raise RecordNotFound.new("Couldn't find #{name} with '#{primary_key}'=#{id}",
                                    name, primary_key, id)
         end
         record
-      rescue ::RangeError
-        raise RecordNotFound.new("Couldn't find #{name} with an out of range value for '#{primary_key}'",
-                                 name, primary_key)
       end
 
       def find_by(*args) # :nodoc:
@@ -201,11 +198,9 @@ def find_by(*args) # :nodoc:
           where(wheres).limit(1)
         }
         begin
-          statement.execute(hash.values, connection).first
+          statement.execute(hash.values, connection)&.first
         rescue TypeError
           raise ActiveRecord::StatementInvalid
-        rescue ::RangeError
-          nil
         end
       end
 

activerecord/lib/active_record/relation/finder_methods.rb

@@ -79,17 +79,12 @@ def find(*args)
     #   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 @@ def exists?(conditions = :none)
       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 @@ def find_with_ids(*ids)
         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)

activerecord/lib/active_record/statement_cache.rb

@@ -132,6 +132,8 @@ def execute(params, connection, &block)
       sql = query_builder.sql_for bind_values, connection
 
       klass.find_by_sql(sql, bind_values, preparable: true, &block)
+    rescue ::RangeError
+      nil
     end
 
     def self.unsupported_value?(value)

activerecord/lib/arel/predications.rb

@@ -35,15 +35,17 @@ def eq_all(others)
     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 @@ def in_all(others)
     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 @@ def quoted_array(others)
       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

activerecord/lib/arel/visitors/to_sql.rb

@@ -629,6 +629,8 @@ def visit_Arel_Nodes_Assignment(o, collector)
         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 @@ def visit_Arel_Nodes_IsDistinctFrom(o, collector)
         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?

activerecord/test/cases/associations/belongs_to_associations_test.rb

@@ -1293,17 +1293,17 @@ def test_belongs_to_with_id_assigning
   end
 
   def test_belongs_to_with_out_of_range_value_assigning
-    model = Class.new(Comment) do
+    model = Class.new(Author) do
       def self.name; "Temp"; end
-      validates :post, presence: true
+      validates :author_address, presence: true
     end
 
-    comment = model.new
-    comment.post_id = 9223372036854775808 # out of range in the bigint
+    author = model.new
+    author.author_address_id = 9223372036854775808 # out of range in the bigint
 
-    assert_nil comment.post
-    assert_not_predicate comment, :valid?
-    assert_equal [{ error: :blank }], comment.errors.details[:post]
+    assert_nil author.author_address
+    assert_not_predicate author, :valid?
+    assert_equal [{ error: :blank }], author.errors.details[:author_address]
   end
 
   def test_polymorphic_with_custom_primary_key

activerecord/test/cases/associations/has_many_associations_test.rb

@@ -27,6 +27,7 @@
 require "models/minivan"
 require "models/speedometer"
 require "models/reference"
+require "models/job"
 require "models/college"
 require "models/student"
 require "models/pirate"
@@ -2926,6 +2927,11 @@ def test_create_children_could_be_rolled_back_by_after_save
     end
   end
 
+  def test_has_many_with_out_of_range_value
+    reference = Reference.create!(id: 2147483648) # out of range in the integer
+    assert_equal [], reference.ideal_jobs
+  end
+
   private
 
     def force_signal37_to_load_all_clients_of_firm

activerecord/test/cases/calculations_test.rb

@@ -838,13 +838,13 @@ def test_pluck_loaded_relation_sql_fragment
   def test_pick_one
     assert_equal "The First Topic", Topic.order(:id).pick(:heading)
     assert_nil Topic.none.pick(:heading)
-    assert_nil Topic.where("1=0").pick(:heading)
+    assert_nil Topic.where(id: 9999999999999999999).pick(:heading)
   end
 
   def test_pick_two
     assert_equal ["David", "david@loudthinking.com"], Topic.order(:id).pick(:author_name, :author_email_address)
     assert_nil Topic.none.pick(:author_name, :author_email_address)
-    assert_nil Topic.where("1=0").pick(:author_name, :author_email_address)
+    assert_nil Topic.where(id: 9999999999999999999).pick(:author_name, :author_email_address)
   end
 
   def test_pick_delegate_to_all

activerecord/test/cases/finder_test.rb

@@ -282,6 +282,17 @@ def test_exists_with_order
     assert_equal true, Topic.order(Arel.sql("invalid sql here")).exists?
   end
 
+  def test_exists_with_large_number
+    assert_equal true, Topic.where(id: [1, 9223372036854775808]).exists?
+    assert_equal true, Topic.where(id: 1..9223372036854775808).exists?
+    assert_equal true, Topic.where(id: -9223372036854775809..9223372036854775808).exists?
+    assert_equal false, Topic.where(id: 9223372036854775808..9223372036854775809).exists?
+    assert_equal false, Topic.where(id: -9223372036854775810..-9223372036854775809).exists?
+    assert_equal false, Topic.where(id: 9223372036854775808..1).exists?
+    assert_equal true, Topic.where(id: 1).or(Topic.where(id: 9223372036854775808)).exists?
+    assert_equal true, Topic.where.not(id: 9223372036854775808).exists?
+  end
+
   def test_exists_with_joins
     assert_equal true, Topic.joins(:replies).where(replies_topics: { approved: true }).order("replies_topics.created_at DESC").exists?
   end
@@ -383,16 +394,19 @@ def test_find_on_relation_with_large_number
     assert_raises(ActiveRecord::RecordNotFound) do
       Topic.where("1=1").find(9999999999999999999999999999999)
     end
+    assert_equal topics(:first), Topic.where(id: [1, 9999999999999999999999999999999]).find(1)
   end
 
   def test_find_by_on_relation_with_large_number
     assert_nil Topic.where("1=1").find_by(id: 9999999999999999999999999999999)
+    assert_equal topics(:first), Topic.where(id: [1, 9999999999999999999999999999999]).find_by(id: 1)
   end
 
   def test_find_by_bang_on_relation_with_large_number
     assert_raises(ActiveRecord::RecordNotFound) do
       Topic.where("1=1").find_by!(id: 9999999999999999999999999999999)
     end
+    assert_equal topics(:first), Topic.where(id: [1, 9999999999999999999999999999999]).find_by!(id: 1)
   end
 
   def test_find_an_empty_array

activerecord/test/cases/relation/or_test.rb

@@ -30,6 +30,11 @@ def test_or_with_null_right
       assert_equal expected, Post.where("id = 1").or(Post.none).to_a
     end
 
+    def test_or_with_large_number
+      expected = Post.where("id = 1 or id = 9223372036854775808").to_a
+      assert_equal expected, Post.where(id: 1).or(Post.where(id: 9223372036854775808)).to_a
+    end
+
     def test_or_with_bind_params
       assert_equal Post.find([1, 2]).sort_by(&:id), Post.where(id: 1).or(Post.where(id: 2)).sort_by(&:id)
     end

activerecord/test/cases/relation/where_test.rb

@@ -359,6 +359,16 @@ def permit!
       assert_equal author, Author.where(params.permit!).first
     end
 
+    def test_where_with_large_number
+      assert_equal [authors(:bob)], Author.where(id: [3, 9223372036854775808])
+      assert_equal [authors(:bob)], Author.where(id: 3..9223372036854775808)
+    end
+
+    def test_to_sql_with_large_number
+      assert_equal [authors(:bob)], Author.find_by_sql(Author.where(id: [3, 9223372036854775808]).to_sql)
+      assert_equal [authors(:bob)], Author.find_by_sql(Author.where(id: 3..9223372036854775808).to_sql)
+    end
+
     def test_where_with_unsupported_arguments
       assert_raises(ArgumentError) { Author.where(42) }
     end

activerecord/test/models/reference.rb

@@ -4,6 +4,7 @@ class Reference < ActiveRecord::Base
   belongs_to :person
   belongs_to :job
 
+  has_many :ideal_jobs, class_name: "Job", foreign_key: :ideal_reference_id
   has_many :agents_posts_authors, through: :person
 
   class << self; attr_accessor :make_comments; end