summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYorick Peterse <yorickpeterse@gmail.com>2018-02-21 13:43:04 +0000
committerYorick Peterse <yorickpeterse@gmail.com>2018-02-21 13:43:04 +0000
commit6d2a5f044f82f9500e8694a848125f976cb6d189 (patch)
tree653777c5cb45607e067e7db4d58608cdeec0420a
parentcc3a3fbf892dcdf36c116b8817cf9cb31740fec8 (diff)
parentb25319f92b096a70b84f645ff80af17954266fb5 (diff)
downloadgitlab-ce-6d2a5f044f82f9500e8694a848125f976cb6d189.tar.gz
Merge branch '41461-project-members-slow-due-to-sql' into 'master'
Simplify database queries in MembersFinder and improve performance for Projects::ProjectMembersController#index Closes #41461 See merge request gitlab-org/gitlab-ce!17190
-rw-r--r--app/finders/members_finder.rb57
-rw-r--r--changelogs/unreleased/41461-project-members-slow-due-to-sql.yml5
2 files changed, 50 insertions, 12 deletions
diff --git a/app/finders/members_finder.rb b/app/finders/members_finder.rb
index af24045886e..4734d97b8c7 100644
--- a/app/finders/members_finder.rb
+++ b/app/finders/members_finder.rb
@@ -10,26 +10,59 @@ class MembersFinder
def execute
project_members = project.project_members
project_members = project_members.non_invite unless can?(current_user, :admin_project, project)
- wheres = ["members.id IN (#{project_members.select(:id).to_sql})"]
if group
- # We need `.where.not(user_id: nil)` here otherwise when a group has an
- # invitee, it would make the following query return 0 rows since a NULL
- # user_id would be present in the subquery
- # See http://stackoverflow.com/questions/129077/not-in-clause-and-null-values
- non_null_user_ids = project_members.where.not(user_id: nil).select(:user_id)
-
group_members = GroupMembersFinder.new(group).execute
- group_members = group_members.where.not(user_id: non_null_user_ids)
- group_members = group_members.non_invite unless can?(current_user, :admin_group, group)
+ group_members = group_members.non_invite
- wheres << "members.id IN (#{group_members.select(:id).to_sql})"
- end
+ union = Gitlab::SQL::Union.new([project_members, group_members], remove_duplicates: false)
- Member.where(wheres.join(' OR '))
+ sql = distinct_on(union)
+
+ Member.includes(:user).from("(#{sql}) AS #{Member.table_name}")
+ else
+ project_members
+ end
end
def can?(*args)
Ability.allowed?(*args)
end
+
+ private
+
+ def distinct_on(union)
+ # We're interested in a list of members without duplicates by user_id.
+ # We prefer project members over group members, project members should go first.
+ if Gitlab::Database.postgresql?
+ <<~SQL
+ SELECT DISTINCT ON (user_id, invite_email) member_union.*
+ FROM (#{union.to_sql}) AS member_union
+ ORDER BY user_id,
+ invite_email,
+ CASE
+ WHEN type = 'ProjectMember' THEN 1
+ WHEN type = 'GroupMember' THEN 2
+ ELSE 3
+ END
+ SQL
+ else
+ # Older versions of MySQL do not support window functions (and DISTINCT ON is postgres-specific).
+ <<~SQL
+ SELECT t1.*
+ FROM (#{union.to_sql}) AS t1
+ JOIN (
+ SELECT
+ COALESCE(user_id, -1) AS user_id,
+ COALESCE(invite_email, 'NULL') AS invite_email,
+ MIN(CASE WHEN type = 'ProjectMember' THEN 1 WHEN type = 'GroupMember' THEN 2 ELSE 3 END) AS type_number
+ FROM
+ (#{union.to_sql}) AS t3
+ GROUP BY COALESCE(user_id, -1), COALESCE(invite_email, 'NULL')
+ ) AS t2 ON COALESCE(t1.user_id, -1) = t2.user_id
+ AND COALESCE(t1.invite_email, 'NULL') = t2.invite_email
+ AND CASE WHEN t1.type = 'ProjectMember' THEN 1 WHEN t1.type = 'GroupMember' THEN 2 ELSE 3 END = t2.type_number
+ SQL
+ end
+ end
end
diff --git a/changelogs/unreleased/41461-project-members-slow-due-to-sql.yml b/changelogs/unreleased/41461-project-members-slow-due-to-sql.yml
new file mode 100644
index 00000000000..27eee7d943b
--- /dev/null
+++ b/changelogs/unreleased/41461-project-members-slow-due-to-sql.yml
@@ -0,0 +1,5 @@
+---
+title: Improve query performance of MembersFinder.
+merge_request: 17190
+author:
+type: performance