From 6cbe60f896880ec644c35d1e2932b3aaf10c55dc Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 10 Jul 2023 11:39:12 +0100 Subject: [PATCH 1/2] Fix downgrading to previous version of Synapse We do this by marking the constraint as deferrable. --- synapse/storage/background_updates.py | 7 ++++++- synapse/storage/databases/main/event_federation.py | 4 +++- .../main/delta/78/03event_extremities_constraints.py | 10 ++++++++-- tests/storage/test_background_update.py | 8 ++++++-- 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/synapse/storage/background_updates.py b/synapse/storage/background_updates.py index 5dce0a01599d..2d5ddc3e7be4 100644 --- a/synapse/storage/background_updates.py +++ b/synapse/storage/background_updates.py @@ -80,10 +80,14 @@ class ForeignKeyConstraint(Constraint): Attributes: referenced_table: The "parent" table name. columns: The list of mappings of columns from table to referenced table + deferred: Whether to defer checking of the constraint to the end of the + transaction. This is useful for e.g. backwards compatibility where + an older version inserted data in the wrong order. """ referenced_table: str columns: Sequence[Tuple[str, str]] + deferred: bool def make_check_clause(self, table: str) -> str: join_clause = " AND ".join( @@ -94,7 +98,8 @@ def make_check_clause(self, table: str) -> str: def make_constraint_clause_postgres(self) -> str: column1_list = ", ".join(col1 for col1, col2 in self.columns) column2_list = ", ".join(col2 for col1, col2 in self.columns) - return f"FOREIGN KEY ({column1_list}) REFERENCES {self.referenced_table} ({column2_list})" + defer_clause = " DEFERRABLE INITIALLY DEFERRED" if self.deferred else "" + return f"FOREIGN KEY ({column1_list}) REFERENCES {self.referenced_table} ({column2_list}) {defer_clause}" @attr.s(auto_attribs=True) diff --git a/synapse/storage/databases/main/event_federation.py b/synapse/storage/databases/main/event_federation.py index dabe603c8cba..b2cda52ce593 100644 --- a/synapse/storage/databases/main/event_federation.py +++ b/synapse/storage/databases/main/event_federation.py @@ -146,7 +146,9 @@ def __init__( update_name="event_forward_extremities_event_id_foreign_key_constraint_update", table="event_forward_extremities", constraint_name="event_forward_extremities_event_id", - constraint=ForeignKeyConstraint("events", [("event_id", "event_id")]), + constraint=ForeignKeyConstraint( + "events", [("event_id", "event_id")], deferred=True + ), unique_columns=("event_id", "room_id"), ) diff --git a/synapse/storage/schema/main/delta/78/03event_extremities_constraints.py b/synapse/storage/schema/main/delta/78/03event_extremities_constraints.py index f12e2a8f3ee9..bf8c57dbe8ba 100644 --- a/synapse/storage/schema/main/delta/78/03event_extremities_constraints.py +++ b/synapse/storage/schema/main/delta/78/03event_extremities_constraints.py @@ -28,19 +28,25 @@ event_id TEXT NOT NULL, room_id TEXT NOT NULL, UNIQUE (event_id, room_id), - CONSTRAINT event_forward_extremities_event_id FOREIGN KEY (event_id) REFERENCES events (event_id) + CONSTRAINT event_forward_extremities_event_id FOREIGN KEY (event_id) REFERENCES events (event_id) DEFERRABLE INITIALLY DEFERRED ) """ def run_create(cur: LoggingTransaction, database_engine: BaseDatabaseEngine) -> None: + # We mark this as a deferred constraint, as the previous version of Synapse + # inserted the event into the forward extremities *before* the events table. + # By marking as deferred we ensure that downgrading to the previous version + # will continue to work. run_validate_constraint_and_delete_rows_schema_delta( cur, ordering=7803, update_name="event_forward_extremities_event_id_foreign_key_constraint_update", table="event_forward_extremities", constraint_name="event_forward_extremities_event_id", - constraint=ForeignKeyConstraint("events", [("event_id", "event_id")]), + constraint=ForeignKeyConstraint( + "events", [("event_id", "event_id")], deferred=True + ), sqlite_table_name="event_forward_extremities2", sqlite_table_schema=FORWARD_EXTREMITIES_TABLE_SCHEMA, ) diff --git a/tests/storage/test_background_update.py b/tests/storage/test_background_update.py index 6ca546f3f76a..a4a823a25242 100644 --- a/tests/storage/test_background_update.py +++ b/tests/storage/test_background_update.py @@ -586,7 +586,9 @@ def delta(txn: LoggingTransaction) -> None: update_name="test_bg_update", table="test_constraint", constraint_name="test_constraint_name", - constraint=ForeignKeyConstraint("base_table", [("b", "b")]), + constraint=ForeignKeyConstraint( + "base_table", [("b", "b")], deferred=False + ), sqlite_table_name="test_constraint2", sqlite_table_schema=table2_sqlite, ) @@ -604,7 +606,9 @@ def delta(txn: LoggingTransaction) -> None: "test_bg_update", table="test_constraint", constraint_name="test_constraint_name", - constraint=ForeignKeyConstraint("base_table", [("b", "b")]), + constraint=ForeignKeyConstraint( + "base_table", [("b", "b")], deferred=False + ), unique_columns=["a"], ) From 0af82ad8a919b95e707dddf00d1444faadb1fb14 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 10 Jul 2023 11:41:01 +0100 Subject: [PATCH 2/2] Newsfile --- changelog.d/15907.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/15907.misc diff --git a/changelog.d/15907.misc b/changelog.d/15907.misc new file mode 100644 index 000000000000..e0ecea6c2fdb --- /dev/null +++ b/changelog.d/15907.misc @@ -0,0 +1 @@ +Add foreign key constraint to `event_forward_extremities`.