From f3fadf41b7d723265094bb90203226941defce58 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Thu, 14 Sep 2017 11:12:59 +0800 Subject: [PATCH] PERF: Avoid unnecessary expensive joins if possible. ``` EXPLAIN ANALYZE SELECT "topics".* FROM "topics" LEFT JOIN topic_users tu ON topics.id = tu.topic_id AND tu.user_id = 13455 WHERE ("topics"."deleted_at" IS NULL) AND (topics.archetype = 'private_message') AND ( topics.id IN ( SELECT topic_id FROM topic_allowed_groups tg JOIN group_users gu ON gu.user_id = 13455 AND gu.group_id = tg.group_id WHERE gu.group_id IN (47) ) ) AND ( topics.id IN ( SELECT ta.topic_id FROM topic_allowed_users ta WHERE ta.user_id IN (32852,-10) ) OR topics.id IN ( SELECT tg.topic_id FROM topic_allowed_groups tg WHERE tg.group_id IN (-10) ) ) AND (topics.id NOT IN (69933,69995,69988,69984,69968,69973,69971,69952)) AND "topics"."visible" = 't' ORDER BY topics.bumped_at DESC LIMIT 3; ``` Planning time: 1.277 ms Execution time: 71.577 ms ``` EXPLAIN ANALYZE SELECT "topics".* FROM "topics" LEFT JOIN topic_users tu ON topics.id = tu.topic_id AND tu.user_id = 13455 LEFT JOIN ( SELECT * FROM topic_allowed_groups _tg LEFT JOIN group_users gu ON gu.user_id = 13455 AND gu.group_id = _tg.group_id AND gu.group_id IN (47) ) tg ON topics.id = tg.topic_id LEFT JOIN topic_allowed_users ta2 ON topics.id = ta2.topic_id AND ta2.user_id IN (32852) WHERE ("topics"."deleted_at" IS NULL) AND (topics.archetype = 'private_message') AND (tg.topic_id IS NOT NULL) AND (ta2.topic_id IS NOT NULL) AND (topics.id NOT IN (69933,69995,69988,69984,69968,69973,69971,69952)) AND "topics"."visible" = 't' ORDER BY topics.bumped_at DESC LIMIT 3; ``` Planning time: 1.191 ms Execution time: 0.129 ms --- lib/topic_query.rb | 89 ++++++++++++++++++++++++++-------------------- 1 file changed, 51 insertions(+), 38 deletions(-) diff --git a/lib/topic_query.rb b/lib/topic_query.rb index 541085a7618..2e99ca16d4c 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -748,53 +748,60 @@ class TopicQuery end def related_messages_user(params) - user_ids = ((params[:target_user_ids] || []) << -10) - user_ids = ActiveRecord::Base.send(:sanitize_sql_array, user_ids.join(',')) - group_ids = (((params[:target_group_ids] - params[:my_group_ids]) || []) << -10) - group_ids = ActiveRecord::Base.send(:sanitize_sql_array, group_ids.join(',')) - - result = messages_for_user - .limit(params[:count]) - .joins(" - LEFT JOIN topic_allowed_users ta2 - ON topics.id = ta2.topic_id - AND ta2.user_id IN (#{user_ids})", - ) - .joins(" - LEFT JOIN topic_allowed_groups tg - ON topics.id = tg.topic_id - AND tg.group_id IN (#{group_ids})", - ) - .where("ta2.topic_id IS NOT NULL OR tg.topic_id IS NOT NULL") + messages = messages_for_user.limit(params[:count]) + messages = allowed_messages(messages, params) end def related_messages_group(params) - messages_for_groups_or_user(params[:my_group_ids]) - .limit(params[:count]) - .where('topics.id IN ( - SELECT ta.topic_id - FROM topic_allowed_users ta - WHERE ta.user_id IN (:user_ids) - ) OR - topics.id IN ( - SELECT tg.topic_id - FROM topic_allowed_groups tg - WHERE tg.group_id IN (:group_ids) - ) - ', user_ids: (params[:target_user_ids] || []) + [-10], - group_ids: ((params[:target_group_ids] - params[:my_group_ids]) || []) + [-10]) + messages = messages_for_groups_or_user(params[:my_group_ids]).limit(params[:count]) + messages = allowed_messages(messages, params) + end + def allowed_messages(messages, params) + user_ids = (params[:target_user_ids] || []) + group_ids = ((params[:target_group_ids] - params[:my_group_ids]) || []) + + if user_ids.present? + messages = + messages.joins(" + LEFT JOIN topic_allowed_users ta2 + ON topics.id = ta2.topic_id + AND ta2.user_id IN (#{sanitize_sql_array(user_ids)}) + ") + end + + if group_ids.present? + messages = + messages.joins(" + LEFT JOIN topic_allowed_groups tg2 + ON topics.id = tg2.topic_id + AND tg2.group_id IN (#{sanitize_sql_array(group_ids)}) + ") + end + + messages = + if user_ids.present? && group_ids.present? + messages.where("ta2.topic_id IS NOT NULL OR tg2.topic_id IS NOT NULL") + elsif user_ids.present? + messages.where("ta2.topic_id IS NOT NULL") + elsif group_ids.present? + messages.where("tg2.topic_id IS NOT NULL") + end end def messages_for_groups_or_user(group_ids) if group_ids.present? base_messages - .where('topics.id IN ( - SELECT topic_id - FROM topic_allowed_groups tg - JOIN group_users gu ON gu.user_id = :user_id AND gu.group_id = tg.group_id - WHERE gu.group_id IN (:group_ids) - )', user_id: @user.id, group_ids: group_ids) + .joins(" + LEFT JOIN ( + SELECT * FROM topic_allowed_groups _tg + LEFT JOIN group_users gu + ON gu.user_id = #{@user.id.to_i} + AND gu.group_id = _tg.group_id + AND gu.group_id IN (#{sanitize_sql_array(group_ids)}) + ) tg ON topics.id = tg.topic_id + ") + .where("tg.topic_id IS NOT NULL") else messages_for_user end @@ -851,4 +858,10 @@ class TopicQuery result.order('topics.bumped_at DESC') end + + private + + def sanitize_sql_array(input) + ActiveRecord::Base.send(:sanitize_sql_array, input.join(',')) + end end