Merge branch 'id-optimize-query-for-ci-pipelines' into 'master'

Optimize query for CI pipelines of merge request

See merge request gitlab-org/gitlab!19653

app/models/merge_request/pipelines.rb

@@ -12,15 +12,18 @@ class MergeRequest::Pipelines
 
   attr_reader :merge_request
 
-  delegate :all_commit_shas, :source_project, :source_branch, to: :merge_request
+  delegate :commit_shas, :source_project, :source_branch, to: :merge_request
 
   def all
-    return Ci::Pipeline.none unless source_project
-
     strong_memoize(:all_pipelines) do
-      pipelines = Ci::Pipeline.from_union(
-        [source_pipelines, detached_pipelines, triggered_for_branch],
-      remove_duplicates: false)
+      next Ci::Pipeline.none unless source_project
+
+      pipelines =
+        if merge_request.persisted?
+          pipelines_using_cte
+        else
+          triggered_for_branch.for_sha(commit_shas)
+        end
 
       sort(pipelines)
     end
@@ -28,38 +31,55 @@ class MergeRequest::Pipelines
 
   private
 
-  def triggered_by_merge_request
-    source_project.ci_pipelines
-      .where(source: :merge_request_event, merge_request: merge_request)
+  def pipelines_using_cte
+    cte = Gitlab::SQL::CTE.new(:shas, merge_request.all_commits.select(:sha))
+
+    source_pipelines_join = cte.table[:sha].eq(Ci::Pipeline.arel_table[:source_sha])
+    source_pipelines = filter_by(triggered_by_merge_request, cte, source_pipelines_join)
+    detached_pipelines = filter_by_sha(triggered_by_merge_request, cte)
+    pipelines_for_branch = filter_by_sha(triggered_for_branch, cte)
+
+    Ci::Pipeline.with(cte.to_arel)
+      .from_union([source_pipelines, detached_pipelines, pipelines_for_branch])
+  end
+
+  def filter_by_sha(pipelines, cte)
+    hex = Arel::Nodes::SqlLiteral.new("'hex'")
+    string_sha = Arel::Nodes::NamedFunction.new('encode', [cte.table[:sha], hex])
+    join_condition = string_sha.eq(Ci::Pipeline.arel_table[:sha])
+
+    filter_by(pipelines, cte, join_condition)
   end
 
-  def detached_pipelines
-    triggered_by_merge_request.for_sha(all_commit_shas)
+  def filter_by(pipelines, cte, join_condition)
+    shas_table =
+      Ci::Pipeline.arel_table
+        .join(cte.table, Arel::Nodes::InnerJoin)
+        .on(join_condition)
+        .join_sources
+
+    pipelines.joins(shas_table)
   end
 
-  def source_pipelines
-    triggered_by_merge_request.for_source_sha(all_commit_shas)
+  def triggered_by_merge_request
+    source_project.ci_pipelines
+      .where(source: :merge_request_event, merge_request: merge_request)
   end
 
   def triggered_for_branch
     source_project.ci_pipelines
       .where(source: branch_pipeline_sources, ref: source_branch, tag: false)
-      .for_sha(all_commit_shas)
-  end
-
-  def sources
-    ::Ci::Pipeline.sources
   end
 
   def branch_pipeline_sources
     strong_memoize(:branch_pipeline_sources) do
-      sources.reject { |source| source == EVENT }.values
+      Ci::Pipeline.sources.reject { |source| source == EVENT }.values
     end
   end
 
   def sort(pipelines)
     sql = 'CASE ci_pipelines.source WHEN (?) THEN 0 ELSE 1 END, ci_pipelines.id DESC'
-    query = ApplicationRecord.send(:sanitize_sql_array, [sql, sources[:merge_request_event]]) # rubocop:disable GitlabSecurity/PublicSend
+    query = ApplicationRecord.send(:sanitize_sql_array, [sql, Ci::Pipeline.sources[:merge_request_event]]) # rubocop:disable GitlabSecurity/PublicSend
 
     pipelines.order(Arel.sql(query))
   end

changelogs/unreleased/id-optimize-query-for-ci-pipelines.yml

+---
+title: Optimize query for CI pipelines of merge request
+merge_request: 19653
+author:
+type: performance

spec/factories/merge_request_diff_commits.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :merge_request_diff_commit do
+    association :merge_request_diff
+
+    sha { Digest::SHA1.hexdigest(SecureRandom.hex) }
+    relative_order { 0 }
+  end
+end

spec/models/merge_request/pipelines_spec.rb

@@ -75,7 +75,9 @@ describe MergeRequest::Pipelines do
       let(:shas) { project.repository.commits(source_ref, limit: 2).map(&:id) }
 
       before do
-        allow(merge_request).to receive(:all_commit_shas) { shas }
+        create(:merge_request_diff_commit,
+          merge_request_diff: merge_request.merge_request_diff,
+          sha: shas.second, relative_order: 1)
       end
 
       it 'returns merge request pipeline first' do
@@ -119,7 +121,11 @@ describe MergeRequest::Pipelines do
         end
 
         before do
-          allow(merge_request_2).to receive(:all_commit_shas) { shas }
+          shas.each.with_index do |sha, index|
+            create(:merge_request_diff_commit,
+              merge_request_diff: merge_request_2.merge_request_diff,
+              sha: sha, relative_order: index)
+          end
         end
 
         it 'returns only related merge request pipelines' do