Commit d8042c4c authored by Adam Hegyi's avatar Adam Hegyi

Improve the MR analytics list query

This improves the MR analytics queries by moving the target_project_id
filter to the `merge_request_metrics` table.
parent 7a19b738
...@@ -10,7 +10,7 @@ module MergedAtFilter ...@@ -10,7 +10,7 @@ module MergedAtFilter
mr_metrics_scope = mr_metrics_scope.merged_after(merged_after) if merged_after.present? mr_metrics_scope = mr_metrics_scope.merged_after(merged_after) if merged_after.present?
mr_metrics_scope = mr_metrics_scope.merged_before(merged_before) if merged_before.present? mr_metrics_scope = mr_metrics_scope.merged_before(merged_before) if merged_before.present?
items.join_metrics.merge(mr_metrics_scope) join_metrics(items, mr_metrics_scope)
end end
def merged_after def merged_after
...@@ -20,4 +20,22 @@ module MergedAtFilter ...@@ -20,4 +20,22 @@ module MergedAtFilter
def merged_before def merged_before
params[:merged_before] params[:merged_before]
end end
# rubocop: disable CodeReuse/ActiveRecord
#
# This join optimizes merged_at queries when the finder is invoked for a project by moving
# the target_project_id condition from merge_requests table to merge_request_metrics table.
def join_metrics(items, mr_metrics_scope)
scope = if project_id = items.where_values_hash["target_project_id"]
# removing the original merge_requests.target_project_id condition
items = items.unscope(where: :target_project_id)
# adding the target_project_id condition to merge_request_metrics
items.join_metrics(project_id)
else
items.join_metrics
end
scope.merge(mr_metrics_scope)
end
# rubocop: enable CodeReuse/ActiveRecord
end end
...@@ -289,10 +289,19 @@ class MergeRequest < ApplicationRecord ...@@ -289,10 +289,19 @@ class MergeRequest < ApplicationRecord
joins(:notes).where(notes: { commit_id: sha }) joins(:notes).where(notes: { commit_id: sha })
end end
scope :join_project, -> { joins(:target_project) } scope :join_project, -> { joins(:target_project) }
scope :join_metrics, -> do scope :join_metrics, -> (target_project_id = nil) do
# Do not join the relation twice
return self if self.arel.join_sources.any? { |join| join.left.try(:name).eql?(MergeRequest::Metrics.table_name) }
query = joins(:metrics) query = joins(:metrics)
query = query.where(MergeRequest.arel_table[:target_project_id].eq(MergeRequest::Metrics.arel_table[:target_project_id]))
query project_condition = if target_project_id
MergeRequest::Metrics.arel_table[:target_project_id].eq(target_project_id)
else
MergeRequest.arel_table[:target_project_id].eq(MergeRequest::Metrics.arel_table[:target_project_id])
end
query.where(project_condition)
end end
scope :references_project, -> { references(:target_project) } scope :references_project, -> { references(:target_project) }
scope :with_api_entity_associations, -> { scope :with_api_entity_associations, -> {
......
---
title: Improve the performance of Merge Request Analytics table
merge_request: 56380
author:
type: performance
...@@ -156,6 +156,18 @@ RSpec.describe MergeRequestsFinder do ...@@ -156,6 +156,18 @@ RSpec.describe MergeRequestsFinder do
it { is_expected.to eq([merge_request2]) } it { is_expected.to eq([merge_request2]) }
end end
context 'when project_id is given' do
subject(:query) { described_class.new(user, merged_after: 15.days.ago, merged_before: 6.days.ago, project_id: merge_request2.project).execute }
it { is_expected.to eq([merge_request2]) }
it 'queries merge_request_metrics.target_project_id table' do
expect(query.to_sql).to include(%{"merge_request_metrics"."target_project_id" = #{merge_request2.target_project_id}})
expect(query.to_sql).not_to include(%{"merge_requests"."target_project_id"})
end
end
end end
context 'filtering by group' do context 'filtering by group' 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