From a46e5ef6216ca8c6023593c6c24dfb62be49df10 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Mon, 24 Nov 2014 10:50:28 +0000
Subject: [PATCH] SYN-163: Add an order by rowid to selects.

This should fix the bug where the edges of the graph get returned in a
different order than they were inserted in, and so no get_event no
longer returned the exact same JSON as was inserted. This meant that
signature checks failed.
---
 synapse/storage/_base.py   | 15 ++++++++++-----
 tests/storage/test_base.py | 12 ++++++++----
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py
index fd5b2affa..4881f0336 100644
--- a/synapse/storage/_base.py
+++ b/synapse/storage/_base.py
@@ -246,7 +246,10 @@ class SQLBaseStore(object):
                 raise StoreError(404, "No row found")
 
     def _simple_select_onecol_txn(self, txn, table, keyvalues, retcol):
-        sql = "SELECT %(retcol)s FROM %(table)s WHERE %(where)s" % {
+        sql = (
+            "SELECT %(retcol)s FROM %(table)s WHERE %(where)s "
+            "ORDER BY rowid asc"
+        ) % {
             "retcol": retcol,
             "table": table,
             "where": " AND ".join("%s = ?" % k for k in keyvalues.keys()),
@@ -299,7 +302,7 @@ class SQLBaseStore(object):
             keyvalues : dict of column names and values to select the rows with
             retcols : list of strings giving the names of the columns to return
         """
-        sql = "SELECT %s FROM %s WHERE %s" % (
+        sql = "SELECT %s FROM %s WHERE %s ORDER BY rowid asc" % (
             ", ".join(retcols),
             table,
             " AND ".join("%s = ?" % (k, ) for k in keyvalues)
@@ -334,7 +337,7 @@ class SQLBaseStore(object):
                                  retcols=None, allow_none=False):
         """ Combined SELECT then UPDATE."""
         if retcols:
-            select_sql = "SELECT %s FROM %s WHERE %s" % (
+            select_sql = "SELECT %s FROM %s WHERE %s ORDER BY rowid asc" % (
                 ", ".join(retcols),
                 table,
                 " AND ".join("%s = ?" % (k) for k in keyvalues)
@@ -461,7 +464,7 @@ class SQLBaseStore(object):
     def _get_events_txn(self, txn, event_ids):
         # FIXME (erikj): This should be batched?
 
-        sql = "SELECT * FROM events WHERE event_id = ?"
+        sql = "SELECT * FROM events WHERE event_id = ? ORDER BY rowid asc"
 
         event_rows = []
         for e_id in event_ids:
@@ -478,7 +481,9 @@ class SQLBaseStore(object):
     def _parse_events_txn(self, txn, rows):
         events = [self._parse_event_from_row(r) for r in rows]
 
-        select_event_sql = "SELECT * FROM events WHERE event_id = ?"
+        select_event_sql = (
+            "SELECT * FROM events WHERE event_id = ? ORDER BY rowid asc"
+        )
 
         for i, ev in enumerate(events):
             signatures = self._get_event_signatures_txn(
diff --git a/tests/storage/test_base.py b/tests/storage/test_base.py
index fabd364be..a6f1d6a33 100644
--- a/tests/storage/test_base.py
+++ b/tests/storage/test_base.py
@@ -84,7 +84,8 @@ class SQLBaseStoreTestCase(unittest.TestCase):
 
         self.assertEquals("Value", value)
         self.mock_txn.execute.assert_called_with(
-                "SELECT retcol FROM tablename WHERE keycol = ?",
+                "SELECT retcol FROM tablename WHERE keycol = ? "
+                "ORDER BY rowid asc",
                 ["TheKey"]
         )
 
@@ -101,7 +102,8 @@ class SQLBaseStoreTestCase(unittest.TestCase):
 
         self.assertEquals({"colA": 1, "colB": 2, "colC": 3}, ret)
         self.mock_txn.execute.assert_called_with(
-                "SELECT colA, colB, colC FROM tablename WHERE keycol = ?",
+                "SELECT colA, colB, colC FROM tablename WHERE keycol = ? "
+                "ORDER BY rowid asc",
                 ["TheKey"]
         )
 
@@ -135,7 +137,8 @@ class SQLBaseStoreTestCase(unittest.TestCase):
 
         self.assertEquals([{"colA": 1}, {"colA": 2}, {"colA": 3}], ret)
         self.mock_txn.execute.assert_called_with(
-                "SELECT colA FROM tablename WHERE keycol = ?",
+                "SELECT colA FROM tablename WHERE keycol = ? "
+                "ORDER BY rowid asc",
                 ["A set"]
         )
 
@@ -184,7 +187,8 @@ class SQLBaseStoreTestCase(unittest.TestCase):
 
         self.assertEquals({"columname": "Old Value"}, ret)
         self.mock_txn.execute.assert_has_calls([
-                call('SELECT columname FROM tablename WHERE keycol = ?',
+                call('SELECT columname FROM tablename WHERE keycol = ? '
+                     'ORDER BY rowid asc',
                     ['TheKey']),
                 call("UPDATE tablename SET columname = ? WHERE keycol = ?",
                     ["New Value", "TheKey"])