Merge branch...

Merge branch 'rollout-decomposed_ci_query_in_pipelines_for_merge_request_finder-feature-flag' into 'master'

Avoid cross-joins in PipelinesForMergeRequestFinder

See merge request gitlab-org/gitlab!74293

app/finders/ci/pipelines_for_merge_request_finder.rb

@@ -31,7 +31,6 @@ module Ci
 
     # Fetch all pipelines without permission check.
     def all
-      ::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336891') do
       strong_memoize(:all_pipelines) do
         next Ci::Pipeline.none unless source_project
 
@@ -42,56 +41,17 @@ module Ci
             triggered_for_branch.for_sha(commit_shas)
           end
 
-          pipelines = pipelines.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336891')
-
         sort(pipelines)
       end
     end
-    end
 
     private
 
-    # rubocop: disable CodeReuse/ActiveRecord
-    def pipelines_using_cte
-      sha_relation = merge_request.all_commits.select(:sha).distinct
-
-      cte = Gitlab::SQL::CTE.new(:shas, sha_relation)
-
-      pipelines_for_merge_requests = triggered_by_merge_request
-      pipelines_for_branch = filter_by_sha(triggered_for_branch, cte)
-
-      Ci::Pipeline.with(cte.to_arel) # rubocop: disable CodeReuse/ActiveRecord
-        .from_union([pipelines_for_merge_requests, pipelines_for_branch])
-    end
-    # rubocop: enable CodeReuse/ActiveRecord
-
-    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 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) # rubocop: disable CodeReuse/ActiveRecord
-    end
-
     def all_pipelines_for_merge_request
-      if Feature.enabled?(:decomposed_ci_query_in_pipelines_for_merge_request_finder, target_project, default_enabled: :yaml)
       pipelines_for_merge_request = triggered_by_merge_request
       pipelines_for_branch = triggered_for_branch.for_sha(recent_diff_head_shas(COMMITS_LIMIT))
 
       Ci::Pipeline.from_union([pipelines_for_merge_request, pipelines_for_branch])
-      else
-        pipelines_using_cte
-      end
     end
 
     # NOTE: this method returns only parent merge request pipelines.

config/feature_flags/development/decomposed_ci_query_in_pipelines_for_merge_request_finder.yml

----
-name: decomposed_ci_query_in_pipelines_for_merge_request_finder
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68549
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/341341
-milestone: '14.4'
-type: development
-group: group::pipeline execution
-default_enabled: false

spec/finders/ci/pipelines_for_merge_request_finder_spec.rb

@@ -135,9 +135,33 @@ RSpec.describe Ci::PipelinesForMergeRequestFinder do
     end
 
     context 'when pipelines exist for the branch and merge request' do
-      shared_examples 'returns all pipelines for merge request' do
+      let(:source_ref) { 'feature' }
+      let(:target_ref) { 'master' }
+
+      let!(:branch_pipeline) do
+        create(:ci_pipeline, source: :push, project: project,
+               ref: source_ref, sha: merge_request.merge_request_diff.head_commit_sha)
+      end
+
+      let!(:tag_pipeline) do
+        create(:ci_pipeline, project: project, ref: source_ref, tag: true)
+      end
+
+      let!(:detached_merge_request_pipeline) do
+        create(:ci_pipeline, source: :merge_request_event, project: project,
+               ref: source_ref, sha: shas.second, merge_request: merge_request)
+      end
+
+      let(:merge_request) do
+        create(:merge_request, source_project: project, source_branch: source_ref,
+               target_project: project, target_branch: target_ref)
+      end
+
+      let(:project) { create(:project, :repository) }
+      let(:shas) { project.repository.commits(source_ref, limit: 2).map(&:id) }
+
       it 'returns merge request pipeline first' do
-          expect(subject.all).to eq([detached_merge_request_pipeline, branch_pipeline])
+        expect(subject.all).to match_array([detached_merge_request_pipeline, branch_pipeline])
       end
 
       context 'when there are a branch pipeline and a merge request pipeline' do
@@ -153,10 +177,7 @@ RSpec.describe Ci::PipelinesForMergeRequestFinder do
 
         it 'returns merge request pipelines first' do
           expect(subject.all)
-              .to eq([detached_merge_request_pipeline_2,
-                      detached_merge_request_pipeline,
-                      branch_pipeline_2,
-                      branch_pipeline])
+            .to match_array([detached_merge_request_pipeline_2, detached_merge_request_pipeline, branch_pipeline_2, branch_pipeline])
         end
       end
 
@@ -189,10 +210,7 @@ RSpec.describe Ci::PipelinesForMergeRequestFinder do
         end
 
         it 'returns only related merge request pipelines' do
-            expect(subject.all)
-              .to eq([detached_merge_request_pipeline,
-                      branch_pipeline_2,
-                      branch_pipeline])
+          expect(subject.all).to match_array([detached_merge_request_pipeline, branch_pipeline_2, branch_pipeline])
 
           expect(described_class.new(merge_request_2, nil).all)
             .to match_array([detached_merge_request_pipeline_2, branch_pipeline_2, branch_pipeline])
@@ -214,47 +232,5 @@ RSpec.describe Ci::PipelinesForMergeRequestFinder do
         end
       end
     end
-
-      let(:source_ref) { 'feature' }
-      let(:target_ref) { 'master' }
-
-      let!(:branch_pipeline) do
-        create(:ci_pipeline, source: :push, project: project,
-               ref: source_ref, sha: merge_request.merge_request_diff.head_commit_sha)
-      end
-
-      let!(:tag_pipeline) do
-        create(:ci_pipeline, project: project, ref: source_ref, tag: true)
-      end
-
-      let!(:detached_merge_request_pipeline) do
-        create(:ci_pipeline, source: :merge_request_event, project: project,
-               ref: source_ref, sha: shas.second, merge_request: merge_request)
-      end
-
-      let(:merge_request) do
-        create(:merge_request, source_project: project, source_branch: source_ref,
-               target_project: project, target_branch: target_ref)
-      end
-
-      let(:project) { create(:project, :repository) }
-      let(:shas) { project.repository.commits(source_ref, limit: 2).map(&:id) }
-
-      context 'when `decomposed_ci_query_in_pipelines_for_merge_request_finder` feature flag enabled' do
-        before do
-          stub_feature_flags(decomposed_ci_query_in_pipelines_for_merge_request_finder: merge_request.target_project)
-        end
-
-        it_behaves_like 'returns all pipelines for merge request'
-      end
-
-      context 'when `decomposed_ci_query_in_pipelines_for_merge_request_finder` feature flag disabled' do
-        before do
-          stub_feature_flags(decomposed_ci_query_in_pipelines_for_merge_request_finder: false)
-        end
-
-        it_behaves_like 'returns all pipelines for merge request'
-      end
-    end
   end
 end