Merge branch 'ah-global-use-distinct-object-hierarchy' into 'master'

Enable DISTINCT optimization for ObjectHierarchy globally [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!57052

changelogs/unreleased/sh-global-use-distinct-object-hierarchy.yml

+---
+title: Enable DISTINCT optimization for ObjectHierarchy globally
+merge_request: 57052
+author:
+type: changed

config/feature_flags/development/use_distinct_for_all_object_hierarchy.yml

+---
+name: use_distinct_for_all_object_hierarchy
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/57052
+rollout_issue_url: 
+milestone: '13.11'
+type: development
+group: group::database
+default_enabled: false

lib/gitlab/object_hierarchy.rb

@@ -68,12 +68,14 @@ module Gitlab
         expose_depth = hierarchy_order.present?
         hierarchy_order ||= :asc
 
-        recursive_query = base_and_ancestors_cte(upto, hierarchy_order).apply_to(model.all).distinct
-
         # if hierarchy_order is given, the calculated `depth` should be present in SELECT
         if expose_depth
+          recursive_query = base_and_ancestors_cte(upto, hierarchy_order).apply_to(model.all).distinct
           read_only(model.from(Arel::Nodes::As.new(recursive_query.arel, objects_table)).order(depth: hierarchy_order))
         else
+          recursive_query = base_and_ancestors_cte(upto).apply_to(model.all)
+          recursive_query = recursive_query.reselect(*recursive_query.arel.projections, 'ROW_NUMBER() OVER () as depth').distinct
+          recursive_query = model.from(Arel::Nodes::As.new(recursive_query.arel, objects_table))
           read_only(remove_depth_and_maintain_order(recursive_query, hierarchy_order: hierarchy_order))
         end
       else
@@ -93,11 +95,13 @@ module Gitlab
     def base_and_descendants(with_depth: false)
       if use_distinct?
         # Always calculate `depth`, remove it later if with_depth is false
-        base_cte = base_and_descendants_cte(with_depth: true).apply_to(model.all).distinct
-
         if with_depth
-          read_only(model.from(Arel::Nodes::As.new(recursive_query.arel, objects_table)).order(depth: :asc))
+          base_cte = base_and_descendants_cte(with_depth: true).apply_to(model.all).distinct
+          read_only(model.from(Arel::Nodes::As.new(base_cte.arel, objects_table)).order(depth: :asc))
         else
+          base_cte = base_and_descendants_cte.apply_to(model.all)
+          base_cte = base_cte.reselect(*base_cte.arel.projections, 'ROW_NUMBER() OVER () as depth').distinct
+          base_cte = model.from(Arel::Nodes::As.new(base_cte.arel, objects_table))
           read_only(remove_depth_and_maintain_order(base_cte, hierarchy_order: :asc))
         end
       else
@@ -161,7 +165,12 @@ module Gitlab
 
     # Use distinct on the Namespace queries to avoid bad planner behavior in PG11.
     def use_distinct?
-      (model <= Namespace) && options[:use_distinct]
+      return unless model <= Namespace
+      # Global use_distinct_for_all_object_hierarchy takes precedence over use_distinct_in_object_hierarchy
+      return true if Feature.enabled?(:use_distinct_for_all_object_hierarchy)
+      return options[:use_distinct] if options.key?(:use_distinct)
+
+      false
     end
 
     # Remove the extra `depth` field using an INNER JOIN to avoid breaking UNION queries

spec/lib/gitlab/object_hierarchy_spec.rb

@@ -187,6 +187,21 @@ RSpec.describe Gitlab::ObjectHierarchy do
   context 'when the use_distinct_in_object_hierarchy feature flag is enabled' do
     before do
       stub_feature_flags(use_distinct_in_object_hierarchy: true)
+      stub_feature_flags(use_distinct_for_all_object_hierarchy: false)
+    end
+
+    it_behaves_like 'Gitlab::ObjectHierarchy test cases'
+
+    it 'calls DISTINCT' do
+      expect(parent.self_and_descendants.to_sql).to include("DISTINCT")
+      expect(child2.self_and_ancestors.to_sql).to include("DISTINCT")
+    end
+  end
+
+  context 'when the use_distinct_for_all_object_hierarchy feature flag is enabled' do
+    before do
+      stub_feature_flags(use_distinct_in_object_hierarchy: false)
+      stub_feature_flags(use_distinct_for_all_object_hierarchy: true)
     end
 
     it_behaves_like 'Gitlab::ObjectHierarchy test cases'
@@ -200,6 +215,7 @@ RSpec.describe Gitlab::ObjectHierarchy do
   context 'when the use_distinct_in_object_hierarchy feature flag is disabled' do
     before do
       stub_feature_flags(use_distinct_in_object_hierarchy: false)
+      stub_feature_flags(use_distinct_for_all_object_hierarchy: false)
     end
 
     it_behaves_like 'Gitlab::ObjectHierarchy test cases'

spec/services/ci/register_job_service_spec.rb

@@ -228,6 +228,7 @@ module Ci
           context 'when the use_distinct_in_register_job_object_hierarchy feature flag is enabled' do
             before do
               stub_feature_flags(use_distinct_in_register_job_object_hierarchy: true)
+              stub_feature_flags(use_distinct_for_all_object_hierarchy: true)
             end
 
             it 'calls DISTINCT' do
@@ -238,6 +239,7 @@ module Ci
           context 'when the use_distinct_in_register_job_object_hierarchy feature flag is disabled' do
             before do
               stub_feature_flags(use_distinct_in_register_job_object_hierarchy: false)
+              stub_feature_flags(use_distinct_for_all_object_hierarchy: false)
             end
 
             it 'does not call DISTINCT' do