Additionally order by primary key if implicit_order_column is not uniq

activerecord/CHANGELOG.md

+*   Specifying `implicit_order_column` now subsorts the records by primary key if available to ensure deterministic results.
+
+    *Paweł Urbanek*
+
 *   `where(attr => [])` now loads an empty result without making a query.
 
     *John Hawthorn*

activerecord/lib/active_record/model_schema.rb

@@ -115,8 +115,8 @@ module ModelSchema
     #
     # Sets the column to sort records by when no explicit order clause is used
     # during an ordered finder call. Useful when the primary key is not an
-    # auto-incrementing integer, for example when it's a UUID. Note that using
-    # a non-unique column can result in non-deterministic results.
+    # auto-incrementing integer, for example when it's a UUID. Records are subsorted
+    # by the primary key if it exists to ensure deterministic results.
     included do
       mattr_accessor :primary_key_prefix_type, instance_writer: false
 

activerecord/lib/active_record/relation/finder_methods.rb

@@ -560,7 +560,11 @@ def find_last(limit)
 
       def ordered_relation
         if order_values.empty? && (implicit_order_column || primary_key)
-          order(arel_attribute(implicit_order_column || primary_key).asc)
+          if implicit_order_column && primary_key && implicit_order_column != primary_key
+            order(arel_attribute(implicit_order_column).asc, arel_attribute(primary_key).asc)
+          else
+            order(arel_attribute(implicit_order_column || primary_key).asc)
+          end
         else
           self
         end

activerecord/test/cases/finder_test.rb

@@ -22,6 +22,7 @@
 require "models/car"
 require "models/tyre"
 require "models/subscriber"
+require "models/non_primary_key"
 require "support/stubs/strong_parameters"
 
 class FinderTest < ActiveRecord::TestCase
@@ -797,10 +798,39 @@ def test_implicit_order_column_is_configurable
 
     assert_equal topics(:fifth), Topic.first
     assert_equal topics(:third), Topic.last
+
+    c = Topic.connection
+    assert_sql(/ORDER BY #{Regexp.escape(c.quote_table_name("topics.title"))} DESC, #{Regexp.escape(c.quote_table_name("topics.id"))} DESC LIMIT/i) {
+      Topic.last
+    }
+  ensure
+    Topic.implicit_order_column = old_implicit_order_column
+  end
+
+  def test_implicit_order_set_to_primary_key
+    old_implicit_order_column = Topic.implicit_order_column
+    Topic.implicit_order_column = "id"
+
+    c = Topic.connection
+    assert_sql(/ORDER BY #{Regexp.escape(c.quote_table_name("topics.id"))} DESC LIMIT/i) {
+      Topic.last
+    }
   ensure
     Topic.implicit_order_column = old_implicit_order_column
   end
 
+  def test_implicit_order_for_model_without_primary_key
+    old_implicit_order_column = NonPrimaryKey.implicit_order_column
+    NonPrimaryKey.implicit_order_column = "created_at"
+
+    c = NonPrimaryKey.connection
+    assert_sql(/ORDER BY #{Regexp.escape(c.quote_table_name("non_primary_keys.created_at"))} DESC LIMIT/i) {
+      NonPrimaryKey.last
+    }
+  ensure
+    NonPrimaryKey.implicit_order_column = old_implicit_order_column
+  end
+
   def test_take_and_first_and_last_with_integer_should_return_an_array
     assert_kind_of Array, Topic.take(5)
     assert_kind_of Array, Topic.first(5)

activerecord/test/schema/schema.rb

@@ -1117,6 +1117,7 @@
 
   create_table :non_primary_keys, force: true, id: false do |t|
     t.integer :id
+    t.datetime :created_at
   end
 end