@@ -88,54 +88,34 @@ def logged_in_non_admin_allowed_to(user, permissions)
8888 WHERE entity_id IS NULL
8989 SQL
9090
91- # Remove those entries from before that are
92- # * entity (WorkPackage) specific AND
93- # * have the same project as a non-entity specific entry.
94- # That is the case if a work package is shared with a user
95- # while the user is already a member in the project.
96- # Since the allowed_to filtering is already specific to the permissions, that removal is safe.
97- entity_member_projects_without_duplicates = Arel . sql ( <<~SQL . squish )
98- SELECT * FROM entity_member_projects
99- WHERE NOT EXISTS (
100- SELECT 1 FROM project_member_projects
101- WHERE project_member_projects.id = entity_member_projects.id
102- )
103- SQL
104-
10591 # Take all work packages allowed by either project-wide or entity-specific membership.
106- # But now remove all those that are in a project for which an entity-specific membership exists that is not
107- # for that entity (work package).
108- # An alternative way of formulating this would be by comparing
109- # * That the project_id matches AND
110- # * the entity_id matches OR the entity_id is null
111- # ```
112- # SELECT * from work_packages
113- # WHERE EXISTS (
114- # SELECT 1 FROM allowed_projects projects
115- # WHERE projects.id = work_packages.project_id
116- # AND (projects.entity_id = work_packages.id OR projects.entity_id IS NULL)
117- # )
118- # ```
119- # Postgresql however sometimes turns to a sequential scan with the query above.
92+ # PostgreSQL however sometimes turns to a sequential scan with the query above.
12093 #
121- # Index scans can still happen in the combination of the CTE with the check outside of the
122- # CTEs for the existence of any record.
123- # This is particularly likely in case AR.exists? is used which adds a LIMIT 1
94+ # It is currently unclear if index scans can still happen in the combination of the CTE with the check
95+ # outside of the CTEs for the existence of any record.
96+ # This happened in the past, before changing this CTE to a UNION, in case AR.exists? is used which adds a LIMIT 1
12497 # to the query. In this case, there is a known shortcoming that PostgreSQL's query planner
12598 # will make poor choices
12699 # (https://www.postgresql.org/message-id/flat/CA%2BU5nMLbXfUT9cWDHJ3tpxjC3bTWqizBKqTwDgzebCB5bAGCgg%40mail.gmail.com).
127100 #
128101 # Once AR supports adding materialization hints (https://github.com/rails/rails/pull/54322), the inner
129102 # `allowed` CTE can be abandoned as it is only used for being able to provide such a hint.
103+ # Having the inner materialized CTE has no known negative side effects which is why it is kept.
130104 allowed_by_projects_and_work_packages = Arel . sql ( <<~SQL . squish )
131105 WITH allowed AS MATERIALIZED (
132- SELECT id from work_packages
133- WHERE project_id in (SELECT id FROM member_projects)
134- AND NOT EXISTS (
135- SELECT 1 FROM entity_member_projects_without_duplicates
136- WHERE entity_member_projects_without_duplicates.id = work_packages.project_id
137- AND entity_member_projects_without_duplicates.entity_id != work_packages.id
138- )
106+ SELECT
107+ work_packages.id
108+ FROM
109+ work_packages
110+ JOIN project_member_projects ON project_member_projects.id = work_packages.project_id
111+
112+ UNION
113+
114+ SELECT
115+ work_packages.id
116+ FROM
117+ work_packages
118+ JOIN entity_member_projects ON entity_member_projects.entity_id = work_packages.id
139119 )
140120
141121 SELECT * from allowed
@@ -144,7 +124,6 @@ def logged_in_non_admin_allowed_to(user, permissions)
144124 with ( member_projects : Arel . sql ( allowed_via_project_or_work_package_membership . to_sql ) ,
145125 entity_member_projects :,
146126 project_member_projects :,
147- entity_member_projects_without_duplicates :,
148127 allowed_by_projects_and_work_packages :)
149128 . where ( <<~SQL . squish )
150129 EXISTS (
0 commit comments