diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 657ca68e90..2808bfd266 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* Deprecate `.reorder(nil)` with `.first` / `.first!` taking non-deterministic result. + + To continue taking non-deterministic result, use `.take` / `.take!` instead. + + *Ryuta Kamizono* + * Ensure custom PK types are casted in through reflection queries. *Gannon McGibbon* @@ -78,4 +84,5 @@ *Michael Duchemin* + Please check [6-0-stable](https://github.com/rails/rails/blob/6-0-stable/activerecord/CHANGELOG.md) for previous changes. diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 1dbf4808fd..a08c46d58e 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -114,6 +114,15 @@ def take! # Person.first(3) # returns the first three objects fetched by SELECT * FROM people ORDER BY people.id LIMIT 3 # def first(limit = nil) + if !order_values.empty? && order_values.all?(&:blank?) + blank_value = order_values.first + ActiveSupport::Deprecation.warn(<<~MSG.squish) + `.reorder(#{blank_value.inspect})` with `.first` / `.first!` no longer + takes non-deterministic result in Rails 6.2. + To continue taking non-deterministic result, use `.take` / `.take!` instead. + MSG + end + if limit find_nth_with_limit(0, limit) else diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 1620143936..cf2da90326 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1739,7 +1739,14 @@ def test_reverse_order_with_reorder_nil_removes_the_order def test_reorder_with_first sql_log = capture_sql do - assert Post.order(:title).reorder(nil).first + message = <<~MSG.squish + `.reorder(nil)` with `.first` / `.first!` no longer + takes non-deterministic result in Rails 6.2. + To continue taking non-deterministic result, use `.take` / `.take!` instead. + MSG + assert_deprecated(message) do + assert Post.order(:title).reorder(nil).first + end end assert sql_log.all? { |sql| !/order by/i.match?(sql) }, "ORDER BY was used in the query: #{sql_log}" end