From 72a5d99ae37efe9ebb46946c99ccb5dd8d246472 Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Fri, 29 Apr 2022 14:42:38 -0300 Subject: [PATCH] MM-43770: Rename to Threads.ThreadDeleteAt (#20074) Old versions of the Mattermost server did not qualify queries scanning both `Posts` and `Threads`, and choke on the ambiguity in deciding between the new `DeleteAt` on `Threads` and the `DeleteAt` on `Posts` in existing queries. While this problem is transient only while running multiple server versions, it effectively makes our backwards compatibility guarantee void, not to mention complicating cloud deployments. Work around this by renaming `Threads.DeleteAt` to `Threads.ThreadDeleteAt`. The old migration is nulled out, but remains, since some test servers have already upgraded and manually fixing each affected instance would be problematic. Thew new migration takes care of removing the old column -- if it ever existed. Fixes: https://mattermost.atlassian.net/browse/MM-43770 Co-authored-by: Mattermod --- .../mysql/000081_threads_deleteat.down.sql | 15 +------- .../mysql/000081_threads_deleteat.up.sql | 20 +---------- .../000083_threads_threaddeleteat.down.sql | 14 ++++++++ .../000083_threads_threaddeleteat.up.sql | 35 +++++++++++++++++++ .../postgres/000081_threads_deleteat.down.sql | 2 +- .../postgres/000081_threads_deleteat.up.sql | 3 +- .../000083_threads_threaddeleteat.down.sql | 1 + .../000083_threads_threaddeleteat.up.sql | 5 +++ model/thread.go | 3 +- store/sqlstore/post_store.go | 4 +-- store/sqlstore/thread_store.go | 14 ++++---- 11 files changed, 70 insertions(+), 46 deletions(-) create mode 100644 db/migrations/mysql/000083_threads_threaddeleteat.down.sql create mode 100644 db/migrations/mysql/000083_threads_threaddeleteat.up.sql create mode 100644 db/migrations/postgres/000083_threads_threaddeleteat.down.sql create mode 100644 db/migrations/postgres/000083_threads_threaddeleteat.up.sql diff --git a/db/migrations/mysql/000081_threads_deleteat.down.sql b/db/migrations/mysql/000081_threads_deleteat.down.sql index 3bae8762bc..cacb45cb5a 100644 --- a/db/migrations/mysql/000081_threads_deleteat.down.sql +++ b/db/migrations/mysql/000081_threads_deleteat.down.sql @@ -1,14 +1 @@ -SET @preparedStatement = (SELECT IF( - EXISTS( - SELECT 1 FROM INFORMATION_SCHEMA.STATISTICS - WHERE table_name = 'Threads' - AND table_schema = DATABASE() - AND column_name = 'DeleteAt' - ) > 0, - 'ALTER TABLE Threads DROP COLUMN DeleteAt;', - 'SELECT 1;' -)); - -PREPARE removeColumnIfExists FROM @preparedStatement; -EXECUTE removeColumnIfExists; -DEALLOCATE PREPARE removeColumnIfExists; +-- Replaced by 000083_threads_threaddeleteat.down.sql diff --git a/db/migrations/mysql/000081_threads_deleteat.up.sql b/db/migrations/mysql/000081_threads_deleteat.up.sql index 578294be26..ceeffcc078 100644 --- a/db/migrations/mysql/000081_threads_deleteat.up.sql +++ b/db/migrations/mysql/000081_threads_deleteat.up.sql @@ -1,19 +1 @@ -SET @preparedStatement = (SELECT IF( - NOT EXISTS( - SELECT 1 FROM INFORMATION_SCHEMA.COLUMNS - WHERE table_name = 'Threads' - AND table_schema = DATABASE() - AND column_name = 'DeleteAt' - ), - 'ALTER TABLE Threads ADD COLUMN DeleteAt bigint(20);', - 'SELECT 1;' -)); - -PREPARE addColumnIfNotExists FROM @preparedStatement; -EXECUTE addColumnIfNotExists; -DEALLOCATE PREPARE addColumnIfNotExists; - -UPDATE Threads, Posts -SET Threads.DeleteAt = Posts.DeleteAt -WHERE Posts.Id = Threads.PostId -AND Threads.DeleteAt IS NULL; +-- Replaced by 000083_threads_threaddeleteat.up.sql diff --git a/db/migrations/mysql/000083_threads_threaddeleteat.down.sql b/db/migrations/mysql/000083_threads_threaddeleteat.down.sql new file mode 100644 index 0000000000..0068929f06 --- /dev/null +++ b/db/migrations/mysql/000083_threads_threaddeleteat.down.sql @@ -0,0 +1,14 @@ +SET @preparedStatement = (SELECT IF( + EXISTS( + SELECT 1 FROM INFORMATION_SCHEMA.STATISTICS + WHERE table_name = 'Threads' + AND table_schema = DATABASE() + AND column_name = 'ThreadDeleteAt' + ) > 0, + 'ALTER TABLE Threads DROP COLUMN ThreadDeleteAt;', + 'SELECT 1;' +)); + +PREPARE removeColumnIfExists FROM @preparedStatement; +EXECUTE removeColumnIfExists; +DEALLOCATE PREPARE removeColumnIfExists; diff --git a/db/migrations/mysql/000083_threads_threaddeleteat.up.sql b/db/migrations/mysql/000083_threads_threaddeleteat.up.sql new file mode 100644 index 0000000000..5c2bc73f0b --- /dev/null +++ b/db/migrations/mysql/000083_threads_threaddeleteat.up.sql @@ -0,0 +1,35 @@ +-- Drop any existing DeleteAt column from 000081_threads_deleteat.up.sql +SET @preparedStatement = (SELECT IF( + EXISTS( + SELECT 1 FROM INFORMATION_SCHEMA.STATISTICS + WHERE table_name = 'Threads' + AND table_schema = DATABASE() + AND column_name = 'DeleteAt' + ) > 0, + 'ALTER TABLE Threads DROP COLUMN DeleteAt;', + 'SELECT 1;' +)); + +PREPARE removeColumnIfExists FROM @preparedStatement; +EXECUTE removeColumnIfExists; +DEALLOCATE PREPARE removeColumnIfExists; + +SET @preparedStatement = (SELECT IF( + NOT EXISTS( + SELECT 1 FROM INFORMATION_SCHEMA.COLUMNS + WHERE table_name = 'Threads' + AND table_schema = DATABASE() + AND column_name = 'ThreadDeleteAt' + ), + 'ALTER TABLE Threads ADD COLUMN ThreadDeleteAt bigint(20);', + 'SELECT 1;' +)); + +PREPARE addColumnIfNotExists FROM @preparedStatement; +EXECUTE addColumnIfNotExists; +DEALLOCATE PREPARE addColumnIfNotExists; + +UPDATE Threads, Posts +SET Threads.ThreadDeleteAt = Posts.DeleteAt +WHERE Posts.Id = Threads.PostId +AND Threads.ThreadDeleteAt IS NULL; diff --git a/db/migrations/postgres/000081_threads_deleteat.down.sql b/db/migrations/postgres/000081_threads_deleteat.down.sql index 5d9d16db1f..cacb45cb5a 100644 --- a/db/migrations/postgres/000081_threads_deleteat.down.sql +++ b/db/migrations/postgres/000081_threads_deleteat.down.sql @@ -1 +1 @@ -ALTER TABLE threads DROP COLUMN IF EXISTS deleteat; +-- Replaced by 000083_threads_threaddeleteat.down.sql diff --git a/db/migrations/postgres/000081_threads_deleteat.up.sql b/db/migrations/postgres/000081_threads_deleteat.up.sql index 5de4aa1905..ceeffcc078 100644 --- a/db/migrations/postgres/000081_threads_deleteat.up.sql +++ b/db/migrations/postgres/000081_threads_deleteat.up.sql @@ -1,2 +1 @@ -ALTER TABLE threads ADD COLUMN IF NOT EXISTS deleteat bigint; -UPDATE threads SET deleteat = posts.deleteat FROM posts WHERE threads.deleteat IS NULL AND posts.id = threads.postid; +-- Replaced by 000083_threads_threaddeleteat.up.sql diff --git a/db/migrations/postgres/000083_threads_threaddeleteat.down.sql b/db/migrations/postgres/000083_threads_threaddeleteat.down.sql new file mode 100644 index 0000000000..5bfd63b47b --- /dev/null +++ b/db/migrations/postgres/000083_threads_threaddeleteat.down.sql @@ -0,0 +1 @@ +ALTER TABLE threads DROP COLUMN IF EXISTS threaddeleteat; diff --git a/db/migrations/postgres/000083_threads_threaddeleteat.up.sql b/db/migrations/postgres/000083_threads_threaddeleteat.up.sql new file mode 100644 index 0000000000..51afbdfc19 --- /dev/null +++ b/db/migrations/postgres/000083_threads_threaddeleteat.up.sql @@ -0,0 +1,5 @@ +-- Drop any existing DeleteAt column from 000081_threads_deleteat.up.sql +ALTER TABLE threads DROP COLUMN IF EXISTS deleteat; + +ALTER TABLE threads ADD COLUMN IF NOT EXISTS threaddeleteat bigint; +UPDATE threads SET threaddeleteat = posts.deleteat FROM posts WHERE threads.threaddeleteat IS NULL AND posts.id = threads.postid; diff --git a/model/thread.go b/model/thread.go index d2ca025a72..f9c4842c71 100644 --- a/model/thread.go +++ b/model/thread.go @@ -23,7 +23,8 @@ type Thread struct { // to newest. Note that the root post author is not included in this list until they reply. Participants StringArray `json:"participants"` - // DeleteAt is a denormalized copy of the root posts's DeleteAt. + // DeleteAt is a denormalized copy of the root posts's DeleteAt. In the database, it's + // named ThreadDeleteAt to avoid introducing a query conflict with older server versions. DeleteAt int64 `json:"delete_at"` } diff --git a/store/sqlstore/post_store.go b/store/sqlstore/post_store.go index 0e1e3e3251..cc271e09e1 100644 --- a/store/sqlstore/post_store.go +++ b/store/sqlstore/post_store.go @@ -2605,7 +2605,7 @@ func (s *SqlPostStore) permanentDeleteThreads(transaction *sqlxTxWrapper, postId func (s *SqlPostStore) deleteThread(transaction *sqlxTxWrapper, postId string, deleteAtTime int64) error { queryString, args, err := s.getQueryBuilder(). Update("Threads"). - Set("DeleteAt", deleteAtTime). + Set("ThreadDeleteAt", deleteAtTime). Where(sq.Eq{"PostId": postId}). ToSql() if err != nil { @@ -2701,7 +2701,7 @@ func (s *SqlPostStore) updateThreadsFromPosts(transaction *sqlxTxWrapper, posts "Threads.ReplyCount", "Threads.LastReplyAt", "Threads.Participants", - "COALESCE(Threads.DeleteAt, 0) AS DeleteAt", + "COALESCE(Threads.ThreadDeleteAt, 0) AS DeleteAt", ). From("Threads"). Where(sq.Eq{"Threads.PostId": rootIds}). diff --git a/store/sqlstore/thread_store.go b/store/sqlstore/thread_store.go index 6b8c417607..39549271c6 100644 --- a/store/sqlstore/thread_store.go +++ b/store/sqlstore/thread_store.go @@ -50,7 +50,7 @@ func (s *SqlThreadStore) initializeQueries() { "Threads.ReplyCount", "Threads.LastReplyAt", "Threads.Participants", - "COALESCE(Threads.DeleteAt, 0) AS DeleteAt", + "COALESCE(Threads.ThreadDeleteAt, 0) AS DeleteAt", ). From("Threads") @@ -61,7 +61,7 @@ func (s *SqlThreadStore) initializeQueries() { "Threads.ReplyCount", "Threads.LastReplyAt", "Threads.Participants", - "COALESCE(Threads.DeleteAt, 0) AS ThreadDeleteAt", + "COALESCE(Threads.ThreadDeleteAt, 0) AS ThreadDeleteAt", ). From("Threads") } @@ -107,7 +107,7 @@ func (s *SqlThreadStore) getTotalThreadsQuery(userId, teamId string, opts model. } if !opts.Deleted { - query = query.Where(sq.Eq{"COALESCE(Threads.DeleteAt, 0)": 0}) + query = query.Where(sq.Eq{"COALESCE(Threads.ThreadDeleteAt, 0)": 0}) } return query @@ -180,7 +180,7 @@ func (s *SqlThreadStore) GetTotalUnreadMentions(userId, teamId string, opts mode } if !opts.Deleted { - query = query.Where(sq.Eq{"COALESCE(Threads.DeleteAt, 0)": 0}) + query = query.Where(sq.Eq{"COALESCE(Threads.ThreadDeleteAt, 0)": 0}) } sql, args, err := query.ToSql() @@ -256,8 +256,8 @@ func (s *SqlThreadStore) GetThreadsForUser(userId, teamId string, opts model.Get if !opts.Deleted { query = query.Where(sq.Or{ - sq.Eq{"Threads.DeleteAt": nil}, - sq.Eq{"Threads.DeleteAt": 0}, + sq.Eq{"Threads.ThreadDeleteAt": nil}, + sq.Eq{"Threads.ThreadDeleteAt": 0}, }) } @@ -352,7 +352,7 @@ func (s *SqlThreadStore) GetTeamsUnreadForUser(userID string, teamIDs []string) sq.Eq{"ThreadMemberships.UserId": userID}, sq.Eq{"ThreadMemberships.Following": true}, sq.Eq{"Channels.TeamId": teamIDs}, - sq.Eq{"COALESCE(Threads.DeleteAt, 0)": 0}, + sq.Eq{"COALESCE(Threads.ThreadDeleteAt, 0)": 0}, } var wg sync.WaitGroup