1 files changed, 27 insertions, 33 deletions

diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb
index 3cbb12a09d..5d4a045097 100644
--- a/activerecord/lib/active_record/relation/finder_methods.rb
+++ b/activerecord/lib/active_record/relation/finder_methods.rb
@@ -145,15 +145,23 @@ module ActiveRecord
     #
     #   [#<Person id:4>, #<Person id:3>, #<Person id:2>]
     def last(limit = nil)
-      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
+      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
     end
 
     # Same as #last but raises ActiveRecord::RecordNotFound if no record
@@ -489,20 +497,19 @@ module ActiveRecord
     end
 
     def find_nth(index, offset = nil)
+      # TODO: once the offset argument is removed we rely on offset_index
+      # within find_nth_with_limit, rather than pass it in via
+      # find_nth_with_limit_and_offset
+      if offset
+        ActiveSupport::Deprecation.warn(<<-MSG.squish)
+          Passing an offset argument to find_nth is deprecated,
+          please use Relation#offset instead.
+        MSG
+      end
       if loaded?
         @records[index]
       else
-        # TODO: once the offset argument is removed we rely on offset_index
-        # within find_nth_with_limit, rather than pass it in via
-        # find_nth_with_limit_and_offset
-        if offset
-          ActiveSupport::Deprecation.warn(<<-MSG.squish)
-            Passing an offset argument to find_nth is deprecated,
-            please use Relation#offset instead.
-          MSG
-        else
-          offset = offset_index
-        end
+        offset ||= offset_index
         @offsets[offset + index] ||= find_nth_with_limit_and_offset(index, 1, offset: offset).first
       end
     end
@@ -524,19 +531,6 @@ 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: