Merge pull request #24131 from brchristian/limit_and_primary_key

ActiveRecord: limit() and primary_key

3 files changed, 22 insertions, 1 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index d1ae41ab97..456f569718 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,9 @@
+*   Don't impose primary key order if limit() has already been supplied.
+
+    Fixes #23607
+
+    *Brian Christian*
+
 *   Add environment & load_config dependency to `bin/rake db:seed` to enable
     seed load in environments without Rails and custom DB configuration
 
diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb
index c5562c1ff0..93f3b67686 100644
--- a/activerecord/lib/active_record/relation/finder_methods.rb
+++ b/activerecord/lib/active_record/relation/finder_methods.rb
@@ -550,7 +550,7 @@ module ActiveRecord
       end
 
       def ordered_relation
-        if order_values.empty? && primary_key
+        if order_values.empty? && primary_key && limit_value.blank?
           order(arel_attribute(primary_key).asc)
         else
           self
diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
index 39ffe12ec6..2f256dd36a 100644
--- a/activerecord/test/cases/finder_test.rb
+++ b/activerecord/test/cases/finder_test.rb
@@ -1265,6 +1265,21 @@ class FinderTest < ActiveRecord::TestCase
     end
   end
 
+  def test_first_and_last_with_limit_for_order_without_primary_key
+    # While Topic.first should impose an ordering by primary key,
+    # Topic.limit(n).first should not
+
+    Topic.first.touch # PostgreSQL changes the default order if no order clause is used
+
+    assert_equal Topic.limit(1).to_a.first, Topic.limit(1).first
+    assert_equal Topic.limit(2).to_a.first, Topic.limit(2).first
+    assert_equal Topic.limit(2).to_a.first(2), Topic.limit(2).first(2)
+
+    assert_equal Topic.limit(1).to_a.last, Topic.limit(1).last
+    assert_equal Topic.limit(2).to_a.last, Topic.limit(2).last
+    assert_equal Topic.limit(2).to_a.last(2), Topic.limit(2).last(2)
+  end
+
   def test_finder_with_offset_string
     assert_nothing_raised { Topic.offset("3").to_a }
   end