mirror of
https://github.com/element-hq/synapse.git
synced 2026-07-19 15:57:30 +00:00
Erik JohnstonandGitHub
87439e55b5
A handful of places in the storage layer bound a value whose Python type didn't match the declared column type — an `int` into a `TEXT` column, or a `str` into a `BIGINT` column — and relied on psycopg2's loose coercion to paper over the mismatch. These are latent correctness bugs: they only work because the driver silently converts, and a stricter driver that binds typed parameters rejects them outright. Found during a Rust port of the database pool where the driver does not coerce automatically. Each fix binds the value with the type the column actually declares, rather than depending on driver-specific coercion. All changes are behaviour-preserving on psycopg2 and sqlite. There is also a fix to the multi-writer id-gen tests where we forgot to commit. This is tangential, but was found during the same effort. ### Changes - **`device_lists_remote_extremeties.stream_id (TEXT)`** — `_update_remote_device_list_cache_txn` (typed `stream_id: int`) bound an int; store it as a string, matching the column and the sibling `_update_remote_device_list_cache_entry_txn` (typed `stream_id: str`). The old mismatch, when rejected, was swallowed inside the device-list resync and hung `query_devices`. - **`user_filters.filter_id (BIGINT)`** — `get_user_filter` bound the raw `int | str` (a string, from sync requests). It already validates via `int(filter_id)`; bind that int so it matches the column. - **`rejections.last_check (TEXT)`** — both writers stored `clock.time_msec()` (an int); store the timestamp as a string. - **user-directory temp position** — the `populate_user_directory` background update's staging column `_temp_populate_user_directory_position.position` was `TEXT` but held an int (read back into a `BIGINT` column). Declare it `BIGINT`. The temp table is created and dropped within the background update, so there's no migration. - **`test_batched_state_group_storing`** — selected from `state_group_edges` with a stringified `state_group`; bind the int directly (the column is an integer). - **Multi-writer id-generator tests** — constructing a `MultiWriterIdGenerator` prunes stale `stream_positions` rows, but the harness never committed, so the cleanup only survived because adbapi keeps one connection per thread with its transaction open. Commit after construction so it persists regardless of pool semantics (the delete is legitimate work that should be committed anyway).