Commit d2f1d585 authored by Jan Provaznik's avatar Jan Provaznik

Skip projects filter on merge requests search

When searching for merge requests, an additional subquery
is added which by default filters only merge requests which belong
to source or target project user has permission for.

This filter is not needed because more restrictive filter
which checks if user has permission for target project
is used in the query.

So unless a custom projects filter is used by user, it's possible
to skip the default projects filter and speed up the final query.

Related to #40540
parent 889c7081
module Search module Search
class GlobalService class GlobalService
attr_accessor :current_user, :params attr_accessor :current_user, :params
attr_reader :default_project_filter
def initialize(user, params) def initialize(user, params)
@current_user, @params = user, params.dup @current_user, @params = user, params.dup
@default_project_filter = true
end end
def execute def execute
Gitlab::SearchResults.new(current_user, projects, params[:search]) Gitlab::SearchResults.new(current_user, projects, params[:search],
default_project_filter: default_project_filter)
end end
def projects def projects
......
...@@ -5,6 +5,7 @@ module Search ...@@ -5,6 +5,7 @@ module Search
def initialize(user, group, params) def initialize(user, group, params)
super(user, params) super(user, params)
@default_project_filter = false
@group = group @group = group
end end
......
---
title: Improve search query for merge requests.
merge_request:
author:
type: performance
...@@ -27,10 +27,17 @@ module Gitlab ...@@ -27,10 +27,17 @@ module Gitlab
# It allows us to search only for projects user has access to # It allows us to search only for projects user has access to
attr_reader :limit_projects attr_reader :limit_projects
def initialize(current_user, limit_projects, query) # Whether a custom filter is used to restrict scope of projects.
# If the default filter (which lists all projects user has access to)
# is used, we can skip it when filtering merge requests and optimize the
# query
attr_reader :default_project_filter
def initialize(current_user, limit_projects, query, default_project_filter: false)
@current_user = current_user @current_user = current_user
@limit_projects = limit_projects || Project.all @limit_projects = limit_projects || Project.all
@query = query @query = query
@default_project_filter = default_project_filter
end end
def objects(scope, page = nil) def objects(scope, page = nil)
...@@ -94,7 +101,11 @@ module Gitlab ...@@ -94,7 +101,11 @@ module Gitlab
end end
def merge_requests def merge_requests
merge_requests = MergeRequestsFinder.new(current_user).execute.in_projects(project_ids_relation) merge_requests = MergeRequestsFinder.new(current_user).execute
unless default_project_filter
merge_requests = merge_requests.in_projects(project_ids_relation)
end
merge_requests = merge_requests =
if query =~ /[#!](\d+)\z/ if query =~ /[#!](\d+)\z/
merge_requests.where(iid: $1) merge_requests.where(iid: $1)
......
...@@ -51,6 +51,17 @@ describe Gitlab::SearchResults do ...@@ -51,6 +51,17 @@ describe Gitlab::SearchResults do
expect(results.objects('merge_requests')).to include merge_request_2 expect(results.objects('merge_requests')).to include merge_request_2
end end
it 'includes project filter by default' do
expect(results).to receive(:project_ids_relation).and_call_original
results.objects('merge_requests')
end
it 'it skips project filter if default is used' do
allow(results).to receive(:default_project_filter).and_return(true)
expect(results).not_to receive(:project_ids_relation)
results.objects('merge_requests')
end
end end
it 'does not list issues on private projects' do it 'does not list issues on private projects' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment