summary refs log tree commit diff
diff options
context:
space:
mode:
authorRichard van der Hoff <richard@matrix.org>2021-04-08 13:45:19 +0100
committerRichard van der Hoff <richard@matrix.org>2021-04-08 16:42:32 +0100
commit3ada9b42640137d908fa2db64e78f3f79d11dd3a (patch)
treef27106f38f792717c64eb1b885fbed49f5a6ae30
parentRequire py36 and Postgres 9.6 (diff)
downloadsynapse-3ada9b42640137d908fa2db64e78f3f79d11dd3a.tar.xz
Drop support for sqlite<3.22 as well
-rw-r--r--changelog.d/9766.feature2
-rw-r--r--synapse/storage/database.py62
-rw-r--r--synapse/storage/engines/_base.py8
-rw-r--r--synapse/storage/engines/postgres.py7
-rw-r--r--synapse/storage/engines/sqlite.py15
-rw-r--r--tests/storage/test_database.py12
6 files changed, 14 insertions, 92 deletions
diff --git a/changelog.d/9766.feature b/changelog.d/9766.feature
index 030bdb4561..ecf49cfee1 100644
--- a/changelog.d/9766.feature
+++ b/changelog.d/9766.feature
@@ -1 +1 @@
-Synapse now requires Python 3.6 or later and Postgres 9.6 or later.
+Synapse now requires Python 3.6 or later. It also requires Postgres 9.6 or later or SQLite 3.22 or later.
diff --git a/synapse/storage/database.py b/synapse/storage/database.py
index 94590e7b45..a2f016a7ef 100644
--- a/synapse/storage/database.py
+++ b/synapse/storage/database.py
@@ -2060,68 +2060,20 @@ KV = TypeVar("KV")
 
 
 def make_tuple_comparison_clause(
-    database_engine: BaseDatabaseEngine, keys: List[Tuple[str, KV]]
+    _database_engine: BaseDatabaseEngine, keys: List[Tuple[str, KV]]
 ) -> Tuple[str, List[KV]]:
     """Returns a tuple comparison SQL clause
 
-    Depending what the SQL engine supports, builds a SQL clause that looks like either
-    "(a, b) > (?, ?)", or "(a > ?) OR (a == ? AND b > ?)".
+    Builds a SQL clause that looks like "(a, b) > (?, ?)"
 
     Args:
-        database_engine
+        _database_engine
         keys: A set of (column, value) pairs to be compared.
 
     Returns:
         A tuple of SQL query and the args
     """
-    if database_engine.supports_tuple_comparison:
-        return (
-            "(%s) > (%s)" % (",".join(k[0] for k in keys), ",".join("?" for _ in keys)),
-            [k[1] for k in keys],
-        )
-
-    # we want to build a clause
-    #    (a > ?) OR
-    #    (a == ? AND b > ?) OR
-    #    (a == ? AND b == ? AND c > ?)
-    #    ...
-    #    (a == ? AND b == ? AND ... AND z > ?)
-    #
-    # or, equivalently:
-    #
-    #  (a > ? OR (a == ? AND
-    #    (b > ? OR (b == ? AND
-    #      ...
-    #        (y > ? OR (y == ? AND
-    #          z > ?
-    #        ))
-    #      ...
-    #    ))
-    #  ))
-    #
-    # which itself is equivalent to (and apparently easier for the query optimiser):
-    #
-    #  (a >= ? AND (a > ? OR
-    #    (b >= ? AND (b > ? OR
-    #      ...
-    #        (y >= ? AND (y > ? OR
-    #          z > ?
-    #        ))
-    #      ...
-    #    ))
-    #  ))
-    #
-    #
-
-    clause = ""
-    args = []  # type: List[KV]
-    for k, v in keys[:-1]:
-        clause = clause + "(%s >= ? AND (%s > ? OR " % (k, k)
-        args.extend([v, v])
-
-    (k, v) = keys[-1]
-    clause += "%s > ?" % (k,)
-    args.append(v)
-
-    clause += "))" * (len(keys) - 1)
-    return clause, args
+    return (
+        "(%s) > (%s)" % (",".join(k[0] for k in keys), ",".join("?" for _ in keys)),
+        [k[1] for k in keys],
+    )
diff --git a/synapse/storage/engines/_base.py b/synapse/storage/engines/_base.py
index cca839c70f..21db1645d3 100644
--- a/synapse/storage/engines/_base.py
+++ b/synapse/storage/engines/_base.py
@@ -44,14 +44,6 @@ class BaseDatabaseEngine(Generic[ConnectionType], metaclass=abc.ABCMeta):
 
     @property
     @abc.abstractmethod
-    def supports_tuple_comparison(self) -> bool:
-        """
-        Do we support comparing tuples, i.e. `(a, b) > (c, d)`?
-        """
-        ...
-
-    @property
-    @abc.abstractmethod
     def supports_using_any_list(self) -> bool:
         """
         Do we support using `a = ANY(?)` and passing a list
diff --git a/synapse/storage/engines/postgres.py b/synapse/storage/engines/postgres.py
index d95f88b3e9..dba8cc51d3 100644
--- a/synapse/storage/engines/postgres.py
+++ b/synapse/storage/engines/postgres.py
@@ -130,13 +130,6 @@ class PostgresEngine(BaseDatabaseEngine):
         return True
 
     @property
-    def supports_tuple_comparison(self):
-        """
-        Do we support comparing tuples, i.e. `(a, b) > (c, d)`?
-        """
-        return True
-
-    @property
     def supports_using_any_list(self):
         """Do we support using `a = ANY(?)` and passing a list"""
         return True
diff --git a/synapse/storage/engines/sqlite.py b/synapse/storage/engines/sqlite.py
index b87e7798da..f4f16456f2 100644
--- a/synapse/storage/engines/sqlite.py
+++ b/synapse/storage/engines/sqlite.py
@@ -57,14 +57,6 @@ class Sqlite3Engine(BaseDatabaseEngine["sqlite3.Connection"]):
         return self.module.sqlite_version_info >= (3, 24, 0)
 
     @property
-    def supports_tuple_comparison(self):
-        """
-        Do we support comparing tuples, i.e. `(a, b) > (c, d)`? This requires
-        SQLite 3.15+.
-        """
-        return self.module.sqlite_version_info >= (3, 15, 0)
-
-    @property
     def supports_using_any_list(self):
         """Do we support using `a = ANY(?)` and passing a list"""
         return False
@@ -72,8 +64,11 @@ class Sqlite3Engine(BaseDatabaseEngine["sqlite3.Connection"]):
     def check_database(self, db_conn, allow_outdated_version: bool = False):
         if not allow_outdated_version:
             version = self.module.sqlite_version_info
-            if version < (3, 11, 0):
-                raise RuntimeError("Synapse requires sqlite 3.11 or above.")
+            # Synapse is untested against older SQLite versions, and we don't want
+            # to let users upgrade to a version of Synapse with broken support for their
+            # sqlite version, because it risks leaving them with a half-upgraded db.
+            if version < (3, 22, 0):
+                raise RuntimeError("Synapse requires sqlite 3.22 or above.")
 
     def check_new_database(self, txn):
         """Gets called when setting up a brand new database. This allows us to
diff --git a/tests/storage/test_database.py b/tests/storage/test_database.py
index 5a77c84962..1bba58fc03 100644
--- a/tests/storage/test_database.py
+++ b/tests/storage/test_database.py
@@ -36,17 +36,7 @@ def _stub_db_engine(**kwargs) -> BaseDatabaseEngine:
 
 class TupleComparisonClauseTestCase(unittest.TestCase):
     def test_native_tuple_comparison(self):
-        db_engine = _stub_db_engine(supports_tuple_comparison=True)
+        db_engine = _stub_db_engine()
         clause, args = make_tuple_comparison_clause(db_engine, [("a", 1), ("b", 2)])
         self.assertEqual(clause, "(a,b) > (?,?)")
         self.assertEqual(args, [1, 2])
-
-    def test_emulated_tuple_comparison(self):
-        db_engine = _stub_db_engine(supports_tuple_comparison=False)
-        clause, args = make_tuple_comparison_clause(
-            db_engine, [("a", 1), ("b", 2), ("c", 3)]
-        )
-        self.assertEqual(
-            clause, "(a >= ? AND (a > ? OR (b >= ? AND (b > ? OR c > ?))))"
-        )
-        self.assertEqual(args, [1, 1, 2, 2, 3])