From 27bad28c530c89acab35a56b945b6a3924280f4b Mon Sep 17 00:00:00 2001
From: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Date: Wed, 15 Sep 2021 11:32:10 +0800
Subject: [PATCH] Partially revert "PERF: Improve query performance all inbox
 private messages. (#14304)" (#14344)

This partially reverts commit ddb458343dc39a7a8c99467dcd809b444514fe2c.

Seeing performance degrade on larger sites so back to drawing board on
this one. Instead of the DISTINCT LEFT JOIN, we switch back to
IN(subquery).
---
 app/controllers/topics_controller.rb     | 12 ++---
 lib/topic_query/private_message_lists.rb | 58 +++++++++++++++---------
 2 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
index 4de9618a941..b23245cc1be 100644
--- a/app/controllers/topics_controller.rb
+++ b/app/controllers/topics_controller.rb
@@ -973,7 +973,7 @@ class TopicsController < ApplicationController
 
     topic_ids = TopicsBulkAction.new(
       current_user,
-      topic_scope.distinct(false).pluck(:id),
+      topic_scope.pluck(:id),
       type: "dismiss_topics"
     ).perform!
 
@@ -1245,11 +1245,7 @@ class TopicsController < ApplicationController
     if inbox = params[:private_message_inbox]
       filter = private_message_filter(topic_query, inbox)
       topic_query.options[:limit] = false
-
-      topic_query
-        .filter_private_messages_unread(current_user, filter)
-        .distinct(false)
-        .pluck(:id)
+      topics = topic_query.filter_private_messages_unread(current_user, filter)
     else
       topics = TopicQuery.unread_filter(topic_query.joined_topic_user, staff: guardian.is_staff?).listable_topics
       topics = TopicQuery.tracked_filter(topics, current_user.id) if params[:tracked].to_s == "true"
@@ -1268,9 +1264,9 @@ class TopicsController < ApplicationController
       if params[:tag_name].present?
         topics = topics.joins(:tags).where("tags.name": params[:tag_name])
       end
-
-      topics.pluck(:id)
     end
+
+    topics.pluck(:id)
   end
 
   def private_message_filter(topic_query, inbox)
diff --git a/lib/topic_query/private_message_lists.rb b/lib/topic_query/private_message_lists.rb
index 1a0c770a730..c056d5946bf 100644
--- a/lib/topic_query/private_message_lists.rb
+++ b/lib/topic_query/private_message_lists.rb
@@ -145,18 +145,25 @@ class TopicQuery
       elsif type == :all
         group_ids = group_with_messages_ids(user)
 
-        result = result.joins(<<~SQL)
-        LEFT JOIN topic_allowed_users tau
-          ON tau.topic_id = topics.id
-          AND tau.user_id = #{user.id.to_i}
-        LEFT JOIN topic_allowed_groups tag
-          ON tag.topic_id = topics.id
-          #{group_ids.present? ? "AND tag.group_id IN (#{group_ids.join(",")})" : ""}
-        SQL
-
-        result = result
-          .where("tag.topic_id IS NOT NULL OR tau.topic_id IS NOT NULL")
-          .distinct
+        result =
+        if group_ids.present?
+          result.where(<<~SQL)
+            topics.id IN (
+              SELECT topic_id
+              FROM topic_allowed_users
+              WHERE user_id = #{user.id.to_i}
+              UNION ALL
+              SELECT topic_id FROM topic_allowed_groups
+              WHERE group_id IN (#{group_ids.join(",")})
+            )
+          SQL
+        else
+          result.joins(<<~SQL)
+          INNER JOIN topic_allowed_users tau
+            ON tau.topic_id = topics.id
+            AND tau.user_id = #{user.id.to_i}
+          SQL
+        end
       end
 
       result = result.joins("LEFT OUTER JOIN topic_users AS tu ON (topics.id = tu.topic_id AND tu.user_id = #{user.id.to_i})")
@@ -238,23 +245,30 @@ class TopicQuery
       # query here as it can easily lead to an inefficient query.
       group_ids = group_with_messages_ids(user)
 
-      list = list.joins(<<~SQL)
-      LEFT JOIN group_archived_messages gm
-        ON gm.topic_id = topics.id
-        #{group_ids.present? ? "AND gm.group_id IN (#{group_ids.join(",")})" : ""}
-      LEFT JOIN user_archived_messages um
-        ON um.user_id = #{user.id.to_i}
-        AND um.topic_id = topics.id
-      SQL
+      if group_ids.present?
+        list = list.joins(<<~SQL)
+          LEFT JOIN group_archived_messages gm
+            ON gm.topic_id = topics.id
+            AND gm.group_id IN (#{group_ids.join(",")})
+          LEFT JOIN user_archived_messages um
+            ON um.user_id = #{user.id.to_i}
+            AND um.topic_id = topics.id
+        SQL
 
-      list =
         if archived
           list.where("um.user_id IS NOT NULL OR gm.topic_id IS NOT NULL")
         else
           list.where("um.user_id IS NULL AND gm.topic_id IS NULL")
         end
+      else
+        list = list.joins(<<~SQL)
+          LEFT JOIN user_archived_messages um
+          ON um.user_id = #{user.id.to_i}
+          AND um.topic_id = topics.id
+        SQL
 
-      list
+        list.where("um.user_id IS #{archived ? "NOT NULL" : "NULL"}")
+      end
     end
 
     def not_archived(list, user)