Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Rewrite the user_filter migration again #6184

Merged
merged 2 commits into from
Oct 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/6184.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Update `user_filters` table to have a unique index, and non-null columns. Thanks to @pik for contributing this.
58 changes: 32 additions & 26 deletions synapse/storage/schema/delta/56/unique_user_filter_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,42 +5,48 @@
logger = logging.getLogger(__name__)


"""
This migration updates the user_filters table as follows:

- drops any (user_id, filter_id) duplicates
- makes the columns NON-NULLable
- turns the index into a UNIQUE index
"""


def run_upgrade(cur, database_engine, *args, **kwargs):
pass


def run_create(cur, database_engine, *args, **kwargs):
if isinstance(database_engine, PostgresEngine):
select_clause = """
CREATE TEMPORARY TABLE user_filters_migration AS
SELECT DISTINCT ON (user_id, filter_id) user_id, filter_id, filter_json
FROM user_filters;
FROM user_filters
"""
else:
select_clause = """
CREATE TEMPORARY TABLE user_filters_migration AS
SELECT * FROM user_filters GROUP BY user_id, filter_id;
SELECT * FROM user_filters GROUP BY user_id, filter_id
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this work for both postgres and sqlite now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, postgres complains because filter_json is neither in the group by nor explicitly aggregated. I considered futzing around with something to pick an arbitrary filter_json from the results, but in the end decided to stick with pik's solution.

"""
sql = (
"""
BEGIN;
%s
DROP INDEX user_filters_by_user_id_filter_id;
DELETE FROM user_filters;
ALTER TABLE user_filters
ALTER COLUMN user_id SET NOT NULL,
ALTER COLUMN filter_id SET NOT NULL,
ALTER COLUMN filter_json SET NOT NULL;
INSERT INTO user_filters(user_id, filter_id, filter_json)
SELECT * FROM user_filters_migration;
DROP TABLE user_filters_migration;
CREATE UNIQUE INDEX user_filters_by_user_id_filter_id_unique
ON user_filters(user_id, filter_id);
END;
"""
% select_clause
sql = """
DROP TABLE IF EXISTS user_filters_migration;
DROP INDEX IF EXISTS user_filters_unique;
CREATE TABLE user_filters_migration (
user_id TEXT NOT NULL,
filter_id BIGINT NOT NULL,
filter_json BYTEA NOT NULL
);
INSERT INTO user_filters_migration (user_id, filter_id, filter_json)
%s;
CREATE UNIQUE INDEX user_filters_unique ON user_filters_migration
(user_id, filter_id);
DROP TABLE user_filters;
ALTER TABLE user_filters_migration RENAME TO user_filters;
""" % (
select_clause,
)

if isinstance(database_engine, PostgresEngine):
cur.execute(sql)
else:
cur.executescript(sql)


def run_create(cur, database_engine, *args, **kwargs):
pass