MM-43045: minimize JOIN Posts (#19934)

app/post_test.go

@@ -2491,13 +2491,12 @@ func TestCollapsedThreadFetch(t *testing.T) {
 		}()
 
 		require.NotPanics(t, func() {
-			_, err = th.App.CreatePost(th.Context, &model.Post{
+			th.App.CreatePost(th.Context, &model.Post{
 				UserId:    user1.Id,
 				ChannelId: channel.Id,
 				RootId:    postRoot.Id,
 				Message:   fmt.Sprintf("@%s", user2.Username),
 			}, channel, false, true)
-			require.Nil(t, err)
 		})
 
 		wg.Wait()

app/user.go

@@ -2311,15 +2311,20 @@ func (a *App) GetThreadsForUser(userID, teamID string, options model.GetUserThre
 			return nil
 		})
 
-		eg.Go(func() error {
+		// Unread is a legacy flag that caused GetTotalThreads to compute the same value as
-			totalCount, err := a.Srv().Store.Thread().GetTotalThreads(userID, teamID, options)
+		// GetTotalUnreadThreads. If unspecified, do this work normally; otherwise, skip,
-			if err != nil {
+		// and send back duplicate values down below.
-				return errors.Wrapf(err, "failed to count threads for user id=%s", userID)
+		if !options.Unread {
-			}
+			eg.Go(func() error {
-			result.Total = totalCount
+				totalCount, err := a.Srv().Store.Thread().GetTotalThreads(userID, teamID, options)
+				if err != nil {
+					return errors.Wrapf(err, "failed to count threads for user id=%s", userID)
+				}
+				result.Total = totalCount
 
-			return nil
+				return nil
-		})
+			})
+		}
 
 		eg.Go(func() error {
 			totalUnreadMentions, err := a.Srv().Store.Thread().GetTotalUnreadMentions(userID, teamID, options)
@@ -2348,6 +2353,10 @@ func (a *App) GetThreadsForUser(userID, teamID string, options model.GetUserThre
 		return nil, model.NewAppError("GetThreadsForUser", "app.user.get_threads_for_user.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
+	if options.Unread {
+		result.Total = result.TotalUnreadThreads
+	}
+
 	for _, thread := range result.Threads {
 		a.sanitizeProfiles(thread.Participants, false)
 		thread.Post.SanitizeProps()

db/migrations/mysql/000081_threads_deleteat.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 = 'DeleteAt'
+    ) > 0,
+    'ALTER TABLE Threads DROP COLUMN DeleteAt;',
+    'SELECT 1;'
+));
+
+PREPARE removeColumnIfExists FROM @preparedStatement;
+EXECUTE removeColumnIfExists;
+DEALLOCATE PREPARE removeColumnIfExists;

db/migrations/mysql/000081_threads_deleteat.up.sql

@@ -0,0 +1,19 @@
+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;

db/migrations/postgres/000081_threads_deleteat.down.sql

@@ -0,0 +1 @@
+ALTER TABLE threads DROP COLUMN IF EXISTS deleteat;

db/migrations/postgres/000081_threads_deleteat.up.sql

@@ -0,0 +1,2 @@
+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;

model/thread.go

@@ -22,6 +22,9 @@ type Thread struct {
 	// Participants is a list of user ids that have replied to the thread, sorted by the oldest
 	// 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 int64 `json:"delete_at"`
 }
 
 type ThreadResponse struct {
@@ -33,6 +36,7 @@ type ThreadResponse struct {
 	Post           *Post   `json:"post"`
 	UnreadReplies  int64   `json:"unread_replies"`
 	UnreadMentions int64   `json:"unread_mentions"`
+	DeleteAt       int64   `json:"delete_at"`
 }
 
 type Threads struct {

store/sqlstore/post_store.go

@@ -215,7 +215,7 @@ func (s *SqlPostStore) SaveMultiple(posts []*model.Post) ([]*model.Post, int, er
 	}
 
 	if err = s.updateThreadsFromPosts(transaction, posts); err != nil {
-		mlog.Warn("Error updating posts, thread update failed", mlog.Err(err))
+		return nil, -1, errors.Wrap(err, "update thread from posts failed")
 	}
 
 	if err = transaction.Commit(); err != nil {
@@ -290,7 +290,7 @@ func (s *SqlPostStore) populateReplyCount(posts []*model.Post) error {
 		Select("RootId, COUNT(Id) AS Count").
 		From("Posts").
 		Where(sq.Eq{"RootId": rootIds}).
-		Where(sq.Eq{"DeleteAt": 0}).
+		Where(sq.Eq{"Posts.DeleteAt": 0}).
 		GroupBy("RootId")
 
 	queryString, args, err := query.ToSql()
@@ -487,7 +487,7 @@ func (s *SqlPostStore) getFlaggedPosts(userId, channelId, teamId string, offset
 							AND Category = ?
 					)
 					CHANNEL_FILTER
-					AND DeleteAt = 0
+					AND Posts.DeleteAt = 0
                 ) as A
             INNER JOIN Channels as B
                 ON B.Id = A.ChannelId
@@ -569,8 +569,8 @@ func (s *SqlPostStore) getPostWithCollapsedThreads(id, userID string, opts model
 		From("Posts").
 		LeftJoin("Threads ON Threads.PostId = Id").
 		LeftJoin("ThreadMemberships ON ThreadMemberships.PostId = Id AND ThreadMemberships.UserId = ?", userID).
-		Where(sq.Eq{"DeleteAt": 0}).
+		Where(sq.Eq{"Posts.DeleteAt": 0}).
-		Where(sq.Eq{"Id": id}).ToSql()
+		Where(sq.Eq{"Posts.Id": id}).ToSql()
 
 	err := s.GetReplicaX().Get(&post, postFetchQuery, args...)
 	if err != nil {
@@ -586,8 +586,8 @@ func (s *SqlPostStore) getPostWithCollapsedThreads(id, userID string, opts model
 		Select("*").
 		From("Posts").
 		Where(sq.Eq{
-			"RootId":   id,
+			"Posts.RootId":   id,
-			"DeleteAt": 0,
+			"Posts.DeleteAt": 0,
 		})
 
 	var sort string
@@ -873,7 +873,11 @@ func (s *SqlPostStore) Delete(postID string, time int64, deleteByID string) erro
 		return errors.Wrap(err, "failed to update Posts")
 	}
 
-	err = s.cleanupThreadComments(transaction, postID, id.RootId, id.UserId)
+	if id.RootId == "" {
+		err = s.deleteThread(transaction, postID, time)
+	} else {
+		err = s.updateThreadAfterReplyDeletion(transaction, id.RootId, id.UserId)
+	}
 
 	if err != nil {
 		return errors.Wrapf(err, "failed to cleanup Thread with postid=%s", id.RootId)
@@ -933,13 +937,12 @@ func (s *SqlPostStore) permanentDeleteAllCommentByUser(userId string) error {
 	}
 
 	_, err = transaction.Exec("DELETE FROM Posts WHERE UserId = ? AND RootId != ''", userId)
-
 	if err != nil {
 		return errors.Wrapf(err, "failed to delete Posts with userId=%s", userId)
 	}
 
 	for _, ids := range results {
-		if err = s.cleanupThreadComments(transaction, ids.Id, ids.RootId, userId); err != nil {
+		if err = s.updateThreadAfterReplyDeletion(transaction, ids.RootId, userId); err != nil {
 			return err
 		}
 	}
@@ -1103,14 +1106,14 @@ func (s *SqlPostStore) getPostsCollapsedThreads(options model.GetPostsOptions) (
 	postFetchQuery, args, _ := s.getQueryBuilder().
 		Select(columns...).
 		From("Posts").
-		LeftJoin("Threads ON Threads.PostId = Id").
+		LeftJoin("Threads ON Threads.PostId = Posts.Id").
-		LeftJoin("ThreadMemberships ON ThreadMemberships.PostId = Id AND ThreadMemberships.UserId = ?", options.UserId).
+		LeftJoin("ThreadMemberships ON ThreadMemberships.PostId = Posts.Id AND ThreadMemberships.UserId = ?", options.UserId).
-		Where(sq.Eq{"DeleteAt": 0}).
+		Where(sq.Eq{"Posts.DeleteAt": 0}).
 		Where(sq.Eq{"Posts.ChannelId": options.ChannelId}).
-		Where(sq.Eq{"RootId": ""}).
+		Where(sq.Eq{"Posts.RootId": ""}).
 		Limit(uint64(options.PerPage)).
 		Offset(uint64(offset)).
-		OrderBy("CreateAt DESC").ToSql()
+		OrderBy("Posts.CreateAt DESC").ToSql()
 
 	err := s.GetReplicaX().Select(&posts, postFetchQuery, args...)
 	if err != nil {
@@ -1187,13 +1190,13 @@ func (s *SqlPostStore) getPostsSinceCollapsedThreads(options model.GetPostsSince
 	postFetchQuery, args, _ := s.getQueryBuilder().
 		Select(columns...).
 		From("Posts").
-		LeftJoin("Threads ON Threads.PostId = Id").
+		LeftJoin("Threads ON Threads.PostId = Posts.Id").
-		LeftJoin("ThreadMemberships ON ThreadMemberships.PostId = Id AND ThreadMemberships.UserId = ?", options.UserId).
+		LeftJoin("ThreadMemberships ON ThreadMemberships.PostId = Posts.Id AND ThreadMemberships.UserId = ?", options.UserId).
-		Where(sq.Eq{"DeleteAt": 0}).
+		Where(sq.Eq{"Posts.DeleteAt": 0}).
 		Where(sq.Eq{"Posts.ChannelId": options.ChannelId}).
-		Where(sq.Gt{"UpdateAt": options.Time}).
+		Where(sq.Gt{"Posts.UpdateAt": options.Time}).
-		Where(sq.Eq{"RootId": ""}).
+		Where(sq.Eq{"Posts.RootId": ""}).
-		OrderBy("CreateAt DESC").ToSql()
+		OrderBy("Posts.CreateAt DESC").ToSql()
 
 	err := s.GetReplicaX().Select(&posts, postFetchQuery, args...)
 	if err != nil {
@@ -1316,16 +1319,16 @@ func (s *SqlPostStore) GetPostsSinceForSync(options model.GetPostsSinceForSyncOp
 	query := s.getQueryBuilder().
 		Select("*").
 		From("Posts").
-		Where(sq.Or{sq.Gt{"UpdateAt": cursor.LastPostUpdateAt}, sq.And{sq.Eq{"UpdateAt": cursor.LastPostUpdateAt}, sq.Gt{"Id": cursor.LastPostId}}}).
+		Where(sq.Or{sq.Gt{"Posts.UpdateAt": cursor.LastPostUpdateAt}, sq.And{sq.Eq{"Posts.UpdateAt": cursor.LastPostUpdateAt}, sq.Gt{"Posts.Id": cursor.LastPostId}}}).
-		OrderBy("UpdateAt", "Id").
+		OrderBy("Posts.UpdateAt", "Id").
 		Limit(uint64(limit))
 
 	if options.ChannelId != "" {
-		query = query.Where(sq.Eq{"ChannelId": options.ChannelId})
+		query = query.Where(sq.Eq{"Posts.ChannelId": options.ChannelId})
 	}
 
 	if !options.IncludeDeleted {
-		query = query.Where(sq.Eq{"DeleteAt": 0})
+		query = query.Where(sq.Eq{"Posts.DeleteAt": 0})
 	}
 
 	if options.ExcludeRemoteId != "" {
@@ -1402,7 +1405,7 @@ func (s *SqlPostStore) getPostsAround(before bool, options model.GetPostsOptions
 	conditions := sq.And{
 		sq.Expr(`CreateAt `+direction+` (SELECT CreateAt FROM Posts WHERE Id = ?)`, options.PostId),
 		sq.Eq{"p.ChannelId": options.ChannelId},
-		sq.Eq{"DeleteAt": int(0)},
+		sq.Eq{"p.DeleteAt": int(0)},
 	}
 	if options.CollapsedThreads {
 		conditions = append(conditions, sq.Eq{"RootId": ""})
@@ -1415,7 +1418,7 @@ func (s *SqlPostStore) getPostsAround(before bool, options model.GetPostsOptions
 		// Adding ChannelId and DeleteAt order columns
 		// to let mysql choose the "idx_posts_channel_id_delete_at_create_at" index always.
 		// See MM-24170.
-		OrderBy("p.ChannelId", "DeleteAt", "CreateAt "+sort).
+		OrderBy("p.ChannelId", "p.DeleteAt", "p.CreateAt "+sort).
 		Limit(uint64(options.PerPage)).
 		Offset(uint64(offset))
 
@@ -1448,8 +1451,8 @@ func (s *SqlPostStore) getPostsAround(before bool, options model.GetPostsOptions
 		rootQuery = rootQuery.From("Posts p").
 			Where(sq.And{
 				idQuery,
-				sq.Eq{"ChannelId": options.ChannelId},
+				sq.Eq{"p.ChannelId": options.ChannelId},
-				sq.Eq{"DeleteAt": 0},
+				sq.Eq{"p.DeleteAt": 0},
 			}).
 			OrderBy("CreateAt DESC")
 
@@ -1505,11 +1508,11 @@ func (s *SqlPostStore) getPostIdAroundTime(channelId string, time int64, before
 
 	conditions := sq.And{
 		direction,
-		sq.Eq{"ChannelId": channelId},
+		sq.Eq{"Posts.ChannelId": channelId},
-		sq.Eq{"DeleteAt": int(0)},
+		sq.Eq{"Posts.DeleteAt": int(0)},
 	}
 	if collapsedThreads {
-		conditions = sq.And{conditions, sq.Eq{"RootId": ""}}
+		conditions = sq.And{conditions, sq.Eq{"Posts.RootId": ""}}
 	}
 	query := s.getQueryBuilder().
 		Select("Id").
@@ -1518,7 +1521,7 @@ func (s *SqlPostStore) getPostIdAroundTime(channelId string, time int64, before
 		// Adding ChannelId and DeleteAt order columns
 		// to let mysql choose the "idx_posts_channel_id_delete_at_create_at" index always.
 		// See MM-23369.
-		OrderBy("ChannelId", "DeleteAt", "CreateAt "+sort).
+		OrderBy("Posts.ChannelId", "Posts.DeleteAt", "Posts.CreateAt "+sort).
 		Limit(1)
 
 	queryString, args, err := query.ToSql()
@@ -1545,9 +1548,9 @@ func (s *SqlPostStore) GetPostAfterTime(channelId string, time int64, collapsedT
 		table += " USE INDEX(idx_posts_channel_id_delete_at_create_at)"
 	}
 	conditions := sq.And{
-		sq.Gt{"CreateAt": time},
+		sq.Gt{"Posts.CreateAt": time},
-		sq.Eq{"ChannelId": channelId},
+		sq.Eq{"Posts.ChannelId": channelId},
-		sq.Eq{"DeleteAt": int(0)},
+		sq.Eq{"Posts.DeleteAt": int(0)},
 	}
 	if collapsedThreads {
 		conditions = sq.And{conditions, sq.Eq{"RootId": ""}}
@@ -1559,7 +1562,7 @@ func (s *SqlPostStore) GetPostAfterTime(channelId string, time int64, collapsedT
 		// Adding ChannelId and DeleteAt order columns
 		// to let mysql choose the "idx_posts_channel_id_delete_at_create_at" index always.
 		// See MM-23369.
-		OrderBy("ChannelId", "DeleteAt", "CreateAt ASC").
+		OrderBy("Posts.ChannelId", "Posts.DeleteAt", "Posts.CreateAt ASC").
 		Limit(1)
 
 	queryString, args, err := query.ToSql()
@@ -1581,9 +1584,9 @@ func (s *SqlPostStore) getRootPosts(channelId string, offset int, limit int, ski
 	posts := []*model.Post{}
 	var fetchQuery string
 	if skipFetchThreads {
-		fetchQuery = "SELECT p.*, (SELECT COUNT(*) FROM Posts WHERE Posts.RootId = (CASE WHEN p.RootId = '' THEN p.Id ELSE p.RootId END) AND Posts.DeleteAt = 0) as ReplyCount FROM Posts p WHERE ChannelId = ? AND DeleteAt = 0 ORDER BY CreateAt DESC LIMIT ? OFFSET ?"
+		fetchQuery = "SELECT p.*, (SELECT COUNT(*) FROM Posts WHERE Posts.RootId = (CASE WHEN p.RootId = '' THEN p.Id ELSE p.RootId END) AND Posts.DeleteAt = 0) as ReplyCount FROM Posts p WHERE p.ChannelId = ? AND p.DeleteAt = 0 ORDER BY p.CreateAt DESC LIMIT ? OFFSET ?"
 	} else {
-		fetchQuery = "SELECT * FROM Posts WHERE ChannelId = ? AND DeleteAt = 0 ORDER BY CreateAt DESC LIMIT ? OFFSET ?"
+		fetchQuery = "SELECT * FROM Posts WHERE Posts.ChannelId = ? AND Posts.DeleteAt = 0 ORDER BY Posts.CreateAt DESC LIMIT ? OFFSET ?"
 	}
 	err := s.GetReplicaX().Select(&posts, fetchQuery, channelId, limit, offset)
 	if err != nil {
@@ -1604,13 +1607,13 @@ func (s *SqlPostStore) getParentsPosts(channelId string, offset int, limit int,
 			q.RootId
 		FROM
 			(SELECT
-				RootId
+				Posts.RootId
 			FROM
 				Posts
 			WHERE
-				ChannelId = ?
+				Posts.ChannelId = ?
-					AND DeleteAt = 0
+					AND Posts.DeleteAt = 0
-			ORDER BY CreateAt DESC
+			ORDER BY Posts.CreateAt DESC
 			LIMIT ? OFFSET ?) q
 		WHERE q.RootId != ''`
 
@@ -1639,10 +1642,10 @@ func (s *SqlPostStore) getParentsPosts(channelId string, offset int, limit int,
 		From("Posts p").
 		Where(sq.And{
 			where,
-			sq.Eq{"ChannelId": channelId},
+			sq.Eq{"p.ChannelId": channelId},
-			sq.Eq{"DeleteAt": 0},
+			sq.Eq{"p.DeleteAt": 0},
 		}).
-		OrderBy("CreateAt")
+		OrderBy("p.CreateAt")
 
 	sql, args, err := query.ToSql()
 	if err != nil {
@@ -1675,20 +1678,20 @@ func (s *SqlPostStore) getParentsPostsPostgreSQL(channelId string, offset int, l
                 q3.RootId
             FROM
                 (SELECT
-                    RootId
+                    Posts.RootId
                 FROM
                     Posts
                 WHERE
-                    ChannelId = ?
+                    Posts.ChannelId = ?
-                        AND DeleteAt = 0
+                        AND Posts.DeleteAt = 0
-                ORDER BY CreateAt DESC
+                ORDER BY Posts.CreateAt DESC
                 LIMIT ? OFFSET ?) q3
             WHERE q3.RootId != '') q1
             ON `+onStatement+`
         WHERE
-            ChannelId = ?
+            q2.ChannelId = ?
-            AND DeleteAt = 0
+            AND q2.DeleteAt = 0
-        ORDER BY CreateAt`, channelId, limit, offset, channelId)
+        ORDER BY q2.CreateAt`, channelId, limit, offset, channelId)
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to find Posts with channelId=%s", channelId)
 	}
@@ -1835,9 +1838,9 @@ func (s *SqlPostStore) search(teamId string, userId string, params *model.Search
 		"*",
 		"(SELECT COUNT(*) FROM Posts WHERE Posts.RootId = (CASE WHEN q2.RootId = '' THEN q2.Id ELSE q2.RootId END) AND Posts.DeleteAt = 0) as ReplyCount",
 	).From("Posts q2").
-		Where("DeleteAt = 0").
+		Where("q2.DeleteAt = 0").
-		Where(fmt.Sprintf("Type NOT LIKE '%s%%'", model.PostSystemMessagePrefix)).
+		Where(fmt.Sprintf("q2.Type NOT LIKE '%s%%'", model.PostSystemMessagePrefix)).
-		OrderByClause("CreateAt DESC").
+		OrderByClause("q2.CreateAt DESC").
 		Limit(100)
 
 	var err error
@@ -1928,11 +1931,11 @@ func (s *SqlPostStore) search(teamId string, userId string, params *model.Search
 		Where("Id = ChannelId")
 
 	if !params.IncludeDeletedChannels {
-		inQuery = inQuery.Where("DeleteAt = 0")
+		inQuery = inQuery.Where("Channels.DeleteAt = 0")
 	}
 
 	if !params.SearchWithoutUserId {
-		inQuery = inQuery.Where("UserId = ?", userId)
+		inQuery = inQuery.Where("ChannelMembers.UserId = ?", userId)
 	}
 
 	inQuery = s.buildSearchTeamFilterClause(teamId, inQuery)
@@ -2378,10 +2381,10 @@ func (s *SqlPostStore) GetParentsForExportAfter(limit int, afterId string) ([]*m
 			FROM
 				Posts
 			WHERE
-				Id > ?
+				Posts.Id > ?
-				AND RootId = ''
+				AND Posts.RootId = ''
-				AND DeleteAt = 0
+				AND Posts.DeleteAt = 0
-			ORDER BY Id
+			ORDER BY Posts.Id
 			LIMIT ?`,
 			afterId, limit)
 		if err != nil {
@@ -2395,7 +2398,7 @@ func (s *SqlPostStore) GetParentsForExportAfter(limit int, afterId string) ([]*m
 
 		builder := s.getQueryBuilder().
 			Select("p1.*, Users.Username as Username, Teams.Name as TeamName, Channels.Name as ChannelName").
-			FromSelect(sq.Select("*").From("Posts").Where(sq.Eq{"Id": rootIds}), "p1").
+			FromSelect(sq.Select("*").From("Posts").Where(sq.Eq{"Posts.Id": rootIds}), "p1").
 			InnerJoin("Channels ON p1.ChannelId = Channels.Id").
 			InnerJoin("Teams ON Channels.TeamId = Teams.Id").
 			InnerJoin("Users ON p1.UserId = Users.Id").
@@ -2598,19 +2601,36 @@ func (s *SqlPostStore) permanentDeleteThreads(transaction *sqlxTxWrapper, postId
 	return nil
 }
 
-// Thread cleanup upon post deletion
+// deleteThread marks a thread as deleted at the given time.
-// if the post is a comment
+func (s *SqlPostStore) deleteThread(transaction *sqlxTxWrapper, postId string, deleteAtTime int64) error {
-// reply count is reduced by 1 and,
+	queryString, args, err := s.getQueryBuilder().
-// the user is removed from participants if the comment deleted is the last reply from said user.
+		Update("Threads").
-func (s *SqlPostStore) cleanupThreadComments(transaction *sqlxTxWrapper, postId, rootId string, userId string) error {
+		Set("DeleteAt", deleteAtTime).
+		Where(sq.Eq{"PostId": postId}).
+		ToSql()
+	if err != nil {
+		return errors.Wrapf(err, "failed to create SQL query to mark thread for root post %s as deleted", postId)
+	}
+
+	_, err = transaction.Exec(queryString, args...)
+	if err != nil {
+		return errors.Wrapf(err, "failed to mark thread for root post %s as deleted", postId)
+	}
+
+	return nil
+}
+
+// updateThreadAfterReplyDeletion decrements the thread reply count and adjusts the participants
+// list as necessary.
+func (s *SqlPostStore) updateThreadAfterReplyDeletion(transaction *sqlxTxWrapper, rootId string, userId string) error {
 	if rootId != "" {
 		queryString, args, err := s.getQueryBuilder().
-			Select("COUNT(Id)").
+			Select("COUNT(Posts.Id)").
 			From("Posts").
 			Where(sq.And{
-				sq.Eq{"RootId": rootId},
+				sq.Eq{"Posts.RootId": rootId},
-				sq.Eq{"UserId": userId},
+				sq.Eq{"Posts.UserId": userId},
-				sq.Eq{"DeleteAt": 0},
+				sq.Eq{"Posts.DeleteAt": 0},
 			}).
 			ToSql()
 
@@ -2675,9 +2695,16 @@ func (s *SqlPostStore) updateThreadsFromPosts(transaction *sqlxTxWrapper, posts
 		return nil
 	}
 	threadsByRootsSql, threadsByRootsArgs, _ := s.getQueryBuilder().
-		Select("*").
+		Select(
+			"Threads.PostId",
+			"Threads.ChannelId",
+			"Threads.ReplyCount",
+			"Threads.LastReplyAt",
+			"Threads.Participants",
+			"COALESCE(Threads.DeleteAt, 0) AS DeleteAt",
+		).
 		From("Threads").
-		Where(sq.Eq{"PostId": rootIds}).
+		Where(sq.Eq{"Threads.PostId": rootIds}).
 		ToSql()
 	threadsByRoots := []*model.Thread{}
 	if err := transaction.Select(&threadsByRoots, threadsByRootsSql, threadsByRootsArgs...); err != nil {
@@ -2697,7 +2724,7 @@ func (s *SqlPostStore) updateThreadsFromPosts(transaction *sqlxTxWrapper, posts
 			}{}
 
 			// calculate participants
-			if err := transaction.Select(&data, "SELECT UserId, MAX(CreateAt) as RepliedAt FROM Posts WHERE RootId=? AND DeleteAt=0 GROUP BY UserId ORDER BY RepliedAt ASC", rootId); err != nil {
+			if err := transaction.Select(&data, "SELECT Posts.UserId, MAX(Posts.CreateAt) as RepliedAt FROM Posts WHERE Posts.RootId=? AND Posts.DeleteAt=0 GROUP BY Posts.UserId ORDER BY RepliedAt ASC", rootId); err != nil {
 				return err
 			}
 
@@ -2708,13 +2735,13 @@ func (s *SqlPostStore) updateThreadsFromPosts(transaction *sqlxTxWrapper, posts
 
 			// calculate reply count
 			var count int64
-			err := transaction.Get(&count, "SELECT COUNT(Id) FROM Posts WHERE RootId=? And DeleteAt=0", rootId)
+			err := transaction.Get(&count, "SELECT COUNT(Posts.Id) FROM Posts WHERE Posts.RootId=? And Posts.DeleteAt=0", rootId)
 			if err != nil {
 				return err
 			}
 			// calculate last reply at
 			var lastReplyAt int64
-			err = transaction.Get(&lastReplyAt, "SELECT COALESCE(MAX(Posts.CreateAt), 0) FROM Posts WHERE RootID=? and DeleteAt=0", rootId)
+			err = transaction.Get(&lastReplyAt, "SELECT COALESCE(MAX(Posts.CreateAt), 0) FROM Posts WHERE Posts.RootID=? and Posts.DeleteAt=0", rootId)
 			if err != nil {
 				return err
 			}

store/sqlstore/thread_store.go

@@ -20,28 +20,63 @@ import (
 
 type SqlThreadStore struct {
 	*SqlStore
+
+	// threadsSelectQuery is for querying directly into model.Thread
+	threadsSelectQuery sq.SelectBuilder
+
+	// threadsAndPostsSelectQuery is for querying into a struct embedding fields from
+	// model.Thread and model.Post.
+	threadsAndPostsSelectQuery sq.SelectBuilder
 }
 
 func (s *SqlThreadStore) ClearCaches() {
 }
 
 func newSqlThreadStore(sqlStore *SqlStore) store.ThreadStore {
-	return &SqlThreadStore{
+	s := SqlThreadStore{
 		SqlStore: sqlStore,
 	}
+
+	s.initializeQueries()
+
+	return &s
+}
+
+func (s *SqlThreadStore) initializeQueries() {
+	s.threadsSelectQuery = s.getQueryBuilder().
+		Select(
+			"Threads.PostId",
+			"Threads.ChannelId",
+			"Threads.ReplyCount",
+			"Threads.LastReplyAt",
+			"Threads.Participants",
+			"COALESCE(Threads.DeleteAt, 0) AS DeleteAt",
+		).
+		From("Threads")
+
+	s.threadsAndPostsSelectQuery = s.getQueryBuilder().
+		Select(
+			"Threads.PostId",
+			"Threads.ChannelId",
+			"Threads.ReplyCount",
+			"Threads.LastReplyAt",
+			"Threads.Participants",
+			"COALESCE(Threads.DeleteAt, 0) AS ThreadDeleteAt",
+		).
+		From("Threads")
 }
 
 func (s *SqlThreadStore) Get(id string) (*model.Thread, error) {
 	var thread model.Thread
-	query, args, err := s.getQueryBuilder().
+
-		Select("*").
+	query, args, err := s.threadsSelectQuery.
-		From("Threads").
 		Where(sq.Eq{"PostId": id}).
 		ToSql()
 	if err != nil {
 		return nil, errors.Wrap(err, "thread_tosql")
 	}
-	err = s.GetMasterX().Get(&thread, query, args...)
+
+	err = s.GetReplicaX().Get(&thread, query, args...)
 	if err != nil {
 		if err == sql.ErrNoRows {
 			return nil, nil
@@ -52,55 +87,7 @@ func (s *SqlThreadStore) Get(id string) (*model.Thread, error) {
 	return &thread, nil
 }
 
-// GetTotalUnreadThreads counts the number of unread threads for the given user, optionally
+func (s *SqlThreadStore) getTotalThreadsQuery(userId, teamId string, opts model.GetUserThreadsOpts) sq.SelectBuilder {
-// constrained to the given team + DMs/GMs.
-func (s *SqlThreadStore) GetTotalUnreadThreads(userId, teamId string, opts model.GetUserThreadsOpts) (int64, error) {
-	var totalUnreadThreads int64
-
-	query := s.getQueryBuilder().
-		Select("COUNT(DISTINCT(Posts.RootId))").
-		From("Posts").
-		LeftJoin("ThreadMemberships ON Posts.RootId = ThreadMemberships.PostId").
-		Where("Posts.CreateAt > ThreadMemberships.LastViewed").
-		Where(sq.Eq{
-			"ThreadMemberships.UserId":    userId,
-			"ThreadMemberships.Following": true,
-		})
-
-	if teamId != "" {
-		query = query.
-			LeftJoin("Channels ON Posts.ChannelId = Channels.Id").
-			Where(sq.Or{
-				sq.Eq{"Channels.TeamId": teamId},
-				sq.Eq{"Channels.TeamId": ""},
-			})
-	}
-
-	if !opts.Deleted {
-		query = query.Where(sq.Eq{"COALESCE(Posts.DeleteAt, 0)": 0})
-	}
-
-	sql, args, err := query.ToSql()
-	if err != nil {
-		return 0, errors.Wrapf(err, "failed to build query to count unread threads for user id=%s", userId)
-	}
-
-	err = s.GetMasterX().Get(&totalUnreadThreads, sql, args...)
-	if err != nil {
-		return 0, errors.Wrapf(err, "failed to count unread threads for user id=%s", userId)
-	}
-
-	return totalUnreadThreads, nil
-}
-
-// GetTotalUnreadThreads counts the number of threads for the given user, optionally constrained
-// to the given team + DMs/GMs.
-//
-// TODO: Why do we support an Unread flag here? It's basically the same as GetTotalUnreadThreads,
-// but with different comparison semantics.
-func (s *SqlThreadStore) GetTotalThreads(userId, teamId string, opts model.GetUserThreadsOpts) (int64, error) {
-	var totalCount int64
-
 	query := s.getQueryBuilder().
 		Select("COUNT(ThreadMemberships.PostId)").
 		From("ThreadMemberships").
@@ -120,27 +107,53 @@ func (s *SqlThreadStore) GetTotalThreads(userId, teamId string, opts model.GetUs
 	}
 
 	if !opts.Deleted {
-		query = query.
+		query = query.Where(sq.Eq{"COALESCE(Threads.DeleteAt, 0)": 0})
-			LeftJoin("Posts ON Posts.Id = ThreadMemberships.PostId").
-			Where(sq.Eq{"COALESCE(Posts.DeleteAt, 0)": 0})
 	}
 
-	if opts.Unread {
+	return query
-		query = query.
+}
-			Where(sq.Expr("ThreadMemberships.LastViewed < Threads.LastReplyAt"))
+
+// GetTotalUnreadThreads counts the number of unread threads for the given user, optionally
+// constrained to the given team + DMs/GMs.
+func (s *SqlThreadStore) GetTotalUnreadThreads(userId, teamId string, opts model.GetUserThreadsOpts) (int64, error) {
+	query := s.getTotalThreadsQuery(userId, teamId, opts).
+		Where(sq.Expr("ThreadMemberships.LastViewed < Threads.LastReplyAt"))
+
+	sql, args, err := query.ToSql()
+	if err != nil {
+		return 0, errors.Wrapf(err, "failed to build query to count unread threads for user id=%s", userId)
 	}
 
+	var totalUnreadThreads int64
+	err = s.GetReplicaX().Get(&totalUnreadThreads, sql, args...)
+	if err != nil {
+		return 0, errors.Wrapf(err, "failed to count unread threads for user id=%s", userId)
+	}
+
+	return totalUnreadThreads, nil
+}
+
+// GetTotalUnreadThreads counts the number of threads for the given user, optionally constrained
+// to the given team + DMs/GMs.
+func (s *SqlThreadStore) GetTotalThreads(userId, teamId string, opts model.GetUserThreadsOpts) (int64, error) {
+	if opts.Unread {
+		return 0, errors.New("GetTotalThreads does not support the Unread flag; use GetTotalUnreadThreads instead")
+	}
+
+	query := s.getTotalThreadsQuery(userId, teamId, opts)
+
 	sql, args, err := query.ToSql()
 	if err != nil {
 		return 0, errors.Wrapf(err, "failed to build query to count threads for user id=%s", userId)
 	}
 
-	err = s.GetMasterX().Get(&totalCount, sql, args...)
+	var totalThreads int64
+	err = s.GetReplicaX().Get(&totalThreads, sql, args...)
 	if err != nil {
 		return 0, errors.Wrapf(err, "failed to count threads for user id=%s", userId)
 	}
 
-	return totalCount, nil
+	return totalThreads, nil
 }
 
 // GetTotalUnreadMentions counts the number of unread mentions for the given user, optionally
@@ -151,6 +164,7 @@ func (s *SqlThreadStore) GetTotalUnreadMentions(userId, teamId string, opts mode
 	query := s.getQueryBuilder().
 		Select("COALESCE(SUM(ThreadMemberships.UnreadMentions),0)").
 		From("ThreadMemberships").
+		LeftJoin("Threads ON Threads.PostId = ThreadMemberships.PostId").
 		Where(sq.Eq{
 			"ThreadMemberships.UserId":    userId,
 			"ThreadMemberships.Following": true,
@@ -158,7 +172,6 @@ func (s *SqlThreadStore) GetTotalUnreadMentions(userId, teamId string, opts mode
 
 	if teamId != "" {
 		query = query.
-			LeftJoin("Threads ON Threads.PostId = ThreadMemberships.PostId").
 			LeftJoin("Channels ON Threads.ChannelId = Channels.Id").
 			Where(sq.Or{
 				sq.Eq{"Channels.TeamId": teamId},
@@ -167,9 +180,7 @@ func (s *SqlThreadStore) GetTotalUnreadMentions(userId, teamId string, opts mode
 	}
 
 	if !opts.Deleted {
-		query = query.
+		query = query.Where(sq.Eq{"COALESCE(Threads.DeleteAt, 0)": 0})
-			LeftJoin("Posts ON Posts.Id = ThreadMemberships.PostId").
-			Where(sq.Eq{"COALESCE(Posts.DeleteAt, 0)": 0})
 	}
 
 	sql, args, err := query.ToSql()
@@ -177,7 +188,7 @@ func (s *SqlThreadStore) GetTotalUnreadMentions(userId, teamId string, opts mode
 		return 0, errors.Wrapf(err, "failed to build query to count unread mentions for user id=%s", userId)
 	}
 
-	err = s.GetMasterX().Get(&totalUnreadMentions, sql, args...)
+	err = s.GetReplicaX().Get(&totalUnreadMentions, sql, args...)
 	if err != nil {
 		return 0, errors.Wrapf(err, "failed to count unread mentions for user id=%s", userId)
 	}
@@ -199,6 +210,7 @@ func (s *SqlThreadStore) GetThreadsForUser(userId, teamId string, opts model.Get
 		UnreadReplies  int64
 		UnreadMentions int64
 		Participants   model.StringArray
+		ThreadDeleteAt int64
 		model.Post
 	}
 
@@ -217,12 +229,12 @@ func (s *SqlThreadStore) GetThreadsForUser(userId, teamId string, opts model.Get
 		return nil, errors.Wrapf(err, "failed to build subquery to count unread replies when getting threads for user id=%s", userId)
 	}
 
-	query := s.getQueryBuilder().
+	query := s.threadsAndPostsSelectQuery.
-		Select(`Threads.*,
+		Column(postSliceCoalesceQuery()).
-				` + postSliceCoalesceQuery() + `,
+		Columns(
-				ThreadMemberships.LastViewed as LastViewedAt,
+			"ThreadMemberships.LastViewed as LastViewedAt",
-				ThreadMemberships.UnreadMentions as UnreadMentions`).
+			"ThreadMemberships.UnreadMentions as UnreadMentions",
-		From("Threads").
+		).
 		Column(sq.Alias(sq.Expr(unreadRepliesSql, unreadRepliesArgs...), "UnreadReplies")).
 		Join("Posts ON Posts.Id = Threads.PostId").
 		Join("ThreadMemberships ON ThreadMemberships.PostId = Threads.PostId")
@@ -235,7 +247,7 @@ func (s *SqlThreadStore) GetThreadsForUser(userId, teamId string, opts model.Get
 	// a team at all.
 	if teamId != "" {
 		query = query.
-			Join("Channels ON Posts.ChannelId = Channels.Id").
+			Join("Channels ON Threads.ChannelId = Channels.Id").
 			Where(sq.Or{
 				sq.Eq{"Channels.TeamId": teamId},
 				sq.Eq{"Channels.TeamId": ""},
@@ -243,7 +255,10 @@ func (s *SqlThreadStore) GetThreadsForUser(userId, teamId string, opts model.Get
 	}
 
 	if !opts.Deleted {
-		query = query.Where(sq.Eq{"Posts.DeleteAt": 0})
+		query = query.Where(sq.Or{
+			sq.Eq{"Threads.DeleteAt": nil},
+			sq.Eq{"Threads.DeleteAt": 0},
+		})
 	}
 
 	if opts.Since > 0 {
@@ -256,11 +271,11 @@ func (s *SqlThreadStore) GetThreadsForUser(userId, teamId string, opts model.Get
 
 	order := "DESC"
 	if opts.Before != "" {
-		query = query.Where(sq.Expr(`LastReplyAt < (SELECT LastReplyAt FROM Threads WHERE PostId = ?)`, opts.Before))
+		query = query.Where(sq.Expr(`Threads.LastReplyAt < (SELECT LastReplyAt FROM Threads WHERE PostId = ?)`, opts.Before))
 	}
 	if opts.After != "" {
 		order = "ASC"
-		query = query.Where(sq.Expr(`LastReplyAt > (SELECT LastReplyAt FROM Threads WHERE PostId = ?)`, opts.After))
+		query = query.Where(sq.Expr(`Threads.LastReplyAt > (SELECT LastReplyAt FROM Threads WHERE PostId = ?)`, opts.After))
 	}
 
 	query = query.
@@ -323,6 +338,7 @@ func (s *SqlThreadStore) GetThreadsForUser(userId, teamId string, opts model.Get
 			UnreadMentions: thread.UnreadMentions,
 			Participants:   threadParticipants,
 			Post:           thread.Post.ToNilIfInvalid(),
+			DeleteAt:       thread.ThreadDeleteAt,
 		})
 	}
 
@@ -336,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(Posts.DeleteAt, 0)": 0},
+		sq.Eq{"COALESCE(Threads.DeleteAt, 0)": 0},
 	}
 
 	var wg sync.WaitGroup
@@ -362,7 +378,6 @@ func (s *SqlThreadStore) GetTeamsUnreadForUser(userID string, teamIDs []string)
 			From("Threads").
 			LeftJoin("ThreadMemberships ON Threads.PostId = ThreadMemberships.PostId").
 			LeftJoin("Channels ON Threads.ChannelId = Channels.Id").
-			LeftJoin("Posts ON Posts.Id = Threads.PostId").
 			Where(fetchConditions).
 			Where("Threads.LastReplyAt > ThreadMemberships.LastViewed").
 			GroupBy("Channels.TeamId").
@@ -385,7 +400,6 @@ func (s *SqlThreadStore) GetTeamsUnreadForUser(userID string, teamIDs []string)
 			Select("COALESCE(SUM(ThreadMemberships.UnreadMentions),0) AS Count, TeamId").
 			From("ThreadMemberships").
 			LeftJoin("Threads ON Threads.PostId = ThreadMemberships.PostId").
-			LeftJoin("Posts ON Posts.Id = ThreadMemberships.PostId").
 			LeftJoin("Channels ON Threads.ChannelId = Channels.Id").
 			Where(fetchConditions).
 			GroupBy("Channels.TeamId").
@@ -476,6 +490,7 @@ func (s *SqlThreadStore) GetThreadForUser(teamId string, threadMembership *model
 		UnreadReplies  int64
 		UnreadMentions int64
 		Participants   model.StringArray
+		ThreadDeleteAt int64
 		model.Post
 	}
 
@@ -496,21 +511,26 @@ func (s *SqlThreadStore) GetThreadForUser(teamId string, threadMembership *model
 		sq.Eq{"Threads.PostId": threadMembership.PostId},
 	}
 
+	query := s.threadsAndPostsSelectQuery
+
+	for _, c := range postSliceColumns() {
+		query = query.Column("Posts." + c)
+	}
+
 	var thread JoinedThread
-	query, threadArgs, err := s.getQueryBuilder().
+	querySQL, threadArgs, err := query.
-		Select("Threads.*, Posts.*").
-		From("Threads").
 		Column(sq.Alias(sq.Expr(unreadRepliesQuery), "UnreadReplies")).
 		LeftJoin("Posts ON Posts.Id = Threads.PostId").
 		LeftJoin("Channels ON Posts.ChannelId = Channels.Id").
-		Where(fetchConditions).ToSql()
+		Where(fetchConditions).
+		ToSql()
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to build query to get thread for user id=%s, post id=%s", threadMembership.UserId, threadMembership.PostId)
 	}
 
 	args := append(unreadRepliesArgs, threadArgs...)
 
-	err = s.GetReplicaX().Get(&thread, query, args...)
+	err = s.GetReplicaX().Get(&thread, querySQL, args...)
 	if err != nil {
 		if err == sql.ErrNoRows {
 			return nil, store.NewErrNotFound("Thread", threadMembership.PostId)
@@ -557,6 +577,7 @@ func (s *SqlThreadStore) GetThreadForUser(teamId string, threadMembership *model
 		UnreadMentions: thread.UnreadMentions,
 		Participants:   participants,
 		Post:           thread.Post.ToNilIfInvalid(),
+		DeleteAt:       thread.ThreadDeleteAt,
 	}
 
 	return result, nil

store/storetest/post_store.go

@@ -27,8 +27,6 @@ func TestPostStore(t *testing.T, ss store.Store, s SqlStore) {
 	t.Run("GetSingle", func(t *testing.T) { testPostStoreGetSingle(t, ss) })
 	t.Run("Update", func(t *testing.T) { testPostStoreUpdate(t, ss) })
 	t.Run("Delete", func(t *testing.T) { testPostStoreDelete(t, ss) })
-	t.Run("Delete1Level", func(t *testing.T) { testPostStoreDelete1Level(t, ss) })
-	t.Run("Delete2Level", func(t *testing.T) { testPostStoreDelete2Level(t, ss) })
 	t.Run("PermDelete1Level", func(t *testing.T) { testPostStorePermDelete1Level(t, ss) })
 	t.Run("PermDelete1Level2", func(t *testing.T) { testPostStorePermDelete1Level2(t, ss) })
 	t.Run("GetWithChildren", func(t *testing.T) { testPostStoreGetWithChildren(t, ss) })
@@ -317,6 +315,9 @@ func testPostStoreSaveMultiple(t *testing.T, ss store.Store) {
 		replyPost3.Message = NewTestId()
 		replyPost3.RootId = rootPost.Id
 
+		// Ensure update does not occur in the same timestamp as creation
+		time.Sleep(time.Millisecond)
+
 		_, _, err = ss.Post().SaveMultiple([]*model.Post{&replyPost2, &replyPost3})
 		require.NoError(t, err)
 
@@ -819,109 +820,132 @@ func testPostStoreUpdate(t *testing.T, ss store.Store) {
 }
 
 func testPostStoreDelete(t *testing.T, ss store.Store) {
-	o1 := &model.Post{}
+	t.Run("single post, no replies", func(t *testing.T) {
-	o1.ChannelId = model.NewId()
+		// Create a post
-	o1.UserId = model.NewId()
+		rootPost, err := ss.Post().Save(&model.Post{
-	o1.Message = model.NewRandomString(10)
+			ChannelId: model.NewId(),
-	deleteByID := model.NewId()
+			UserId:    model.NewId(),
+			Message:   model.NewRandomString(10),
+		})
+		require.NoError(t, err)
 
-	etag1 := ss.Post().GetEtag(o1.ChannelId, false, false)
+		// Verify etag generation for the channel containing the post.
-	require.Equal(t, 0, strings.Index(etag1, model.CurrentVersion+"."), "Invalid Etag")
+		etag1 := ss.Post().GetEtag(rootPost.ChannelId, false, false)
+		require.Equal(t, 0, strings.Index(etag1, model.CurrentVersion+"."), "Invalid Etag")
 
-	o1, err := ss.Post().Save(o1)
+		// Verify the created post.
-	require.NoError(t, err)
+		r1, err := ss.Post().Get(context.Background(), rootPost.Id, model.GetPostsOptions{}, "")
+		require.NoError(t, err)
+		require.NotNil(t, r1.Posts[rootPost.Id])
+		require.Equal(t, rootPost, r1.Posts[rootPost.Id])
 
-	r1, err := ss.Post().Get(context.Background(), o1.Id, model.GetPostsOptions{}, "")
+		// Mark the post as deleted by the user identified with deleteByID.
-	require.NoError(t, err)
+		deleteByID := model.NewId()
-	require.Equal(t, r1.Posts[o1.Id].CreateAt, o1.CreateAt, "invalid returned post")
+		err = ss.Post().Delete(rootPost.Id, model.GetMillis(), deleteByID)
+		require.NoError(t, err)
 
-	err = ss.Post().Delete(o1.Id, model.GetMillis(), deleteByID)
+		// Ensure the appropriate posts prop reflects the user deleting the post.
-	require.NoError(t, err)
+		posts, err := ss.Post().GetPostsCreatedAt(rootPost.ChannelId, rootPost.CreateAt)
+		require.NoError(t, err)
+		require.NotEmpty(t, posts)
+		assert.Equal(t, deleteByID, posts[0].GetProp(model.PostPropsDeleteBy), "unexpected Props[model.PostPropsDeleteBy]")
 
-	posts, _ := ss.Post().GetPostsCreatedAt(o1.ChannelId, o1.CreateAt)
+		// Verify that the post is no longer fetched by default.
-	post := posts[0]
+		_, err = ss.Post().Get(context.Background(), rootPost.Id, model.GetPostsOptions{}, "")
-	actual := post.GetProp(model.PostPropsDeleteBy)
+		require.Error(t, err, "fetching deleted post should have failed")
+		require.IsType(t, &store.ErrNotFound{}, err)
 
-	assert.Equal(t, deleteByID, actual, "Expected (*Post).Props[model.PostPropsDeleteBy] to be %v but got %v.", deleteByID, actual)
+		// Verify etag generation for the channel containing the now deleted post.
+		etag2 := ss.Post().GetEtag(rootPost.ChannelId, false, false)
+		require.Equal(t, 0, strings.Index(etag2, model.CurrentVersion+"."), "Invalid Etag")
+	})
 
-	r3, err := ss.Post().Get(context.Background(), o1.Id, model.GetPostsOptions{}, "")
+	t.Run("thread with one reply", func(t *testing.T) {
-	require.Error(t, err, "Missing id should have failed - PostList %v", r3)
+		// Create a root post
+		rootPost, err := ss.Post().Save(&model.Post{
+			ChannelId: model.NewId(),
+			UserId:    model.NewId(),
+			Message:   NewTestId(),
+		})
+		require.NoError(t, err)
 
-	etag2 := ss.Post().GetEtag(o1.ChannelId, false, false)
+		// Reply to that root post
-	require.Equal(t, 0, strings.Index(etag2, model.CurrentVersion+"."), "Invalid Etag")
+		replyPost, err := ss.Post().Save(&model.Post{
-}
+			ChannelId: rootPost.ChannelId,
+			UserId:    model.NewId(),
+			Message:   NewTestId(),
+			RootId:    rootPost.Id,
+		})
+		require.NoError(t, err)
 
-func testPostStoreDelete1Level(t *testing.T, ss store.Store) {
+		// Delete the root post
-	o1 := &model.Post{}
+		err = ss.Post().Delete(rootPost.Id, model.GetMillis(), "")
-	o1.ChannelId = model.NewId()
+		require.NoError(t, err)
-	o1.UserId = model.NewId()
-	o1.Message = NewTestId()
-	o1, err := ss.Post().Save(o1)
-	require.NoError(t, err)
 
-	o2 := &model.Post{}
+		// Verify the root post deleted
-	o2.ChannelId = o1.ChannelId
+		_, err = ss.Post().Get(context.Background(), rootPost.Id, model.GetPostsOptions{}, "")
-	o2.UserId = model.NewId()
+		require.Error(t, err, "Deleted id should have failed")
-	o2.Message = NewTestId()
+		require.IsType(t, &store.ErrNotFound{}, err)
-	o2.RootId = o1.Id
-	o2, err = ss.Post().Save(o2)
-	require.NoError(t, err)
 
-	err = ss.Post().Delete(o1.Id, model.GetMillis(), "")
+		// Verify the reply post deleted
-	require.NoError(t, err)
+		_, err = ss.Post().Get(context.Background(), replyPost.Id, model.GetPostsOptions{}, "")
+		require.Error(t, err, "Deleted id should have failed")
+		require.IsType(t, &store.ErrNotFound{}, err)
+	})
 
-	_, err = ss.Post().Get(context.Background(), o1.Id, model.GetPostsOptions{}, "")
+	t.Run("thread with multiple replies", func(t *testing.T) {
-	require.Error(t, err, "Deleted id should have failed")
+		// Create a root post
+		rootPost1, err := ss.Post().Save(&model.Post{
+			ChannelId: model.NewId(),
+			UserId:    model.NewId(),
+			Message:   NewTestId(),
+		})
+		require.NoError(t, err)
 
-	_, err = ss.Post().Get(context.Background(), o2.Id, model.GetPostsOptions{}, "")
+		// Reply to that root post
-	require.Error(t, err, "Deleted id should have failed")
+		replyPost1, err := ss.Post().Save(&model.Post{
-}
+			ChannelId: rootPost1.ChannelId,
+			UserId:    model.NewId(),
+			Message:   NewTestId(),
+			RootId:    rootPost1.Id,
+		})
+		require.NoError(t, err)
 
-func testPostStoreDelete2Level(t *testing.T, ss store.Store) {
+		// Reply to that root post a second time
-	o1 := &model.Post{}
+		replyPost2, err := ss.Post().Save(&model.Post{
-	o1.ChannelId = model.NewId()
+			ChannelId: rootPost1.ChannelId,
-	o1.UserId = model.NewId()
+			UserId:    model.NewId(),
-	o1.Message = NewTestId()
+			Message:   NewTestId(),
-	o1, err := ss.Post().Save(o1)
+			RootId:    rootPost1.Id,
-	require.NoError(t, err)
+		})
+		require.NoError(t, err)
 
-	o2 := &model.Post{}
+		// Create another root post in a separate channel
-	o2.ChannelId = o1.ChannelId
+		rootPost2, err := ss.Post().Save(&model.Post{
-	o2.UserId = model.NewId()
+			ChannelId: model.NewId(),
-	o2.Message = NewTestId()
+			UserId:    model.NewId(),
-	o2.RootId = o1.Id
+			Message:   NewTestId(),
-	o2, err = ss.Post().Save(o2)
+		})
-	require.NoError(t, err)
+		require.NoError(t, err)
 
-	o3 := &model.Post{}
+		// Delete the root post
-	o3.ChannelId = o1.ChannelId
+		err = ss.Post().Delete(rootPost1.Id, model.GetMillis(), "")
-	o3.UserId = model.NewId()
+		require.NoError(t, err)
-	o3.Message = NewTestId()
-	o3.RootId = o1.Id
-	o3, err = ss.Post().Save(o3)
-	require.NoError(t, err)
 
-	o4 := &model.Post{}
+		// Verify the root post and replies deleted
-	o4.ChannelId = model.NewId()
+		_, err = ss.Post().Get(context.Background(), rootPost1.Id, model.GetPostsOptions{}, "")
-	o4.UserId = model.NewId()
+		require.Error(t, err, "Deleted id should have failed")
-	o4.Message = NewTestId()
-	o4, err = ss.Post().Save(o4)
-	require.NoError(t, err)
 
-	err = ss.Post().Delete(o1.Id, model.GetMillis(), "")
+		_, err = ss.Post().Get(context.Background(), replyPost1.Id, model.GetPostsOptions{}, "")
-	require.NoError(t, err)
+		require.Error(t, err, "Deleted id should have failed")
 
-	_, err = ss.Post().Get(context.Background(), o1.Id, model.GetPostsOptions{}, "")
+		_, err = ss.Post().Get(context.Background(), replyPost2.Id, model.GetPostsOptions{}, "")
-	require.Error(t, err, "Deleted id should have failed")
+		require.Error(t, err, "Deleted id should have failed")
 
-	_, err = ss.Post().Get(context.Background(), o2.Id, model.GetPostsOptions{}, "")
+		// Verify other root posts remain undeleted.
-	require.Error(t, err, "Deleted id should have failed")
+		_, err = ss.Post().Get(context.Background(), rootPost2.Id, model.GetPostsOptions{}, "")
-
+		require.NoError(t, err)
-	_, err = ss.Post().Get(context.Background(), o3.Id, model.GetPostsOptions{}, "")
+	})
-	require.Error(t, err, "Deleted id should have failed")
-
-	_, err = ss.Post().Get(context.Background(), o4.Id, model.GetPostsOptions{}, "")
-	require.NoError(t, err)
 }
 
 func testPostStorePermDelete1Level(t *testing.T, ss store.Store) {

store/storetest/thread_store.go

@@ -684,6 +684,12 @@ func testGetTeamsUnreadForUser(t *testing.T, ss store.Store) {
 	assert.Equal(t, int64(1), teamsUnread[team2.Id].ThreadMentionCount)
 }
 
+type byPostId []*model.Post
+
+func (a byPostId) Len() int           { return len(a) }
+func (a byPostId) Swap(i, j int)      { a[i], a[j] = a[j], a[i] }
+func (a byPostId) Less(i, j int) bool { return a[i].Id < a[j].Id }
+
 func testVarious(t *testing.T, ss store.Store) {
 	createThreadMembership := func(userID, postID string, isMention bool) {
 		t.Helper()
@@ -818,6 +824,16 @@ func testVarious(t *testing.T, ss store.Store) {
 	})
 	require.NoError(t, err)
 
+	postNames := map[string]string{
+		team1channel1post1.Id:        "team1channel1post1",
+		team1channel1post2.Id:        "team1channel1post2",
+		team1channel1post3.Id:        "team1channel1post3",
+		team2channel1post1.Id:        "team2channel1post1",
+		team2channel1post2deleted.Id: "team2channel1post2deleted",
+		dm1post1.Id:                  "dm1post1",
+		gm1post1.Id:                  "gm1post1",
+	}
+
 	threadStoreCreateReply(t, ss, team1channel1.Id, team1channel1post1.Id, user2ID, model.GetMillis())
 	threadStoreCreateReply(t, ss, team1channel1.Id, team1channel1post2.Id, user2ID, model.GetMillis())
 	threadStoreCreateReply(t, ss, team1channel1.Id, team1channel1post3.Id, user2ID, model.GetMillis())
@@ -845,9 +861,37 @@ func testVarious(t *testing.T, ss store.Store) {
 	time.Sleep(1 * time.Millisecond)
 	threadStoreCreateReply(t, ss, team1channel1.Id, team1channel1post2.Id, user2ID, model.GetMillis())
 
+	// Actually make team2channel1post2deleted deleted
 	err = ss.Post().Delete(team2channel1post2deleted.Id, model.GetMillis(), user1ID)
 	require.NoError(t, err)
 
+	// Re-fetch posts to ensure metadata up-to-date
+	allPosts := []*model.Post{
+		team1channel1post1,
+		team1channel1post2,
+		team1channel1post3,
+		team2channel1post1,
+		team2channel1post2deleted,
+		dm1post1,
+		gm1post1,
+	}
+	for i := range allPosts {
+		updatedPost, err := ss.Post().GetSingle(allPosts[i].Id, true)
+		require.NoError(t, err)
+
+		// Fix some inconsistencies with how the post store returns posts vs. how the
+		// thread store returns it.
+		if updatedPost.RemoteId == nil {
+			updatedPost.RemoteId = new(string)
+		}
+
+		// Also, we don't populate ReplyCount for posts when querying threads, so don't
+		// assert same.
+		updatedPost.ReplyCount = 0
+
+		updatedPost.ShallowCopy(allPosts[i])
+	}
+
 	t.Run("GetTotalUnreadThreads", func(t *testing.T) {
 		testCases := []struct {
 			Description string
@@ -899,27 +943,15 @@ func testVarious(t *testing.T, ss store.Store) {
 			{"team1, user1", user1ID, team1.Id, model.GetUserThreadsOpts{}, []*model.Post{
 				team1channel1post1, team1channel1post2, team1channel1post3, dm1post1, gm1post1,
 			}},
-			{"team1, user1, unread", user1ID, team1.Id, model.GetUserThreadsOpts{Unread: true}, []*model.Post{
-				team1channel1post2, team1channel1post3, gm1post1,
-			}},
 			{"team1, user1, deleted", user1ID, team1.Id, model.GetUserThreadsOpts{Deleted: true}, []*model.Post{
 				team1channel1post1, team1channel1post2, team1channel1post3, dm1post1, gm1post1, // (no deleted threads in team1)
 			}},
-			{"team1, user1, unread + deleted", user1ID, team1.Id, model.GetUserThreadsOpts{Unread: true, Deleted: true}, []*model.Post{
-				team1channel1post2, team1channel1post3, gm1post1, // (no deleted threads in team1)
-			}},
 			{"team2, user1", user1ID, team2.Id, model.GetUserThreadsOpts{}, []*model.Post{
 				team2channel1post1, dm1post1, gm1post1,
 			}},
-			{"team2, user1, unread", user1ID, team2.Id, model.GetUserThreadsOpts{Unread: true}, []*model.Post{
-				gm1post1, // (no unread in team2)
-			}},
 			{"team2, user1, deleted", user1ID, team2.Id, model.GetUserThreadsOpts{Deleted: true}, []*model.Post{
 				team2channel1post1, team2channel1post2deleted, dm1post1, gm1post1,
 			}},
-			{"team2, user1, unread + deleted", user1ID, team2.Id, model.GetUserThreadsOpts{Unread: true, Deleted: true}, []*model.Post{
-				team2channel1post2deleted, gm1post1,
-			}},
 		}
 
 		for _, testCase := range testCases {
@@ -962,6 +994,46 @@ func testVarious(t *testing.T, ss store.Store) {
 		}
 	})
 
+	assertThreadPosts := func(t *testing.T, threads []*model.ThreadResponse, expectedPosts []*model.Post) {
+		t.Helper()
+
+		actualPosts := make([]*model.Post, 0, len(threads))
+		actualPostNames := make([]string, 0, len(threads))
+		for _, thread := range threads {
+			actualPosts = append(actualPosts, thread.Post)
+			postName, ok := postNames[thread.PostId]
+			require.True(t, ok, "failed to find actual %s in post names", thread.PostId)
+			actualPostNames = append(actualPostNames, postName)
+		}
+		sort.Strings(actualPostNames)
+
+		expectedPostNames := make([]string, 0, len(expectedPosts))
+		for _, post := range expectedPosts {
+			postName, ok := postNames[post.Id]
+			require.True(t, ok, "failed to find expected %s in post names", post.Id)
+			expectedPostNames = append(expectedPostNames, postName)
+		}
+		sort.Strings(expectedPostNames)
+
+		assert.Equal(t, expectedPostNames, actualPostNames)
+
+		// Check posts themselves
+		sort.Sort(byPostId(expectedPosts))
+		sort.Sort(byPostId(actualPosts))
+		if assert.Len(t, actualPosts, len(expectedPosts)) {
+			for i := range actualPosts {
+				assert.Equal(t, expectedPosts[i], actualPosts[i], "mismatch comparing expected post %s with actual post %s", postNames[expectedPosts[i].Id], postNames[actualPosts[i].Id])
+			}
+		} else {
+			assert.Equal(t, expectedPosts, actualPosts)
+		}
+
+		// Check common fields between threads and posts.
+		for _, thread := range threads {
+			assert.Equal(t, thread.DeleteAt, thread.Post.DeleteAt, "expected Thread.DeleteAt == Post.DeleteAt")
+		}
+	}
+
 	t.Run("GetThreadsForUser", func(t *testing.T) {
 		testCases := []struct {
 			Description string
@@ -1005,19 +1077,7 @@ func testVarious(t *testing.T, ss store.Store) {
 				threads, err := ss.Thread().GetThreadsForUser(testCase.UserID, testCase.TeamID, testCase.Options)
 				require.NoError(t, err)
 
-				postIDs := make([]string, 0, len(threads))
+				assertThreadPosts(t, threads, testCase.ExpectedThreads)
-				for _, thread := range threads {
-					postIDs = append(postIDs, thread.PostId)
-				}
-				sort.Strings(postIDs)
-
-				expectedPostIDs := make([]string, 0, len(testCase.ExpectedThreads))
-				for _, post := range testCase.ExpectedThreads {
-					expectedPostIDs = append(expectedPostIDs, post.Id)
-				}
-				sort.Strings(expectedPostIDs)
-
-				assert.Equal(t, expectedPostIDs, postIDs)
 			})
 		}
 	})

store/storetest/user_store.go

@@ -244,6 +244,9 @@ func testUserStoreUpdateUpdateAt(t *testing.T, ss store.Store) {
 	_, nErr := ss.Team().SaveMember(&model.TeamMember{TeamId: model.NewId(), UserId: u1.Id}, -1)
 	require.NoError(t, nErr)
 
+	// Ensure UpdateAt has a change to be different below.
+	time.Sleep(1 * time.Millisecond)
+
 	_, err = ss.User().UpdateUpdateAt(u1.Id)
 	require.NoError(t, err)