From 9667c96f97b743a16e585c344ec47daa4584943d Mon Sep 17 00:00:00 2001
From: Jesse Hallam <jesse.hallam@gmail.com>
Date: Mon, 11 Jan 2021 14:01:47 -0400
Subject: [PATCH] MM-30892: Avoid unnecessary table scan in AnalyticsPostCount.
 (#16643)

* MM-30892: Avoid unnecessary table scan in AnalyticsPostCount.

[AnalyticsPostCount](https://github.com/mattermost/mattermost-server/blob/d9e8402dc27bd11137e9301cf14eaf01d2786efd/store/sqlstore/post_store.go#L1513-L1541) unconditionally joins on the `Channels` table even when not filtering to a team.

This is a proposal to stop doing this, reducing this to a simple index query when no team is specified. Note that this is technically a subtle semantic difference, since orphaned posts (from deleted or invalid channel ids) are currently excluded. But I submit the benefits outweigh the technical differences here.

Fixes: https://mattermost.atlassian.net/browse/MM-30892

* Update store/sqlstore/post_store.go

Co-authored-by: Claudio Costa <cstcld91@gmail.com>

* Update store/sqlstore/post_store.go

Co-authored-by: Claudio Costa <cstcld91@gmail.com>

Co-authored-by: Claudio Costa <cstcld91@gmail.com>
---
 store/sqlstore/post_store.go  | 26 ++++++++++++++------------
 store/storetest/post_store.go | 25 +++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/store/sqlstore/post_store.go b/store/sqlstore/post_store.go
index 8c78d31f5d..bfd1a480ca 100644
--- a/store/sqlstore/post_store.go
+++ b/store/sqlstore/post_store.go
@@ -1511,28 +1511,30 @@ func (s *SqlPostStore) AnalyticsPostCountsByDay(options *model.AnalyticsPostCoun
 }
 
 func (s *SqlPostStore) AnalyticsPostCount(teamId string, mustHaveFile bool, mustHaveHashtag bool) (int64, error) {
-	query :=
-		`SELECT
-			COUNT(Posts.Id) AS Value
-		FROM
-			Posts,
-			Channels
-		WHERE
-			Posts.ChannelId = Channels.Id`
+	query := s.getQueryBuilder().
+		Select("COUNT(p.Id) AS Value").
+		From("Posts p")
 
 	if len(teamId) > 0 {
-		query += " AND Channels.TeamId = :TeamId"
+		query = query.
+			Join("Channels c ON (c.Id = p.ChannelId)").
+			Where(sq.Eq{"c.TeamId": teamId})
 	}
 
 	if mustHaveFile {
-		query += " AND (Posts.FileIds != '[]' OR Posts.Filenames != '[]')"
+		query = query.Where(sq.Or{sq.NotEq{"p.FileIds": "[]"}, sq.NotEq{"p.Filenames": "[]"}})
 	}
 
 	if mustHaveHashtag {
-		query += " AND Posts.Hashtags != ''"
+		query = query.Where(sq.NotEq{"p.Hashtags": ""})
 	}
 
-	v, err := s.GetReplica().SelectInt(query, map[string]interface{}{"TeamId": teamId})
+	queryString, args, err := query.ToSql()
+	if err != nil {
+		return 0, errors.Wrap(err, "post_tosql")
+	}
+
+	v, err := s.GetReplica().SelectInt(queryString, args...)
 	if err != nil {
 		return 0, errors.Wrap(err, "failed to count Posts")
 	}
diff --git a/store/storetest/post_store.go b/store/storetest/post_store.go
index affb48670b..2d244b9786 100644
--- a/store/storetest/post_store.go
+++ b/store/storetest/post_store.go
@@ -1602,6 +1602,7 @@ func testPostCountsByDay(t *testing.T, ss store.Store) {
 	o1.UserId = model.NewId()
 	o1.CreateAt = utils.MillisFromTime(utils.Yesterday())
 	o1.Message = "zz" + model.NewId() + "b"
+	o1.Hashtags = "hashtag"
 	o1, nErr = ss.Post().Save(o1)
 	require.Nil(t, nErr)
 
@@ -1610,6 +1611,7 @@ func testPostCountsByDay(t *testing.T, ss store.Store) {
 	o1a.UserId = model.NewId()
 	o1a.CreateAt = o1.CreateAt
 	o1a.Message = "zz" + model.NewId() + "b"
+	o1a.FileIds = []string{"fileId1"}
 	_, nErr = ss.Post().Save(o1a)
 	require.Nil(t, nErr)
 
@@ -1618,6 +1620,7 @@ func testPostCountsByDay(t *testing.T, ss store.Store) {
 	o2.UserId = model.NewId()
 	o2.CreateAt = o1.CreateAt - (1000 * 60 * 60 * 24 * 2)
 	o2.Message = "zz" + model.NewId() + "b"
+	o2.Filenames = []string{"filename1"}
 	o2, nErr = ss.Post().Save(o2)
 	require.Nil(t, nErr)
 
@@ -1626,6 +1629,8 @@ func testPostCountsByDay(t *testing.T, ss store.Store) {
 	o2a.UserId = o2.UserId
 	o2a.CreateAt = o1.CreateAt - (1000 * 60 * 60 * 24 * 2)
 	o2a.Message = "zz" + model.NewId() + "b"
+	o2a.Hashtags = "hashtag"
+	o2a.FileIds = []string{"fileId2"}
 	_, nErr = ss.Post().Save(o2a)
 	require.Nil(t, nErr)
 
@@ -1690,6 +1695,26 @@ func testPostCountsByDay(t *testing.T, ss store.Store) {
 	r2, err := ss.Post().AnalyticsPostCount(t1.Id, false, false)
 	require.Nil(t, err)
 	assert.Equal(t, int64(6), r2)
+
+	// total across teams
+	r2, err = ss.Post().AnalyticsPostCount("", false, false)
+	require.Nil(t, err)
+	assert.GreaterOrEqual(t, r2, int64(6))
+
+	// total across teams with files
+	r2, err = ss.Post().AnalyticsPostCount("", true, false)
+	require.Nil(t, err)
+	assert.GreaterOrEqual(t, r2, int64(3))
+
+	// total across teams with hastags
+	r2, err = ss.Post().AnalyticsPostCount("", false, true)
+	require.Nil(t, err)
+	assert.GreaterOrEqual(t, r2, int64(2))
+
+	// total across teams with hastags and files
+	r2, err = ss.Post().AnalyticsPostCount("", true, true)
+	require.Nil(t, err)
+	assert.GreaterOrEqual(t, r2, int64(1))
 }
 
 func testPostStoreGetFlaggedPostsForTeam(t *testing.T, ss store.Store, s SqlStore) {