From a8d7d6175a94383023a18ef834d6673083a59133 Mon Sep 17 00:00:00 2001
From: Petr Skoda <commits@skodak.org>
Date: Wed, 23 Apr 2014 13:54:28 +0800
Subject: [PATCH] MDL-45214 always try to use ID sort in get_events_select()

This should make PostgreSQL behave more like MySQL
and resolve the problem when events were appearing in different
order.
---
 admin/tool/log/classes/helper/reader.php      | 22 +++++++++++++++++++
 .../log/store/database/classes/log/store.php  |  2 ++
 .../log/store/database/tests/store_test.php   |  2 +-
 .../log/store/legacy/classes/log/store.php    |  2 ++
 .../log/store/standard/classes/log/store.php  |  2 ++
 .../log/store/standard/tests/store_test.php   |  2 +-
 6 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/admin/tool/log/classes/helper/reader.php b/admin/tool/log/classes/helper/reader.php
index 4d8b4bf52d6..93e0af16fb5 100644
--- a/admin/tool/log/classes/helper/reader.php
+++ b/admin/tool/log/classes/helper/reader.php
@@ -61,4 +61,26 @@ trait reader {
         }
         return $this->store;
     }
+
+    /**
+     * Adds ID column to $sort to make sure events from one request
+     * within 1 second are returned in the same order.
+     *
+     * @param string $sort
+     * @return string sort string
+     */
+    protected static function tweak_sort_by_id($sort) {
+        if (empty($sort)) {
+            // Mysql does this - unlikely to be used in real life because $sort is always expected.
+            $sort = "id ASC";
+        } else if (stripos($sort, 'timecreated') === false) {
+            $sort .= ", id ASC";
+        } else if (stripos($sort, 'timecreated DESC') !== false) {
+            $sort .= ", id DESC";
+        } else {
+            $sort .= ", id ASC";
+        }
+
+        return $sort;
+    }
 }
diff --git a/admin/tool/log/store/database/classes/log/store.php b/admin/tool/log/store/database/classes/log/store.php
index cd26e579667..c394d0b8add 100644
--- a/admin/tool/log/store/database/classes/log/store.php
+++ b/admin/tool/log/store/database/classes/log/store.php
@@ -167,6 +167,8 @@ class store implements \tool_log\log\writer, \core\log\sql_select_reader {
             return array();
         }
 
+        $sort = self::tweak_sort_by_id($sort);
+
         $events = array();
         $records = $this->extdb->get_records_select($dbtable, $selectwhere, $params, $sort, '*', $limitfrom, $limitnum);
 
diff --git a/admin/tool/log/store/database/tests/store_test.php b/admin/tool/log/store/database/tests/store_test.php
index e16a89300aa..48e1b3953f2 100644
--- a/admin/tool/log/store/database/tests/store_test.php
+++ b/admin/tool/log/store/database/tests/store_test.php
@@ -152,7 +152,7 @@ class logstore_database_store_testcase extends advanced_testcase {
 
         // Test reading.
         $this->assertSame(3, $store->get_events_select_count('', array()));
-        $events = $store->get_events_select('', array(), 'id', 0, 0);
+        $events = $store->get_events_select('', array(), 'timecreated ASC', 0, 0); // Is actually sorted by "timecreated ASC, id ASC".
         $this->assertCount(3, $events);
         $resev1 = array_shift($events);
         array_shift($events);
diff --git a/admin/tool/log/store/legacy/classes/log/store.php b/admin/tool/log/store/legacy/classes/log/store.php
index a8e981c627d..eb75d4bd1ce 100644
--- a/admin/tool/log/store/legacy/classes/log/store.php
+++ b/admin/tool/log/store/legacy/classes/log/store.php
@@ -86,6 +86,8 @@ class store implements \tool_log\log\store, \core\log\sql_select_reader {
     public function get_events_select($selectwhere, array $params, $sort, $limitfrom, $limitnum) {
         global $DB;
 
+        $sort = self::tweak_sort_by_id($sort);
+
         // Replace the query with hardcoded mappings required for core.
         list($selectwhere, $params, $sort) = self::replace_sql_legacy($selectwhere, $params, $sort);
 
diff --git a/admin/tool/log/store/standard/classes/log/store.php b/admin/tool/log/store/standard/classes/log/store.php
index 310f1a55f19..992d4cfc06e 100644
--- a/admin/tool/log/store/standard/classes/log/store.php
+++ b/admin/tool/log/store/standard/classes/log/store.php
@@ -69,6 +69,8 @@ class store implements \tool_log\log\writer, \core\log\sql_internal_reader {
     public function get_events_select($selectwhere, array $params, $sort, $limitfrom, $limitnum) {
         global $DB;
 
+        $sort = self::tweak_sort_by_id($sort);
+
         $events = array();
         $records = $DB->get_records_select('logstore_standard_log', $selectwhere, $params, $sort, '*', $limitfrom, $limitnum);
 
diff --git a/admin/tool/log/store/standard/tests/store_test.php b/admin/tool/log/store/standard/tests/store_test.php
index 2d42b4cf202..5d25a2ef952 100644
--- a/admin/tool/log/store/standard/tests/store_test.php
+++ b/admin/tool/log/store/standard/tests/store_test.php
@@ -122,7 +122,7 @@ class logstore_standard_store_testcase extends advanced_testcase {
 
         // Test reading.
         $this->assertSame(3, $store->get_events_select_count('', array()));
-        $events = $store->get_events_select('', array(), 'id', 0, 0);
+        $events = $store->get_events_select('', array(), 'timecreated ASC', 0, 0); // Is actually sorted by "timecreated ASC, id ASC".
         $this->assertCount(3, $events);
         $resev1 = array_shift($events);
         array_shift($events);