Resolving group and project membership check cross joins
What does this MR do and why?
Addressing part of : #432604 (closed)
As part of the Cells project, we are changing many of the queries that are doing cross database joins to querying both databases main
and main_clusterwide
, while maintaining the current performance of the querying. For more on the topic, you can refer to this documentation.
This MR was extracted from a bigger MR: !138218 (closed)
This MR addresses mainly the views and specs of solving cross joins on Group#members
and Group#owners
.
To summarize the changes in this MR:
(Changed Query) Checking whether a user a direct owner of a group
This is changed on Group#owned_by?
https://postgres.ai/console/gitlab/gitlab-production-tunnel-pg12/sessions/24872/commands/79012
(Changed Query) Checking whether a user is one of the direct members of a group
This was common in many of the tests. By doing something like group.users
it 1) causes a cross join between members
and users
, which we are solving, and 2) it loads ALL the users from the users
table for no necessary reason. Instead, by replacing that with group.has_user?
(new method), which is using exists?
that uses a very efficient query on the members
table only.
This method is not used in normal code yet, but mainly used for tests as of now.
https://postgres.ai/console/gitlab/gitlab-production-tunnel-pg12/sessions/24872/commands/79013
(Changed Query) Counting the number of group members
Instead of group.users.count
, we can also use group.group_members.count
instead.
https://postgres.ai/console/gitlab/gitlab-production-tunnel-pg12/sessions/24872/commands/79014
Group Membership check in tests
Adding a new matcher expect(group).to have_user(user)
to make it easy to see if a user is a direct member of a group without cross join query or not.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #432604 (closed)