From f980ff267d9cd131e769bf87baf1b60acb1fbdd6 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Thu, 10 Nov 2022 10:06:37 +0000 Subject: [PATCH] Drop primary keys in favour of new indices --- .../storage/postgres/account_data_table.go | 6 ++--- userapi/storage/postgres/accounts_table.go | 5 ++-- .../deltas/2022110411000000_server_names.go | 17 +++++++++++++ userapi/storage/postgres/profile_table.go | 5 ++-- userapi/storage/sqlite3/account_data_table.go | 6 ++--- userapi/storage/sqlite3/accounts_table.go | 5 ++-- .../deltas/2022110411000000_server_names.go | 25 +++++++++++++++++++ userapi/storage/sqlite3/profile_table.go | 5 ++-- 8 files changed, 60 insertions(+), 14 deletions(-) diff --git a/userapi/storage/postgres/account_data_table.go b/userapi/storage/postgres/account_data_table.go index 07dba66b1..2a4777d74 100644 --- a/userapi/storage/postgres/account_data_table.go +++ b/userapi/storage/postgres/account_data_table.go @@ -36,10 +36,10 @@ CREATE TABLE IF NOT EXISTS userapi_account_datas ( -- The account data type type TEXT NOT NULL, -- The account data content - content TEXT NOT NULL, - - PRIMARY KEY(localpart, server_name, room_id, type) + content TEXT NOT NULL ); + +CREATE UNIQUE INDEX IF NOT EXISTS userapi_account_datas_idx ON userapi_account_datas(localpart, server_name, room_id, type); ` const insertAccountDataSQL = ` diff --git a/userapi/storage/postgres/accounts_table.go b/userapi/storage/postgres/accounts_table.go index 1512eb818..31a996527 100644 --- a/userapi/storage/postgres/accounts_table.go +++ b/userapi/storage/postgres/accounts_table.go @@ -46,11 +46,12 @@ CREATE TABLE IF NOT EXISTS userapi_accounts ( -- If the account is currently active is_deactivated BOOLEAN DEFAULT FALSE, -- The account_type (user = 1, guest = 2, admin = 3, appservice = 4) - account_type SMALLINT NOT NULL, + account_type SMALLINT NOT NULL -- TODO: -- upgraded_ts, devices, any email reset stuff? - PRIMARY KEY (localpart, server_name) ); + +CREATE UNIQUE INDEX IF NOT EXISTS userapi_accounts_idx ON userapi_accounts(localpart, server_name); ` const insertAccountSQL = "" + diff --git a/userapi/storage/postgres/deltas/2022110411000000_server_names.go b/userapi/storage/postgres/deltas/2022110411000000_server_names.go index c4e5c75fd..7a84188c1 100644 --- a/userapi/storage/postgres/deltas/2022110411000000_server_names.go +++ b/userapi/storage/postgres/deltas/2022110411000000_server_names.go @@ -20,6 +20,14 @@ var serverNamesTables = []string{ "userapi_threepids", } +// These tables have a PRIMARY KEY constraint which we need to drop so +// that we can recreate a new unique index that contains the server name. +var serverNamesDropPK = []string{ + "userapi_accounts", + "userapi_account_datas", + "userapi_profiles", +} + // I know what you're thinking: you're wondering "why doesn't this use $1 // and pass variadic parameters to ExecContext?" — the answer is because // PostgreSQL doesn't expect the table name to be specified as a substituted @@ -35,5 +43,14 @@ func UpServerNames(ctx context.Context, tx *sql.Tx, serverName gomatrixserverlib return fmt.Errorf("add server name to %q error: %w", table, err) } } + for _, table := range serverNamesDropPK { + q := fmt.Sprintf( + "ALTER TABLE IF EXISTS %s DROP CONSTRAINT %s;", + pq.QuoteIdentifier(table), pq.QuoteIdentifier(table+"_pkey"), + ) + if _, err := tx.ExecContext(ctx, q); err != nil { + return fmt.Errorf("drop PK from %q error: %w", table, err) + } + } return nil } diff --git a/userapi/storage/postgres/profile_table.go b/userapi/storage/postgres/profile_table.go index 0d0ff705c..df4e0db63 100644 --- a/userapi/storage/postgres/profile_table.go +++ b/userapi/storage/postgres/profile_table.go @@ -35,9 +35,10 @@ CREATE TABLE IF NOT EXISTS userapi_profiles ( -- The display name for this account display_name TEXT, -- The URL of the avatar for this account - avatar_url TEXT, - PRIMARY KEY (localpart, server_name) + avatar_url TEXT ); + +CREATE UNIQUE INDEX IF NOT EXISTS userapi_profiles_idx ON userapi_profiles(localpart, server_name); ` const insertProfileSQL = "" + diff --git a/userapi/storage/sqlite3/account_data_table.go b/userapi/storage/sqlite3/account_data_table.go index 4989b88e4..2fbdc5732 100644 --- a/userapi/storage/sqlite3/account_data_table.go +++ b/userapi/storage/sqlite3/account_data_table.go @@ -35,10 +35,10 @@ CREATE TABLE IF NOT EXISTS userapi_account_datas ( -- The account data type type TEXT NOT NULL, -- The account data content - content TEXT NOT NULL, - - PRIMARY KEY(localpart, server_name, room_id, type) + content TEXT NOT NULL ); + +CREATE UNIQUE INDEX IF NOT EXISTS userapi_account_datas_idx ON userapi_account_datas(localpart, server_name, room_id, type); ` const insertAccountDataSQL = ` diff --git a/userapi/storage/sqlite3/accounts_table.go b/userapi/storage/sqlite3/accounts_table.go index 861d794d9..f4ebe2158 100644 --- a/userapi/storage/sqlite3/accounts_table.go +++ b/userapi/storage/sqlite3/accounts_table.go @@ -45,11 +45,12 @@ CREATE TABLE IF NOT EXISTS userapi_accounts ( -- If the account is currently active is_deactivated BOOLEAN DEFAULT 0, -- The account_type (user = 1, guest = 2, admin = 3, appservice = 4) - account_type INTEGER NOT NULL, + account_type INTEGER NOT NULL -- TODO: -- upgraded_ts, devices, any email reset stuff? - PRIMARY KEY (localpart, server_name) ); + +CREATE UNIQUE INDEX IF NOT EXISTS userapi_accounts_idx ON userapi_accounts(localpart, server_name); ` const insertAccountSQL = "" + diff --git a/userapi/storage/sqlite3/deltas/2022110411000000_server_names.go b/userapi/storage/sqlite3/deltas/2022110411000000_server_names.go index 3ba43b2e6..74e1db811 100644 --- a/userapi/storage/sqlite3/deltas/2022110411000000_server_names.go +++ b/userapi/storage/sqlite3/deltas/2022110411000000_server_names.go @@ -20,6 +20,14 @@ var serverNamesTables = []string{ "userapi_threepids", } +// These tables have a PRIMARY KEY constraint which we need to drop so +// that we can recreate a new unique index that contains the server name. +var serverNamesDropPK = []string{ + "userapi_accounts", + "userapi_account_datas", + "userapi_profiles", +} + // I know what you're thinking: you're wondering "why doesn't this use $1 // and pass variadic parameters to ExecContext?" — the answer is because // PostgreSQL doesn't expect the table name to be specified as a substituted @@ -43,5 +51,22 @@ func UpServerNames(ctx context.Context, tx *sql.Tx, serverName gomatrixserverlib return fmt.Errorf("add server name to %q error: %w", table, err) } } + for _, table := range serverNamesDropPK { + q := fmt.Sprintf( + "SELECT COUNT(name) FROM sqlite_schema WHERE type='table' AND name=%s;", + pq.QuoteIdentifier(table), + ) + var c int + if err := tx.QueryRowContext(ctx, q).Scan(&c); err != nil || c == 0 { + continue + } + q = fmt.Sprintf( + "ALTER TABLE %s DROP PRIMARY KEY;", + pq.QuoteIdentifier(table), + ) + if _, err := tx.ExecContext(ctx, q); err != nil { + return fmt.Errorf("drop PK from %q error: %w", table, err) + } + } return nil } diff --git a/userapi/storage/sqlite3/profile_table.go b/userapi/storage/sqlite3/profile_table.go index 7b151adf6..867026d7a 100644 --- a/userapi/storage/sqlite3/profile_table.go +++ b/userapi/storage/sqlite3/profile_table.go @@ -35,9 +35,10 @@ CREATE TABLE IF NOT EXISTS userapi_profiles ( -- The display name for this account display_name TEXT, -- The URL of the avatar for this account - avatar_url TEXT, - PRIMARY KEY (localpart, server_name) + avatar_url TEXT ); + +CREATE UNIQUE INDEX IF NOT EXISTS userapi_profiles_idx ON userapi_profiles(localpart, server_name); ` const insertProfileSQL = "" +