Commit e0dee494 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Fix issue search optimization in GraphQL

This enables the CTE search optimization when searching issuables via
GraphQL.

When going through GraphQL resolvers, a symbol is passed in as the sort
param of the IssuesFinder. This fixes our search optimization which
depends on the given sort order to take symbols into account.

Changelog: fixed
parent cb95c2f7
...@@ -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