Merge branch 'improve-mr-analytics-graphql-count-query' into 'master'

Improve the MR analytics list query

See merge request gitlab-org/gitlab!56380

app/finders/concerns/merged_at_filter.rb

@@ -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_before(merged_before) if merged_before.present?
 
-    items.join_metrics.merge(mr_metrics_scope)
+    join_metrics(items, mr_metrics_scope)
   end
 
   def merged_after
@@ -20,4 +20,22 @@ module MergedAtFilter
   def merged_before
     params[:merged_before]
   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

app/models/merge_request.rb

@@ -289,10 +289,19 @@ class MergeRequest < ApplicationRecord
     joins(:notes).where(notes: { commit_id: sha })
   end
   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 = 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
   scope :references_project, -> { references(:target_project) }
   scope :with_api_entity_associations, -> {

changelogs/unreleased/improve-mr-analytics-graphql-count-query.yml

+---
+title: Improve the performance of Merge Request Analytics table
+merge_request: 56380
+author:
+type: performance

spec/finders/merge_requests_finder_spec.rb

@@ -156,6 +156,18 @@ RSpec.describe MergeRequestsFinder do
 
           it { is_expected.to eq([merge_request2]) }
         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
 
       context 'filtering by group' do