This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Stop writing to column user_id
of tables profiles
and user_filters
#16181
Open
H-Shay
wants to merge
6
commits into
develop
Choose a base branch
from
shay/stop_writing_user_id
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
890bfb4
stop writing to column `user_id` of table `user_filters`
H-Shay f6401d7
stop writing to column `user_id` of table `profiles`
H-Shay 0d33baf
drop not null constraint on column `user_id` of tables `profiles` and…
H-Shay 6470011
allow for upserts to column `full_user_id` of table `profiles` and `u…
H-Shay fd6bd53
drop conflicting tests
H-Shay 095be7b
newsfragment
H-Shay File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Stop writing to column user_id of tables profiles and user_filters. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
64 changes: 64 additions & 0 deletions
64
synapse/storage/schema/main/delta/81/01_drop_user_id_constraint_profiles.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
# Copyright 2023 The Matrix.org Foundation C.I.C | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
from synapse.storage.database import LoggingTransaction | ||
from synapse.storage.engines import BaseDatabaseEngine, PostgresEngine | ||
|
||
|
||
def run_create(cur: LoggingTransaction, database_engine: BaseDatabaseEngine) -> None: | ||
""" | ||
Update to drop the NOT NULL constraint on column `user_id` so that we can cease to | ||
write to it without inserts to other columns triggering the constraint | ||
""" | ||
|
||
if isinstance(database_engine, PostgresEngine): | ||
drop_sql = """ | ||
ALTER TABLE profiles ALTER COLUMN user_id DROP NOT NULL | ||
""" | ||
cur.execute(drop_sql) | ||
else: | ||
# irritatingly in SQLite we need to rewrite the table to drop the constraint. | ||
cur.execute("DROP TABLE IF EXISTS temp_profiles") | ||
|
||
create_sql = """ | ||
CREATE TABLE temp_profiles ( | ||
full_user_id text NOT NULL, | ||
user_id text, | ||
displayname text, | ||
avatar_url text, | ||
UNIQUE (full_user_id), | ||
UNIQUE (user_id) | ||
) | ||
""" | ||
cur.execute(create_sql) | ||
|
||
copy_sql = """ | ||
INSERT INTO temp_profiles ( | ||
user_id, | ||
displayname, | ||
avatar_url, | ||
full_user_id) | ||
SELECT user_id, displayname, avatar_url, full_user_id FROM profiles | ||
""" | ||
cur.execute(copy_sql) | ||
|
||
drop_sql = """ | ||
DROP TABLE profiles | ||
""" | ||
cur.execute(drop_sql) | ||
|
||
rename_sql = """ | ||
ALTER TABLE temp_profiles RENAME to profiles | ||
""" | ||
cur.execute(rename_sql) |
68 changes: 68 additions & 0 deletions
68
synapse/storage/schema/main/delta/81/02_drop_user_id_constraint_user_filters.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
# Copyright 2023 The Matrix.org Foundation C.I.C | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
from synapse.storage.database import LoggingTransaction | ||
from synapse.storage.engines import BaseDatabaseEngine, PostgresEngine | ||
|
||
|
||
def run_create(cur: LoggingTransaction, database_engine: BaseDatabaseEngine) -> None: | ||
""" | ||
Update to drop the NOT NULL constraint on column user_id so that we can cease to | ||
write to it without inserts to other columns triggering the constraint | ||
""" | ||
if isinstance(database_engine, PostgresEngine): | ||
drop_sql = """ | ||
ALTER TABLE user_filters ALTER COLUMN user_id DROP NOT NULL | ||
""" | ||
cur.execute(drop_sql) | ||
|
||
else: | ||
# irritatingly in SQLite we need to rewrite the table to drop the constraint. | ||
cur.execute("DROP TABLE IF EXISTS temp_user_filters") | ||
|
||
create_sql = """ | ||
CREATE TABLE temp_user_filters ( | ||
full_user_id text NOT NULL, | ||
user_id text, | ||
filter_id bigint NOT NULL, | ||
filter_json bytea NOT NULL | ||
) | ||
""" | ||
cur.execute(create_sql) | ||
|
||
index_sql = """ | ||
CREATE UNIQUE INDEX IF NOT EXISTS user_filters_full_user_id_unique ON | ||
temp_user_filters (full_user_id, filter_id) | ||
""" | ||
cur.execute(index_sql) | ||
|
||
copy_sql = """ | ||
INSERT INTO temp_user_filters ( | ||
user_id, | ||
filter_id, | ||
filter_json, | ||
full_user_id) | ||
SELECT user_id, filter_id, filter_json, full_user_id FROM user_filters | ||
""" | ||
cur.execute(copy_sql) | ||
|
||
drop_sql = """ | ||
DROP TABLE user_filters | ||
""" | ||
cur.execute(drop_sql) | ||
|
||
rename_sql = """ | ||
ALTER TABLE temp_user_filters RENAME to user_filters | ||
""" | ||
cur.execute(rename_sql) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
The rationale for dropping these tests is that they both were testing background updates that have already run, and the changes to the code in this PR result in making it pretty impossible to run the tests in sqlite. When I added these tests I felt like they were pretty borderline useful (basically only as a proof of concept) so I think it makes sense to drop them now.