Skip to content
Permalink
Browse files Browse the repository at this point in the history
Partially revert "PERF: Improve query performance all inbox private m…
…essages. (#14304)" (#14344)

This partially reverts commit ddb4583.

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).
  • Loading branch information
tgxworld committed Sep 15, 2021
1 parent ddb4583 commit 27bad28
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 30 deletions.
12 changes: 4 additions & 8 deletions app/controllers/topics_controller.rb
Expand Up @@ -973,7 +973,7 @@ def private_message_reset_new

topic_ids = TopicsBulkAction.new(
current_user,
topic_scope.distinct(false).pluck(:id),
topic_scope.pluck(:id),
type: "dismiss_topics"
).perform!

Expand Down Expand Up @@ -1245,11 +1245,7 @@ def bulk_unread_topic_ids
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"
Expand All @@ -1268,9 +1264,9 @@ def bulk_unread_topic_ids
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)
Expand Down
58 changes: 36 additions & 22 deletions lib/topic_query/private_message_lists.rb
Expand Up @@ -145,18 +145,25 @@ def private_messages_for(user, type)
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})")
Expand Down Expand Up @@ -238,23 +245,30 @@ def filter_archived(list, user, archived: true)
# 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)
Expand Down

1 comment on commit 27bad28

@discoursebot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/inboxes-list-all-messages/203852/1

Please sign in to comment.