diff --git a/changelog.d/20100.bugfix b/changelog.d/20100.bugfix new file mode 100644 index 0000000000..8f488be0bf --- /dev/null +++ b/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. diff --git a/synapse/storage/database.py b/synapse/storage/database.py index 023014276b..85a0e6ea93 100644 --- a/synapse/storage/database.py +++ b/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: diff --git a/tests/storage/test_database.py b/tests/storage/test_database.py index 6213abd753..82d0e4f5aa 100644 --- a/tests/storage/test_database.py +++ b/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