Merge branch '47988-improve-milestone-queries-with-subq' into 'master'

Improve MilestonesFinder to accept project and group relations

Closes #47988

See merge request gitlab-org/gitlab-ce!24325

Author: Andreas Brandl

app/controllers/projects/milestones_controller.rb

@@ -144,7 +144,7 @@ def milestone_params
 
   def search_params
     if request.format.json? && project_group && can?(current_user, :read_group, project_group)
-      groups = project_group.self_and_ancestors_ids
+      groups = project_group.self_and_ancestors.select(:id)
     end
 
     params.permit(:state).merge(project_ids: @project.id, group_ids: groups)

app/finders/milestones_finder.rb

@@ -3,26 +3,22 @@
 # Search for milestones
 #
 # params - Hash
-#   project_ids: Array of project ids or single project id.
-#   group_ids: Array of group ids or single group id.
+#   project_ids: Array of project ids or single project id or ActiveRecord relation.
+#   group_ids: Array of group ids or single group id or ActiveRecord relation.
 #   order - Orders by field default due date asc.
 #   title - filter by title.
 #   state - filters by state.
 
 class MilestonesFinder
   include FinderMethods
 
-  attr_reader :params, :project_ids, :group_ids
+  attr_reader :params
 
   def initialize(params = {})
-    @project_ids = Array(params[:project_ids])
-    @group_ids = Array(params[:group_ids])
     @params = params
   end
 
   def execute
-    return Milestone.none if project_ids.empty? && group_ids.empty?
-
     items = Milestone.all
     items = by_groups_and_projects(items)
     items = by_title(items)
@@ -34,7 +30,7 @@ def execute
   private
 
   def by_groups_and_projects(items)
-    items.for_projects_and_groups(project_ids, group_ids)
+    items.for_projects_and_groups(params[:project_ids], params[:group_ids])
   end
 
   # rubocop: disable CodeReuse/ActiveRecord

app/models/milestone.rb

@@ -45,7 +45,7 @@ class Milestone < ActiveRecord::Base
     groups = groups.compact if groups.is_a? Array
     groups = [] if groups.nil?
 
-    where(project: projects).or(where(group: groups))
+    where(project_id: projects).or(where(group_id: groups))
   end
 
   scope :order_by_name_asc, -> { order(Arel::Nodes::Ascending.new(arel_table[:title].lower)) }
@@ -191,7 +191,7 @@ def self.states_count(projects, groups = nil)
     return STATE_COUNT_HASH unless projects || groups
 
     counts = Milestone
-               .for_projects_and_groups(projects&.map(&:id), groups&.map(&:id))
+               .for_projects_and_groups(projects, groups)
                .reorder(nil)
                .group(:state)
                .count
@@ -275,8 +275,7 @@ def uniqueness_of_title
     if project
       relation = Milestone.for_projects_and_groups([project_id], [project.group&.id])
     elsif group
-      project_ids = group.projects.map(&:id)
-      relation = Milestone.for_projects_and_groups(project_ids, [group.id])
+      relation = Milestone.for_projects_and_groups(group.projects.select(:id), [group.id])
     end
 
     title_exists = relation.find_by_title(title)

app/services/issuable_base_service.rb

@@ -61,10 +61,10 @@ def filter_milestone
     return unless milestone_id
 
     params[:milestone_id] = '' if milestone_id == IssuableFinder::NONE
-    group_ids = project.group&.self_and_ancestors&.pluck(:id)
+    groups = project.group&.self_and_ancestors&.select(:id)
 
     milestone =
-      Milestone.for_projects_and_groups([project.id], group_ids).find_by_id(milestone_id)
+      Milestone.for_projects_and_groups([project.id], groups).find_by_id(milestone_id)
 
     params[:milestone_id] = '' unless milestone
   end

app/services/milestones/promote_service.rb

@@ -82,11 +82,9 @@ def destroy_old_milestones(milestone)
     end
     # rubocop: enable CodeReuse/ActiveRecord
 
-    # rubocop: disable CodeReuse/ActiveRecord
     def group_project_ids
-      @group_project_ids ||= group.projects.pluck(:id)
+      group.projects.select(:id)
     end
-    # rubocop: enable CodeReuse/ActiveRecord
 
     def raise_error(message)
       raise PromoteMilestoneError, "Promotion failed - #{message}"

app/services/projects/autocomplete_service.rb

@@ -14,7 +14,7 @@ def milestones
         order: { due_date: :asc, title: :asc }
       }
 
-      finder_params[:group_ids] = @project.group.self_and_ancestors_ids if @project.group
+      finder_params[:group_ids] = @project.group.self_and_ancestors.select(:id) if @project.group
 
       MilestonesFinder.new(finder_params).execute.select([:iid, :title])
     end

changelogs/unreleased/47988-improve-milestone-queries-with-subqueries.yml

@@ -0,0 +1,5 @@
+---
+title: Improve milestone queries using subqueries instead of separate queries for ids
+merge_request: 24325
+author:
+type: performance

lib/banzai/filter/milestone_reference_filter.rb

@@ -101,9 +101,9 @@ def milestone_finder_params(parent, find_by_iid)
 
       def self_and_ancestors_ids(parent)
         if group_context?(parent)
-          parent.self_and_ancestors_ids
+          parent.self_and_ancestors.select(:id)
         elsif project_context?(parent)
-          parent.group&.self_and_ancestors_ids
+          parent.group&.self_and_ancestors&.select(:id)
         end
       end
 

spec/models/milestone_spec.rb

@@ -286,8 +286,8 @@
     end
 
     context 'relations as params' do
-      let(:projects) { Project.where(id: project.id) }
-      let(:groups) { Group.where(id: group.id) }
+      let(:projects) { Project.where(id: project.id).select(:id) }
+      let(:groups) { Group.where(id: group.id).select(:id) }
 
       it_behaves_like 'filters by projects and groups'
     end