Use use_includes: false where necessary
What does this MR do and why?
where_full_path_in
method has the following definition now:
def where_full_path_in(paths, use_includes: true)
return none if paths.empty?
wheres = paths.map do |path|
"(LOWER(routes.path) = LOWER(#{connection.quote(path)}))"
end
route =
if use_includes
includes(:route).references(:routes)
else
joins(:route)
end
route
.where(wheres.join(' OR '))
.allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/420046")
end
use_includes: true
is the default, and when used, the routes
records are included to prevent N+1.
However, there are different areas in our codebase where routes
are loaded, but that loading isn't necessary because the result of this method is just used to pluck IDs of projects or groups.
Right now, this isn't much of a problem because includes
happens via LEFT OUTER JOIN, and we need to perform this JOIN anyway so as to filter out projects/groups based on the paths
we supply to this method.
However, the definition of this method is going to be changed in the upcoming MR: !137886 (merged), where will be using preload
instead of includes
to prevent N+1.
And, when we use preload
, it will fire an entirely separate query to load routes. So the question is, why make the app fire an extra query to preload routes, in areas where we are certain that preloading routes
are not needed at all?
This MR makes that change in the areas where we are certain that including routes in the result isn't necessary. These calls are now made with use_includes: false
, so that routes aren't included. And when we move to the new definition in !137886 (merged), an extra query won't be un-necessarily fired.
Screenshots or screen recordings
Screenshots are required for UI changes, and strongly recommended for all other merge requests.
Before | After |
---|---|
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
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.