Commit 5cc73aac authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'fix-issue-search-optimization-graphql' into 'master'

Fix issue search optimization in GraphQL

See merge request gitlab-org/gitlab!71351
parents 92ed02f9 e0dee494
...@@ -194,8 +194,7 @@ class IssuableFinder ...@@ -194,8 +194,7 @@ class IssuableFinder
def use_cte_for_search? def use_cte_for_search?
strong_memoize(:use_cte_for_search) do strong_memoize(:use_cte_for_search) do
next false unless search next false unless search
# Only simple unsorted & simple sorts can use CTE next false unless default_or_simple_sort?
next false if params[:sort].present? && !params[:sort].in?(klass.simple_sorts.keys)
attempt_group_search_optimizations? || attempt_project_search_optimizations? attempt_group_search_optimizations? || attempt_project_search_optimizations?
end end
...@@ -244,6 +243,10 @@ class IssuableFinder ...@@ -244,6 +243,10 @@ class IssuableFinder
klass.all klass.all
end end
def default_or_simple_sort?
params[:sort].blank? || params[:sort].to_s.in?(klass.simple_sorts.keys)
end
def attempt_group_search_optimizations? def attempt_group_search_optimizations?
params[:attempt_group_search_optimizations] params[:attempt_group_search_optimizations]
end end
......
...@@ -1199,6 +1199,14 @@ RSpec.describe IssuesFinder do ...@@ -1199,6 +1199,14 @@ RSpec.describe IssuesFinder do
end end
end end
context 'when a non-simple sort is given' do
let(:params) { { search: 'foo', attempt_project_search_optimizations: true, sort: 'popularity' } }
it 'returns false' do
expect(finder.use_cte_for_search?).to be_falsey
end
end
context 'when all conditions are met' do context 'when all conditions are met' do
context "uses group search optimization" do context "uses group search optimization" do
let(:params) { { search: 'foo', attempt_group_search_optimizations: true } } let(:params) { { search: 'foo', attempt_group_search_optimizations: true } }
...@@ -1217,6 +1225,24 @@ RSpec.describe IssuesFinder do ...@@ -1217,6 +1225,24 @@ RSpec.describe IssuesFinder do
expect(finder.execute.to_sql).to match(/^WITH "issues" AS #{Gitlab::Database::AsWithMaterialized.materialized_if_supported}/) expect(finder.execute.to_sql).to match(/^WITH "issues" AS #{Gitlab::Database::AsWithMaterialized.materialized_if_supported}/)
end end
end end
context 'with simple sort' do
let(:params) { { search: 'foo', attempt_project_search_optimizations: true, sort: 'updated_desc' } }
it 'returns true' do
expect(finder.use_cte_for_search?).to be_truthy
expect(finder.execute.to_sql).to match(/^WITH "issues" AS #{Gitlab::Database::AsWithMaterialized.materialized_if_supported}/)
end
end
context 'with simple sort as a symbol' do
let(:params) { { search: 'foo', attempt_project_search_optimizations: true, sort: :updated_desc } }
it 'returns true' do
expect(finder.use_cte_for_search?).to be_truthy
expect(finder.execute.to_sql).to match(/^WITH "issues" AS #{Gitlab::Database::AsWithMaterialized.materialized_if_supported}/)
end
end
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