Commit dd6798a3 authored by Eugenia Grieff's avatar Eugenia Grieff

Use CTE search optimization for board issues

Set param attempt_group_search_optimizations or
attempt_project_search_optimizations as
true in Boards::Issues::ListService to force CTE

Include all Issue columns in group clauses

When ordering labels priority if CTE search
optimisation was used we need to include
all Issue columns in group clauses to
prevent a PG::GroupingError.

Add specs for board issues controller

Test index request using search param

Add relative_position as a simple sort

Check board parent before setting search params

If the board parent is a group we need to use
attempt_group_search_optimizations and if it's
a project attempt_project_search_optimizations

Refactor issue_grouping_columns method

Add feature flag board_search_optimization

Include a new feature flag to test performance
changes introduced by enabling CTE optimization
for boards search.
Because of this change we need to modify the
method order_labels_priority in Issuable concern
to prevent a PG::GroupingError when grouping result

Add more specs to board issues ListService
parent 13e1bb23
......@@ -20,6 +20,9 @@ module Boards
skip_before_action :authenticate_user!, only: [:index]
before_action :validate_id_list, only: [:bulk_move]
before_action :can_move_issues?, only: [:bulk_move]
before_action do
push_frontend_feature_flag(:board_search_optimization, board.group)
end
# rubocop: disable CodeReuse/ActiveRecord
def index
......
......@@ -249,7 +249,7 @@ module Issuable
Gitlab::Database.nulls_last_order('highest_priority', direction))
end
def order_labels_priority(direction = 'ASC', excluded_labels: [], extra_select_columns: [])
def order_labels_priority(direction = 'ASC', excluded_labels: [], extra_select_columns: [], with_cte: false)
params = {
target_type: name,
target_column: "#{table_name}.id",
......@@ -265,7 +265,7 @@ module Issuable
] + extra_select_columns
select(select_columns.join(', '))
.group(arel_table[:id])
.group(issue_grouping_columns(use_cte: with_cte))
.reorder(Gitlab::Database.nulls_last_order('highest_priority', direction))
end
......@@ -294,6 +294,18 @@ module Issuable
grouping_columns
end
# Includes all table keys in group by clause when sorting
# preventing errors in postgres when using CTE search optimisation
#
# Returns an array of arel columns
def issue_grouping_columns(use_cte: false)
if use_cte
[arel_table[:state]] + attribute_names.map { |attr| arel_table[attr.to_sym] }
else
arel_table[:id]
end
end
def to_ability_name
model_name.singular
end
......
......@@ -172,8 +172,10 @@ class Issue < ApplicationRecord
end
end
def self.order_by_position_and_priority
order_labels_priority
# `with_cte` argument allows sorting when using CTE queries and prevents
# errors in postgres when using CTE search optimisation
def self.order_by_position_and_priority(with_cte: false)
order_labels_priority(with_cte: with_cte)
.reorder(Gitlab::Database.nulls_last_order('relative_position', 'ASC'),
Gitlab::Database.nulls_last_order('highest_priority', 'ASC'),
"id DESC")
......
......@@ -10,7 +10,7 @@ module Boards
end
def execute
fetch_issues.order_by_position_and_priority
fetch_issues.order_by_position_and_priority(with_cte: can_attempt_search_optimization?)
end
# rubocop: disable CodeReuse/ActiveRecord
......@@ -63,6 +63,7 @@ module Boards
set_state
set_scope
set_non_archived
set_attempt_search_optimizations
params
end
......@@ -87,6 +88,16 @@ module Boards
params[:non_archived] = parent.is_a?(Group)
end
def set_attempt_search_optimizations
return unless can_attempt_search_optimization?
if board.group_board?
params[:attempt_group_search_optimizations] = true
else
params[:attempt_project_search_optimizations] = true
end
end
# rubocop: disable CodeReuse/ActiveRecord
def board_label_ids
@board_label_ids ||= board.lists.movable.pluck(:label_id)
......@@ -113,6 +124,15 @@ module Boards
.where("label_links.label_id = ?", list.label_id).limit(1))
end
# rubocop: enable CodeReuse/ActiveRecord
def board_group
board.group_board? ? parent : parent.group
end
def can_attempt_search_optimization?
params[:search].present? &&
Feature.enabled?(:board_search_optimization, board_group, default_enabled: false)
end
end
end
end
......
......@@ -49,6 +49,40 @@ describe Boards::IssuesController do
expect(json_response['issues'].length).to eq 2
expect(development.issues.map(&:relative_position)).not_to include(nil)
end
context 'with search param' do
context 'when board_search_optimization is enabled' do
before do
stub_feature_flags(board_search_optimization: true)
end
it 'returns matching issues using optimized search' do
create(:labeled_issue, project: project_1, labels: [planning], title: 'Test Issue')
create(:labeled_issue, project: project_1, labels: [planning], title: 'Sample Issue')
list_issues user: user, board: board, list: list1, search: 'Te'
expect(response).to match_response_schema('issues')
expect(json_response['issues'].length).to eq 1
end
end
context 'when board_search_optimization is disabled' do
before do
stub_feature_flags(board_search_optimization: false)
end
it 'returns empty result' do
create(:labeled_issue, project: project_1, labels: [planning], title: 'Test Issue')
create(:labeled_issue, project: project_1, labels: [planning], title: 'Sample Issue')
list_issues user: user, board: board, list: list1, search: 'Te'
expect(response).to match_response_schema('issues')
expect(json_response['issues'].length).to eq 0
end
end
end
end
context 'with invalid list id' do
......@@ -73,6 +107,40 @@ describe Boards::IssuesController do
expect(response).to match_response_schema('issues')
expect(json_response['issues'].length).to eq 2
end
context 'with search param' do
context 'when board_search_optimization is enabled' do
before do
stub_feature_flags(board_search_optimization: true)
end
it 'returns matching issues using optimized search' do
create(:issue, project: project_1, title: 'Issue XI')
create(:issue, project: project_1, title: 'Issue XX')
list_issues user: user, board: board, search: 'XI'
expect(response).to match_response_schema('issues')
expect(json_response['issues'].length).to eq 1
end
end
context 'when board_search_optimization is disabled' do
before do
stub_feature_flags(board_search_optimization: false)
end
it 'returns empty result' do
create(:issue, project: project_1, title: 'Issue XI')
create(:issue, project: project_1, title: 'Issue XX')
list_issues user: user, board: board, search: 'XI'
expect(response).to match_response_schema('issues')
expect(json_response['issues'].length).to eq 0
end
end
end
end
context 'with unauthorized user' do
......@@ -87,12 +155,13 @@ describe Boards::IssuesController do
end
end
def list_issues(user:, board:, list: nil)
def list_issues(user:, board:, list: nil, search: nil)
sign_in(user)
params = {
board_id: board.to_param,
list_id: list.try(:to_param)
list_id: list.try(:to_param),
search: search.try(:to_param)
}
get :index, params: params.compact
......
......@@ -151,5 +151,46 @@ describe Boards::Issues::ListService, services: true do
end
end
end
context 'when search param is present' do
shared_examples 'returns correct result using 3 characters' do
it 'returns correct issues' do
params = { board_id: board.id, search: 'Iss' }
issues = described_class.new(parent, user, params).execute
expect(issues).to contain_exactly(opened_issue1, opened_issue2, reopened_issue1)
end
end
context 'when board_search_optimization feature is enabled' do
before do
stub_feature_flags(board_search_optimization: true)
end
it_behaves_like 'returns correct result using 3 characters'
it 'returns correct issues using 2 characters' do
params = { board_id: board.id, search: 'Is' }
issues = described_class.new(parent, user, params).execute
expect(issues).to contain_exactly(opened_issue1, opened_issue2, reopened_issue1)
end
end
context 'when board_search_optimization feature is disabled' do
before do
stub_feature_flags(board_search_optimization: false)
end
it_behaves_like 'returns correct result using 3 characters'
it 'returns empty result using 2 characters' do
params = { board_id: board.id, search: 'Is' }
issues = described_class.new(parent, user, params).execute
expect(issues).to be_empty
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