Revert "Merge pull request #16400 from bogdan/last-with-sql"

This reverts commit 9f3730a516, reversing
changes made to 2637fb75d8.

There are additional issues with this commit that need to be addressed
before this change is ready (see #23377). This is a temporary revert in
order for us to have more time to address the issues with that PR,
without blocking the release of beta2.

activerecord/CHANGELOG.md

@@ -1,23 +1,3 @@
-*   Rework `ActiveRecord::Relation#last` 
-    
-    1. Always find last with ruby if relation is loaded
-    2. Always use SQL instead if relation is not loaded.
-    3. Deprecated relation loading when SQL order can not be automatically reversed
-
-        Topic.order("title").load.last(3)
-          # before: SELECT ...
-          # after: No SQL
-
-        Topic.order("title").last
-          # before: SELECT * FROM `topics`
-          # after:  SELECT * FROM `topics` ORDER BY `topics`.`title` DESC LIMIT 1
-
-        Topic.order("coalesce(author, title)").last
-          # before: SELECT * FROM `topics`
-          # after:  Deprecation Warning for irreversible order
-
-    *Bogdan Gusiev*
-
 *   `ActiveRecord::Relation#reverse_order` throws `ActiveRecord::IrreversibleOrderError`
     when the order can not be reversed using current trivial algorithm.
     Also raises the same error when `#reverse_order` is called on

activerecord/lib/active_record/relation/finder_methods.rb

@@ -145,23 +145,15 @@ module ActiveRecord
     #
     #   [#<Person id:4>, #<Person id:3>, #<Person id:2>]
     def last(limit = nil)
-      return find_last(limit) if loaded?
-
-      result = order_values.empty? && primary_key ? order(arel_table[primary_key].desc) : reverse_order
-      result = result.limit!(limit || 1)
-      limit ? result.reverse : result.first
-    rescue ActiveRecord::IrreversibleOrderError
-      ActiveSupport::Deprecation.warn(<<-WARNING.squish)
-          Finding a last element by loading the relation when SQL ORDER
-          can not be reversed is deprecated.
-          Rails 5.1 will raise ActiveRecord::IrreversibleOrderError in this case.
-          Please call `to_a.last` if you still want to load the relation.
-      WARNING
-      find_last(limit)
-    end
-
-    def find_last(limit)
-      limit ? to_a.last(limit) : to_a.last
+      if limit
+        if order_values.empty? && primary_key
+          order(arel_table[primary_key].desc).limit(limit).reverse
+        else
+          to_a.last(limit)
+        end
+      else
+        find_last
+      end
     end
 
     # Same as #last but raises ActiveRecord::RecordNotFound if no record
@@ -531,6 +523,19 @@ module ActiveRecord
       relation.limit(limit).to_a
     end
 
+    def find_last
+      if loaded?
+        @records.last
+      else
+        @last ||=
+          if limit_value
+            to_a.last
+          else
+            reverse_order.limit(1).to_a.first
+          end
+      end
+    end
+
     private
 
     def find_nth_with_limit_and_offset(index, limit, offset:) # :nodoc:

activerecord/test/cases/finder_test.rb

@@ -506,7 +506,7 @@ class FinderTest < ActiveRecord::TestCase
     end
   end
 
-  def test_take_and_first_and_last_with_integer_should_use_sql
+  def test_take_and_first_and_last_with_integer_should_use_sql_limit
     assert_sql(/LIMIT|ROWNUM <=/) { Topic.take(3).entries }
     assert_sql(/LIMIT|ROWNUM <=/) { Topic.first(2).entries }
     assert_sql(/LIMIT|ROWNUM <=/) { Topic.last(5).entries }
@@ -516,30 +516,16 @@ class FinderTest < ActiveRecord::TestCase
     assert_equal Topic.order("title").to_a.last(2), Topic.order("title").last(2)
   end
 
-  def test_last_with_integer_and_order_should_use_sql
-    relation = Topic.order("title")
-    assert_queries(1) { relation.last(5) }
-    assert !relation.loaded?
+  def test_last_with_integer_and_order_should_not_use_sql_limit
+    query = assert_sql { Topic.order("title").last(5).entries }
+    assert_equal 1, query.length
+    assert_no_match(/LIMIT/, query.first)
   end
 
-  def test_last_with_integer_and_reorder_should_use_sql
-    relation = Topic.reorder("title")
-    assert_queries(1) { relation.last(5) }
-    assert !relation.loaded?
-  end
-
-  def test_last_on_loaded_relation_should_not_use_sql
-    relation  = Topic.limit(10).load
-    assert_no_queries do
-      relation.last
-      relation.last(2)
-    end
-  end
-
-  def test_last_with_irreversible_order
-    assert_deprecated do
-      Topic.order("coalesce(author_name, title)").last
-    end
+  def test_last_with_integer_and_reorder_should_not_use_sql_limit
+    query = assert_sql { Topic.reorder("title").last(5).entries }
+    assert_equal 1, query.length
+    assert_no_match(/LIMIT/, query.first)
   end
 
   def test_take_and_first_and_last_with_integer_should_return_an_array