Merge branch 'ap-remove-dups-in-query' into 'master'

Query descendants through superset

See merge request !87643

app/models/namespaces/traversal/linear_scopes.rb

@@ -127,24 +127,41 @@ def use_traversal_ids_for_self_and_hierarchy_scopes?
 
         def self_and_descendants_with_comparison_operators(include_self: true)
           base = all.select(:traversal_ids)
+          base = base.select(:id) if Feature.enabled?(:linear_scopes_superset)
           base_cte = Gitlab::SQL::CTE.new(:descendants_base_cte, base)
 
           namespaces = Arel::Table.new(:namespaces)
 
+          withs = [base_cte.to_arel]
+          froms = []
+
+          if Feature.enabled?(:linear_scopes_superset)
+            superset_cte = self.superset_cte(base_cte.table.name)
+            withs += [superset_cte.to_arel]
+            froms = [superset_cte.table]
+          else
+            froms = [base_cte.table]
+          end
+
+          # Order is important. namespace should be last to handle future joins.
+          froms += [namespaces]
+
+          base_ref = froms.first
+
           # Bound the search space to ourselves (optional) and descendants.
           #
           # WHERE next_traversal_ids_sibling(base_cte.traversal_ids) > namespaces.traversal_ids
           records = unscoped
             .distinct
-            .with(base_cte.to_arel)
-            .from([base_cte.table, namespaces])
-            .where(next_sibling_func(base_cte.table[:traversal_ids]).gt(namespaces[:traversal_ids]))
+            .with(*withs)
+            .from(froms)
+            .where(next_sibling_func(base_ref[:traversal_ids]).gt(namespaces[:traversal_ids]))
 
           #   AND base_cte.traversal_ids <= namespaces.traversal_ids
           if include_self
-            records.where(base_cte.table[:traversal_ids].lteq(namespaces[:traversal_ids]))
+            records.where(base_ref[:traversal_ids].lteq(namespaces[:traversal_ids]))
           else
-            records.where(base_cte.table[:traversal_ids].lt(namespaces[:traversal_ids]))
+            records.where(base_ref[:traversal_ids].lt(namespaces[:traversal_ids]))
           end
         end
 
@@ -166,6 +183,23 @@ def self_and_descendants_with_duplicates_with_array_operator(include_self: true)
           end
         end
 
+        def superset_cte(base_name)
+          superset_sql = <<~SQL
+            SELECT d1.traversal_ids
+            FROM #{base_name} d1
+            LEFT JOIN LATERAL (
+              SELECT d2.id as ancestor_id
+              FROM #{base_name} d2
+              WHERE d2.id = ANY(d1.traversal_ids)
+                AND d2.id <> d1.id
+              LIMIT 1
+            ) covered ON TRUE
+            WHERE covered.ancestor_id IS NULL
+          SQL
+
+          Gitlab::SQL::CTE.new(:superset, superset_sql, materialized: false)
+        end
+
         def ancestor_ctes
           base_scope = all.select('namespaces.id', 'namespaces.traversal_ids')
           base_cte = Gitlab::SQL::CTE.new(:base_ancestors_cte, base_scope)

config/feature_flags/development/linear_scopes_superset.yml

+---
+name: linear_scopes_superset
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/87643
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/362687
+milestone: '15.1'
+type: development
+group: group::workspace
+default_enabled: false

lib/gitlab/sql/cte.rb

@@ -33,7 +33,7 @@ def initialize(name, query, materialized: true)
 
       # Returns the Arel relation for this CTE.
       def to_arel
-        sql = Arel::Nodes::SqlLiteral.new("(#{query.to_sql})")
+        sql = Arel::Nodes::SqlLiteral.new("(#{query_as_sql})")
 
         Gitlab::Database::AsWithMaterialized.new(table, sql, materialized: @materialized)
       end
@@ -54,6 +54,12 @@ def apply_to(relation)
           .with(to_arel)
           .from(alias_to(relation.model.arel_table))
       end
+
+      private
+
+      def query_as_sql
+        query.is_a?(String) ? query : query.to_sql
+      end
     end
   end
 end

spec/lib/gitlab/sql/cte_spec.rb

@@ -3,15 +3,14 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::SQL::CTE do
-  describe '#to_arel' do
+  shared_examples '#to_arel' do
     it 'generates an Arel relation for the CTE body' do
-      relation = User.where(id: 1)
       cte = described_class.new(:cte_name, relation)
       sql = cte.to_arel.to_sql
       name = ApplicationRecord.connection.quote_table_name(:cte_name)
 
       sql1 = ApplicationRecord.connection.unprepared_statement do
-        relation.except(:order).to_sql
+        relation.is_a?(String) ? relation : relation.to_sql
       end
 
       expected = [
@@ -25,6 +24,20 @@
     end
   end
 
+  describe '#to_arel' do
+    context 'when relation is an ActiveRecord::Relation' do
+      let(:relation) { User.where(id: 1) }
+
+      include_examples '#to_arel'
+    end
+
+    context 'when relation is a String' do
+      let(:relation) { User.where(id: 1).to_sql }
+
+      include_examples '#to_arel'
+    end
+  end
+
   describe '#alias_to' do
     it 'returns an alias for the CTE' do
       cte = described_class.new(:cte_name, nil)

spec/support/shared_examples/namespaces/traversal_scope_examples.rb

@@ -238,6 +238,12 @@
       subject { described_class.where(id: [nested_group_1, nested_group_2]).self_and_descendants(include_self: false) }
 
       it { is_expected.to contain_exactly(deep_nested_group_1, deep_nested_group_2) }
+
+      context 'with duplicate descendants' do
+        subject { described_class.where(id: [group_1, nested_group_1]).self_and_descendants(include_self: false) }
+
+        it { is_expected.to contain_exactly(nested_group_1, deep_nested_group_1) }
+      end
     end
 
     context 'with offset and limit' do
@@ -267,6 +273,14 @@
 
       include_examples '.self_and_descendants'
     end
+
+    context 'with linear_scopes_superset feature flag disabled' do
+      before do
+        stub_feature_flags(linear_scopes_superset: false)
+      end
+
+      include_examples '.self_and_descendants'
+    end
   end
 
   shared_examples '.self_and_descendant_ids' do
@@ -310,6 +324,14 @@
 
       include_examples '.self_and_descendant_ids'
     end
+
+    context 'with linear_scopes_superset feature flag disabled' do
+      before do
+        stub_feature_flags(linear_scopes_superset: false)
+      end
+
+      include_examples '.self_and_descendant_ids'
+    end
   end
 
   shared_examples '.self_and_hierarchy' do