2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-09-07 12:02:53 +08:00

PERF: Optimize search in private messages query (#14660)

* PERF: Remove JOIN on categories for PM search

JOIN on categories is not needed when searchin in private messages as
PMs are not categorized.

* DEV: Use == for string comparison

* PERF: Optimize query for allowed topic groups

There was a query that checked for all topics a user or their groups
were allowed to see. This used UNION between topic_allowed_users and
topic_allowed_groups which was very inefficient. That was replaced with
a OR condition that checks in either tables more efficiently.
This commit is contained in:
Dan Ungureanu 2021-10-26 10:16:38 +03:00 committed by GitHub
parent f6528afa01
commit f003e31e2f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 73 additions and 56 deletions

View file

@ -84,9 +84,13 @@ class Post < ActiveRecord::Base
register_custom_field_type(NOTICE, :json) register_custom_field_type(NOTICE, :json)
scope :private_posts_for_user, ->(user) { scope :private_posts_for_user, ->(user) do
where("posts.topic_id IN (#{Topic::PRIVATE_MESSAGES_SQL})", user_id: user.id) where(
} "posts.topic_id IN (#{Topic::PRIVATE_MESSAGES_SQL_USER})
OR posts.topic_id IN (#{Topic::PRIVATE_MESSAGES_SQL_GROUP})",
user_id: user.id
)
end
scope :by_newest, -> { order('created_at DESC, id DESC') } scope :by_newest, -> { order('created_at DESC, id DESC') }
scope :by_post_number, -> { order('post_number ASC') } scope :by_post_number, -> { order('post_number ASC') }

View file

@ -267,19 +267,25 @@ class Topic < ActiveRecord::Base
# Return private message topics # Return private message topics
scope :private_messages, -> { where(archetype: Archetype.private_message) } scope :private_messages, -> { where(archetype: Archetype.private_message) }
PRIVATE_MESSAGES_SQL = <<~SQL PRIVATE_MESSAGES_SQL_USER = <<~SQL
SELECT topic_id SELECT topic_id
FROM topic_allowed_users FROM topic_allowed_users
WHERE user_id = :user_id WHERE user_id = :user_id
UNION ALL SQL
PRIVATE_MESSAGES_SQL_GROUP = <<~SQL
SELECT tg.topic_id SELECT tg.topic_id
FROM topic_allowed_groups tg FROM topic_allowed_groups tg
JOIN group_users gu ON gu.user_id = :user_id AND gu.group_id = tg.group_id JOIN group_users gu ON gu.user_id = :user_id AND gu.group_id = tg.group_id
SQL SQL
scope :private_messages_for_user, ->(user) { scope :private_messages_for_user, ->(user) do
private_messages.where("topics.id IN (#{PRIVATE_MESSAGES_SQL})", user_id: user.id) private_messages.where(
} "topics.id IN (#{PRIVATE_MESSAGES_SQL_USER})
OR topics.id IN (#{PRIVATE_MESSAGES_SQL_GROUP})",
user_id: user.id
)
end
scope :listable_topics, -> { where('topics.archetype <> ?', Archetype.private_message) } scope :listable_topics, -> { where('topics.archetype <> ?', Archetype.private_message) }

View file

@ -895,12 +895,15 @@ class Search
def posts_query(limit, type_filter: nil, aggregate_search: false) def posts_query(limit, type_filter: nil, aggregate_search: false)
posts = Post.where(post_type: Topic.visible_post_types(@guardian.user)) posts = Post.where(post_type: Topic.visible_post_types(@guardian.user))
.joins(:post_search_data, :topic) .joins(:post_search_data, :topic)
.joins("LEFT JOIN categories ON categories.id = topics.category_id")
if type_filter != "private_messages"
posts = posts.joins("LEFT JOIN categories ON categories.id = topics.category_id")
end
is_topic_search = @search_context.present? && @search_context.is_a?(Topic) is_topic_search = @search_context.present? && @search_context.is_a?(Topic)
posts = posts.where("topics.visible") unless is_topic_search posts = posts.where("topics.visible") unless is_topic_search
if type_filter === "private_messages" || (is_topic_search && @search_context.private_message?) if type_filter == "private_messages" || (is_topic_search && @search_context.private_message?)
posts = posts posts = posts
.where( .where(
"topics.archetype = ? AND post_search_data.private_message", "topics.archetype = ? AND post_search_data.private_message",
@ -910,7 +913,7 @@ class Search
unless @guardian.is_admin? unless @guardian.is_admin?
posts = posts.private_posts_for_user(@guardian.user) posts = posts.private_posts_for_user(@guardian.user)
end end
elsif type_filter === "all_topics" elsif type_filter == "all_topics"
private_posts = posts private_posts = posts
.where( .where(
"topics.archetype = ? AND post_search_data.private_message", "topics.archetype = ? AND post_search_data.private_message",
@ -973,7 +976,7 @@ class Search
posts = posts =
if @search_context.present? if @search_context.present?
if @search_context.is_a?(User) if @search_context.is_a?(User)
if type_filter === "private_messages" if type_filter == "private_messages"
if @guardian.is_admin? && !@search_all_pms if @guardian.is_admin? && !@search_all_pms
posts.private_posts_for_user(@search_context) posts.private_posts_for_user(@search_context)
else else
@ -1036,6 +1039,7 @@ class Search
) )
SQL SQL
if type_filter != "private_messages"
category_search_priority = <<~SQL category_search_priority = <<~SQL
( (
CASE categories.search_priority CASE categories.search_priority
@ -1077,16 +1081,19 @@ class Search
else else
posts.order("#{category_search_priority} DESC", "#{data_ranking} DESC") posts.order("#{category_search_priority} DESC", "#{data_ranking} DESC")
end end
end
posts = posts.order("topics.bumped_at DESC") posts = posts.order("topics.bumped_at DESC")
end end
if type_filter != "private_messages"
posts = posts =
if secure_category_ids.present? if secure_category_ids.present?
posts.where("(categories.id IS NULL) OR (NOT categories.read_restricted) OR (categories.id IN (?))", secure_category_ids).references(:categories) posts.where("(categories.id IS NULL) OR (NOT categories.read_restricted) OR (categories.id IN (?))", secure_category_ids).references(:categories)
else else
posts.where("(categories.id IS NULL) OR (NOT categories.read_restricted)").references(:categories) posts.where("(categories.id IS NULL) OR (NOT categories.read_restricted)").references(:categories)
end end
end
if @order if @order
advanced_order = Search.advanced_orders&.fetch(@order, nil) advanced_order = Search.advanced_orders&.fetch(@order, nil)