Commit f06da1c4 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'move-group-issues-search-cte-up-the-chain' into 'master'

Add a flag to use a subquery for group issues search

See merge request gitlab-org/gitlab-ce!23445
parents 77849a2c a267aa56
......@@ -102,7 +102,7 @@ module IssuableCollections
elsif @group
options[:group_id] = @group.id
options[:include_subgroups] = true
options[:use_cte_for_search] = true
options[:attempt_group_search_optimizations] = true
end
params.permit(finder_type.valid_params).merge(options)
......
......@@ -27,12 +27,13 @@
# created_before: datetime
# updated_after: datetime
# updated_before: datetime
# use_cte_for_search: boolean
# attempt_group_search_optimizations: boolean
#
class IssuableFinder
prepend FinderWithCrossProjectAccess
include FinderMethods
include CreatedAtFilter
include Gitlab::Utils::StrongMemoize
requires_cross_project_access unless: -> { project? }
......@@ -75,8 +76,9 @@ class IssuableFinder
items = init_collection
items = filter_items(items)
# This has to be last as we may use a CTE as an optimization fence by
# passing the use_cte_for_search param
# This has to be last as we may use a CTE as an optimization fence
# by passing the attempt_group_search_optimizations param and
# enabling the use_cte_for_group_issues_search feature flag
# https://www.postgresql.org/docs/current/static/queries-with.html
items = by_search(items)
......@@ -85,6 +87,8 @@ class IssuableFinder
def filter_items(items)
items = by_project(items)
items = by_group(items)
items = by_subquery(items)
items = by_scope(items)
items = by_created_at(items)
items = by_updated_at(items)
......@@ -282,12 +286,31 @@ class IssuableFinder
end
# rubocop: enable CodeReuse/ActiveRecord
def use_subquery_for_search?
strong_memoize(:use_subquery_for_search) do
attempt_group_search_optimizations? &&
Feature.enabled?(:use_subquery_for_group_issues_search, default_enabled: false)
end
end
def use_cte_for_search?
strong_memoize(:use_cte_for_search) do
attempt_group_search_optimizations? &&
!use_subquery_for_search? &&
Feature.enabled?(:use_cte_for_group_issues_search, default_enabled: true)
end
end
private
def init_collection
klass.all
end
def attempt_group_search_optimizations?
search && Gitlab::Database.postgresql? && params[:attempt_group_search_optimizations]
end
def count_key(value)
Array(value).last.to_sym
end
......@@ -351,12 +374,13 @@ class IssuableFinder
end
# rubocop: enable CodeReuse/ActiveRecord
def use_cte_for_search?
return false unless search
return false unless Gitlab::Database.postgresql?
return false unless Feature.enabled?(:use_cte_for_group_issues_search, default_enabled: true)
params[:use_cte_for_search]
# Wrap projects and groups in a subquery if the conditions are met.
def by_subquery(items)
if use_subquery_for_search?
klass.where(id: items.select(:id)) # rubocop: disable CodeReuse/ActiveRecord
else
items
end
end
# rubocop: disable CodeReuse/ActiveRecord
......
---
title: Fix error when searching for group issues with priority or popularity sort
merge_request: 23445
author:
type: fixed
......@@ -226,9 +226,10 @@ describe GroupsController do
end
context 'searching' do
# Remove as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/52271
before do
# Remove in https://gitlab.com/gitlab-org/gitlab-ce/issues/54643
stub_feature_flags(use_cte_for_group_issues_search: false)
stub_feature_flags(use_subquery_for_group_issues_search: true)
end
it 'works with popularity sort' do
......
......@@ -640,4 +640,131 @@ describe IssuesFinder do
end
end
end
describe '#use_subquery_for_search?' do
let(:finder) { described_class.new(nil, params) }
before do
allow(Gitlab::Database).to receive(:postgresql?).and_return(true)
stub_feature_flags(use_subquery_for_group_issues_search: true)
end
context 'when there is no search param' do
let(:params) { { attempt_group_search_optimizations: true } }
it 'returns false' do
expect(finder.use_subquery_for_search?).to be_falsey
end
end
context 'when the database is not Postgres' do
let(:params) { { search: 'foo', attempt_group_search_optimizations: true } }
before do
allow(Gitlab::Database).to receive(:postgresql?).and_return(false)
end
it 'returns false' do
expect(finder.use_subquery_for_search?).to be_falsey
end
end
context 'when the attempt_group_search_optimizations param is falsey' do
let(:params) { { search: 'foo' } }
it 'returns false' do
expect(finder.use_subquery_for_search?).to be_falsey
end
end
context 'when the use_subquery_for_group_issues_search flag is disabled' do
let(:params) { { search: 'foo', attempt_group_search_optimizations: true } }
before do
stub_feature_flags(use_subquery_for_group_issues_search: false)
end
it 'returns false' do
expect(finder.use_subquery_for_search?).to be_falsey
end
end
context 'when all conditions are met' do
let(:params) { { search: 'foo', attempt_group_search_optimizations: true } }
it 'returns true' do
expect(finder.use_subquery_for_search?).to be_truthy
end
end
end
describe '#use_cte_for_search?' do
let(:finder) { described_class.new(nil, params) }
before do
allow(Gitlab::Database).to receive(:postgresql?).and_return(true)
stub_feature_flags(use_cte_for_group_issues_search: true)
stub_feature_flags(use_subquery_for_group_issues_search: false)
end
context 'when there is no search param' do
let(:params) { { attempt_group_search_optimizations: true } }
it 'returns false' do
expect(finder.use_cte_for_search?).to be_falsey
end
end
context 'when the database is not Postgres' do
let(:params) { { search: 'foo', attempt_group_search_optimizations: true } }
before do
allow(Gitlab::Database).to receive(:postgresql?).and_return(false)
end
it 'returns false' do
expect(finder.use_cte_for_search?).to be_falsey
end
end
context 'when the attempt_group_search_optimizations param is falsey' do
let(:params) { { search: 'foo' } }
it 'returns false' do
expect(finder.use_cte_for_search?).to be_falsey
end
end
context 'when the use_cte_for_group_issues_search flag is disabled' do
let(:params) { { search: 'foo', attempt_group_search_optimizations: true } }
before do
stub_feature_flags(use_cte_for_group_issues_search: false)
end
it 'returns false' do
expect(finder.use_cte_for_search?).to be_falsey
end
end
context 'when use_subquery_for_search? is true' do
let(:params) { { search: 'foo', attempt_group_search_optimizations: true } }
before do
stub_feature_flags(use_subquery_for_group_issues_search: true)
end
it 'returns false' do
expect(finder.use_cte_for_search?).to be_falsey
end
end
context 'when all conditions are met' do
let(:params) { { search: 'foo', attempt_group_search_optimizations: true } }
it 'returns true' do
expect(finder.use_cte_for_search?).to be_truthy
end
end
end
end
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