Deprecate .reorder(nil) with .first / .first! taking non-deterministic result

To continue taking non-deterministic result, use `.take` / `.take!`
instead.
This commit is contained in:
Ryuta Kamizono 2019-08-08 19:10:25 +09:00
parent 49b617f68c
commit eec562dbcf
3 changed files with 24 additions and 1 deletions

@ -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.

@ -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

@ -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