-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add foreign key constraint to event_forward_extremities
.
#15751
Conversation
b680880
to
3d9adb0
Compare
|
state: State = State.validate | ||
lower_bound: Sequence[Any] = () |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drive-by: are these types being used for validation or is it "just" a dataclass? (If the former, recommend a short unit test to sanity check that the validation behaves as you expect)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ermh, it's mostly just used as a data class I think? So long as the pydantic models correctly round trip it should be fine?
This should help make it easier to add NOT NULL and FOREIGN KEY constraints to existing tables.
ccfe37a
to
0fe647a
Compare
FORWARD_EXTREMITIES_TABLE_SCHEMA = """ | ||
CREATE TABLE event_forward_extremities2( | ||
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) | ||
); | ||
""" | ||
|
||
FORWARD_EXTREMITIES_INDICES_SCHEMA = """ | ||
CREATE INDEX ev_extrem_room ON event_forward_extremities(room_id); | ||
CREATE INDEX ev_extrem_id ON event_forward_extremities(event_id); | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a more automatic way we can copy things instead of relying on our copy-paste skills here to make sure it's right?
Simple, stupid way is also good but I feel like there might be a time when we overlook and miss an index in the future. Or a race with PRs where some new column is added but isn't considered in this background update.
We can kick that can down the road though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, probably using the special sqlite_schema
table. Let me give it a go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have changed it to automatically port the indexes for SQLite, but I can't see a nice way of automating the table creation without having to parse the SQL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fancy!!
From some Googling as well, doesn't seem possible for SQLite out of box 👍
With Postgres, we could use CREATE TABLE new_table LIKE old_table;
(reference) which is exactly what we want to copy the schema/structure only.
SQLite does have CREATE TABLE ... AS SELECT
but it doesn't copy the primary key, constraints, or defaults. Kinda sucks as well because it populates the table as well even if we LIMIT
the SELECT
. Even though, it doesn't copy constraints (I assume that means indexes as well) but we could work around it with the code you already have to copy those over. Perhaps we could overcome all of these problems by applying all indexes/constraints (currently the built-in ones are excluded with AND sql IS NOT NULL
)
synapse/synapse/storage/background_updates.py
Lines 1103 to 1110 in 2451d8d
# Fetch the indexes/triggers/etc. Note that `sql` column being null is | |
# due to indexes being auto created based on the class definition (e.g. | |
# PRIMARY KEY), and so don't need to be recreated. | |
txn.execute( | |
""" | |
SELECT sql FROM sqlite_master | |
WHERE tbl_name = ? AND type != 'table' AND sql IS NOT NULL | |
""", |
synapse/storage/schema/main/delta/78/03event_extremities_constraints.py
Outdated
Show resolved
Hide resolved
synapse/storage/schema/main/delta/78/03event_extremities_constraints.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(approved in case you want to move on without #15751 (comment), we can iterate on that later)
Thanks! I think its good enough for now, as the biggest footgun was forgetting to port over some indexes, which is harder to do now. |
This release - raises the minimum supported version of Python to 3.8, as Python 3.7 is now [end-of-life](https://devguide.python.org/versions/), and - removes deprecated config options related to worker deployment. See [the upgrade notes](https://github.com/matrix-org/synapse/blob/release-v1.88/docs/upgrade.md#upgrading-to-v1880) for more information. - Revert "Stop writing to column `user_id` of tables `profiles` and `user_filters`", which was introduced in Synapse 1.88.0rc1. ([\matrix-org#15953](matrix-org#15953)) - Add `not_user_type` param to the [list accounts admin API](https://matrix-org.github.io/synapse/v1.88/admin_api/user_admin_api.html#list-accounts). ([\matrix-org#15844](matrix-org#15844)) - Pin `pydantic` to `^=1.7.4` to avoid backwards-incompatible API changes from the 2.0.0 release. Contributed by @PaarthShah. ([\matrix-org#15862](matrix-org#15862)) - Correctly resize thumbnails with pillow version >=10. ([\matrix-org#15876](matrix-org#15876)) - Fixed header levels on the [Admin API "Users"](https://matrix-org.github.io/synapse/v1.87/admin_api/user_admin_api.html) documentation page. Contributed by @sumnerevans at @beeper. ([\matrix-org#15852](matrix-org#15852)) - Remove deprecated `worker_replication_host`, `worker_replication_http_port` and `worker_replication_http_tls` configuration options. ([\matrix-org#15872](matrix-org#15872)) - **Remove deprecated `worker_replication_host`, `worker_replication_http_port` and `worker_replication_http_tls` configuration options.** See the [upgrade notes](https://github.com/matrix-org/synapse/blob/release-v1.88/docs/upgrade.md#removal-of-worker_replication_-settings) for more details. ([\matrix-org#15860](matrix-org#15860)) - Remove support for Python 3.7 and hence for Debian Buster. ([\matrix-org#15851](matrix-org#15851), [\matrix-org#15892](matrix-org#15892), [\matrix-org#15893](matrix-org#15893), [\matrix-org#15917](matrix-org#15917)) - Add foreign key constraint to `event_forward_extremities`. ([\matrix-org#15751](matrix-org#15751), [\matrix-org#15907](matrix-org#15907)) - Add read/write style cross-worker locks. ([\matrix-org#15782](matrix-org#15782)) - Stop writing to column `user_id` of tables `profiles` and `user_filters`. ([\matrix-org#15787](matrix-org#15787)) - Use lower isolation level when cleaning old presence stream data to avoid serialization errors. ([\matrix-org#15826](matrix-org#15826)) - Add tracing to media `/upload` code paths. ([\matrix-org#15850](matrix-org#15850), [\matrix-org#15888](matrix-org#15888)) - Add a timeout that aborts any Postgres statement taking more than 1 hour. ([\matrix-org#15853](matrix-org#15853)) - Fix the `devenv up` configuration which was ignoring the config overrides. ([\matrix-org#15854](matrix-org#15854)) - Optimised cleanup of old entries in `device_lists_stream`. ([\matrix-org#15861](matrix-org#15861)) - Update the Matrix clients link in the _It works! Synapse is running_ landing page. ([\matrix-org#15874](matrix-org#15874)) - Fix building Synapse with the nightly Rust compiler. ([\matrix-org#15906](matrix-org#15906)) - Add `Server` to Access-Control-Expose-Headers header. ([\matrix-org#15908](matrix-org#15908)) * Bump authlib from 1.2.0 to 1.2.1. ([\matrix-org#15864](matrix-org#15864)) * Bump importlib-metadata from 6.6.0 to 6.7.0. ([\matrix-org#15865](matrix-org#15865)) * Bump lxml from 4.9.2 to 4.9.3. ([\matrix-org#15897](matrix-org#15897)) * Bump regex from 1.8.4 to 1.9.1. ([\matrix-org#15902](matrix-org#15902)) * Bump ruff from 0.0.275 to 0.0.277. ([\matrix-org#15900](matrix-org#15900)) * Bump sentry-sdk from 1.25.1 to 1.26.0. ([\matrix-org#15867](matrix-org#15867)) * Bump serde_json from 1.0.99 to 1.0.100. ([\matrix-org#15901](matrix-org#15901)) * Bump types-pyopenssl from 23.2.0.0 to 23.2.0.1. ([\matrix-org#15866](matrix-org#15866))
This release - raises the minimum supported version of Python to 3.8, as Python 3.7 is now [end-of-life](https://devguide.python.org/versions/), and - removes deprecated config options related to worker deployment. See [the upgrade notes](https://github.com/matrix-org/synapse/blob/release-v1.88/docs/upgrade.md#upgrading-to-v1880) for more information. - Revert "Stop writing to column `user_id` of tables `profiles` and `user_filters`", which was introduced in Synapse 1.88.0rc1. ([\matrix-org#15953](matrix-org#15953)) - Add `not_user_type` param to the [list accounts admin API](https://matrix-org.github.io/synapse/v1.88/admin_api/user_admin_api.html#list-accounts). ([\matrix-org#15844](matrix-org#15844)) - Pin `pydantic` to `^=1.7.4` to avoid backwards-incompatible API changes from the 2.0.0 release. Contributed by @PaarthShah. ([\matrix-org#15862](matrix-org#15862)) - Correctly resize thumbnails with pillow version >=10. ([\matrix-org#15876](matrix-org#15876)) - Fixed header levels on the [Admin API "Users"](https://matrix-org.github.io/synapse/v1.87/admin_api/user_admin_api.html) documentation page. Contributed by @sumnerevans at @beeper. ([\matrix-org#15852](matrix-org#15852)) - Remove deprecated `worker_replication_host`, `worker_replication_http_port` and `worker_replication_http_tls` configuration options. ([\matrix-org#15872](matrix-org#15872)) - **Remove deprecated `worker_replication_host`, `worker_replication_http_port` and `worker_replication_http_tls` configuration options.** See the [upgrade notes](https://github.com/matrix-org/synapse/blob/release-v1.88/docs/upgrade.md#removal-of-worker_replication_-settings) for more details. ([\matrix-org#15860](matrix-org#15860)) - Remove support for Python 3.7 and hence for Debian Buster. ([\matrix-org#15851](matrix-org#15851), [\matrix-org#15892](matrix-org#15892), [\matrix-org#15893](matrix-org#15893), [\matrix-org#15917](matrix-org#15917)) - Add foreign key constraint to `event_forward_extremities`. ([\matrix-org#15751](matrix-org#15751), [\matrix-org#15907](matrix-org#15907)) - Add read/write style cross-worker locks. ([\matrix-org#15782](matrix-org#15782)) - Stop writing to column `user_id` of tables `profiles` and `user_filters`. ([\matrix-org#15787](matrix-org#15787)) - Use lower isolation level when cleaning old presence stream data to avoid serialization errors. ([\matrix-org#15826](matrix-org#15826)) - Add tracing to media `/upload` code paths. ([\matrix-org#15850](matrix-org#15850), [\matrix-org#15888](matrix-org#15888)) - Add a timeout that aborts any Postgres statement taking more than 1 hour. ([\matrix-org#15853](matrix-org#15853)) - Fix the `devenv up` configuration which was ignoring the config overrides. ([\matrix-org#15854](matrix-org#15854)) - Optimised cleanup of old entries in `device_lists_stream`. ([\matrix-org#15861](matrix-org#15861)) - Update the Matrix clients link in the _It works! Synapse is running_ landing page. ([\matrix-org#15874](matrix-org#15874)) - Fix building Synapse with the nightly Rust compiler. ([\matrix-org#15906](matrix-org#15906)) - Add `Server` to Access-Control-Expose-Headers header. ([\matrix-org#15908](matrix-org#15908)) * Bump authlib from 1.2.0 to 1.2.1. ([\matrix-org#15864](matrix-org#15864)) * Bump importlib-metadata from 6.6.0 to 6.7.0. ([\matrix-org#15865](matrix-org#15865)) * Bump lxml from 4.9.2 to 4.9.3. ([\matrix-org#15897](matrix-org#15897)) * Bump regex from 1.8.4 to 1.9.1. ([\matrix-org#15902](matrix-org#15902)) * Bump ruff from 0.0.275 to 0.0.277. ([\matrix-org#15900](matrix-org#15900)) * Bump sentry-sdk from 1.25.1 to 1.26.0. ([\matrix-org#15867](matrix-org#15867)) * Bump serde_json from 1.0.99 to 1.0.100. ([\matrix-org#15901](matrix-org#15901)) * Bump types-pyopenssl from 23.2.0.0 to 23.2.0.1. ([\matrix-org#15866](matrix-org#15866)) # -----BEGIN PGP SIGNATURE----- # # iQIzBAABCAAdFiEE8SRSDO7gYkSP4chELS76LzL74EcFAmS2ncYACgkQLS76LzL7 # 4EfaiA/9HUdk1tIlnQyIZDAT0d9RhmaL+KL1Fkk4wpi16QrtJ7gqLTrq1BXDxOYM # 2bycL0yHN9gPu3f7TI0ic4p17T/vud8Fd7FoPJTkUZ7dFHsEPICtNmIp6ZkpuDYW # Sv8QKuEeMxe98XCKxiI+zctu8wtNsrnu2RECD0zUqf5rMgbabuYnpSge2CqKftuf # CfmYN161QjnONavQTk4iYSFmJpRZwvwoAlpMPsqkMIrhId2ko2SkPj1HBPrAFrFs # Fq/PaVZQRZk15wnQzrLrlRAEfq/quoOSDnJSlUvMPWjsQUVP8Ug149L9oUIiwhqq # zQtuL9dl5xGoUWMiWOP8937gVeA/lsJpcVPka3G0g3mIR8ukbHUOm2fZReV30xp8 # 81xpu8KwzDR+/Oo3INYsqoOiQ/t7Myg8sTgiJBMParuRmfqnsbdUWG8pEeUMzDYY # 4+yzRrHo9KnHcAMEFT94rqVhgl7OQh2Fx9zduW7YOVk0wo0EBMl/rcfEsvvl//l0 # sSykqGx1XIr3Cdynp1PjCYJsYdLJzG73aSVh5qPY5sftsHIQ8DiiKX+UlSqlguMW # 1ndkCuz3fK/+9CFGbBiixe/RKFxn0CVp3cBU6cCVzyLJerZpXrOkyMmSuZtOuIaH # cLLGK2bQSbOegPtg4qjpL537xmk94tUiSGhEdt7M4wsHm4uRv0M= # =QYwZ # -----END PGP SIGNATURE----- # gpg: Signature made Tue Jul 18 15:12:22 2023 BST # gpg: using RSA key F124520CEEE062448FE1C8442D2EFA2F32FBE047 # gpg: Can't check signature: No public key # Conflicts: # .github/workflows/latest_deps.yml # .github/workflows/release-artifacts.yml # .github/workflows/tests.yml # .github/workflows/twisted_trunk.yml # docker/Dockerfile # poetry.lock # synapse/api/auth/base.py # synapse/config/experimental.py # synapse/handlers/pagination.py # synapse/handlers/room_batch.py # synapse/push/bulk_push_rule_evaluator.py # synapse/rest/admin/users.py # synapse/rest/client/room_batch.py # synapse/storage/databases/main/__init__.py
The bulk of this PR is adding helper functions to make it easier to add constraints.
This is to help with #13476
Reviewable commit-by-commit.