Don't %-substitute SQL that is run without parameters.

`LoggingTransaction.execute` hands its (empty by default) parameter
collection to the driver unconditionally. psycopg2 `%`-substitutes the SQL
whenever it is given any parameters at all, so a statement containing a
literal `%` fails with `IndexError: tuple index out of range`.

This has been the case since #15432 changed the signature from
`execute(self, sql, *args)` to `execute(self, sql, parameters=())`, and it
means a Postgres database older than schema version 58 cannot be upgraded:

  * `main/delta/58/10_pushrules_enabled_delete_obsolete.sql` matches
    `LIKE 'global/%/.m.rule.%'`, and
  * the `event_fix_redactions_bytes` background update, scheduled by
    `main/delta/56/redaction_censor3_fix_update.sql.postgres`, matches
    `json NOT LIKE '{%'`.

Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>

changelog.d/20100.bugfix

@@ -0,0 +1 @@
+Fix schema deltas and background updates containing a literal `%` failing on PostgreSQL. Broken since Synapse v1.82.0.

synapse/storage/database.py

@@ -462,6 +462,17 @@ class LoggingTransaction:
         )
 
     def execute(self, sql: str, parameters: SQLQueryParameters = ()) -> None:
+        if not parameters:
+            # Don't hand an empty parameter collection to the driver.
+            #
+            # psycopg2 performs `%`-substitution on the SQL whenever it is given
+            # *any* parameters, including an empty collection, so a statement
+            # containing a literal `%` (e.g. `LIKE 'foo%'`) fails with
+            # `IndexError: tuple index out of range`. Omitting the parameters
+            # altogether disables the substitution.
+            self._do_execute(self.txn.execute, sql)
+            return
+
         self._do_execute(self.txn.execute, sql, parameters)
 
     def executemany(self, sql: str, *args: Any) -> None:

tests/storage/test_database.py

@@ -142,6 +142,42 @@ class ExecuteScriptTestCase(unittest.HomeserverTestCase):
         )
 
 
+class LiteralPercentTestCase(unittest.HomeserverTestCase):
+    """Tests that SQL containing a literal `%` can be run without parameters.
+
+    psycopg2 `%`-substitutes the SQL it is given whenever any parameters are
+    supplied, so passing it an empty collection of parameters makes it choke on
+    statements like `LIKE 'foo%'`. Several schema deltas and background updates
+    contain such statements.
+    """
+
+    def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
+        self.db_pool: DatabasePool = hs.get_datastores().main.db_pool
+
+    def test_execute(self) -> None:
+        """A literal `%` survives `LoggingTransaction.execute`."""
+
+        def run(txn: LoggingTransaction) -> None:
+            txn.execute("SELECT 'a/b' LIKE 'a/%'")
+            self.assertEqual(txn.fetchall(), [(True,)])
+
+        self.get_success(self.db_pool.runInteraction("test_execute", run))
+
+    def test_executescript(self) -> None:
+        """A literal `%` survives `BaseDatabaseEngine.executescript`.
+
+        This is the path taken by `.sql` schema deltas.
+        """
+
+        def run(conn: LoggingDatabaseConnection) -> None:
+            cur = conn.cursor(txn_name="test_executescript")
+            self.db_pool.engine.executescript(
+                cur, "CREATE TABLE percent_test (name TEXT); SELECT 'a/b' LIKE 'a/%';"
+            )
+
+        self.get_success(self.db_pool.runWithConnection(run))
+
+
 @attr.s(slots=True, auto_attribs=True)
 class TransactionMocks:
     after_callback: Mock