From d9714c21c80ab3b8ff68a5520d8086aa8e91588f Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Fri, 15 Sep 2017 22:21:05 +0800 Subject: [PATCH] Revert "PERF: Avoid unnecessary expensive joins if possible." This reverts commit f3fadf41b7d723265094bb90203226941defce58. * This ended up exposing group pms to users that are not part of a group. --- lib/topic_query.rb | 89 ++++++++++++++++++++-------------------------- 1 file changed, 38 insertions(+), 51 deletions(-) diff --git a/lib/topic_query.rb b/lib/topic_query.rb index a751fad530b..afadc2ddbbd 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -744,60 +744,53 @@ class TopicQuery end def related_messages_user(params) - messages = messages_for_user.limit(params[:count]) - messages = allowed_messages(messages, 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") end def related_messages_group(params) - messages = messages_for_groups_or_user(params[:my_group_ids]).limit(params[:count]) - messages = allowed_messages(messages, params) - end + 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]) - 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 - .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") + .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) else messages_for_user end @@ -854,10 +847,4 @@ 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