Use a JOIN in IssuableFinder#by_project

When using IssuableFinder/IssuesFinder to find issues for multiple projects it's more efficient to use a JOIN + a "WHERE project_id IN" condition opposed to running a sub-query. This change means that when finding issues without labels we're now using the following SQL: SELECT issues.* FROM issues JOIN projects ON projects.id = issues.project_id LEFT JOIN label_links ON label_links.target_type = 'Issue' AND label_links.target_id = issues.id WHERE ( projects.id IN (...) OR projects.visibility_level IN (20, 10) ) AND issues.state IN ('opened','reopened') AND label_links.id IS NULL ORDER BY issues.id DESC; instead of: SELECT issues.* FROM issues LEFT JOIN label_links ON label_links.target_type = 'Issue' AND label_links.target_id = issues.id WHERE issues.project_id IN ( SELECT id FROM projects WHERE id IN (...) OR visibility_level IN (20,10) ) AND issues.state IN ('opened','reopened') AND label_links.id IS NULL ORDER BY issues.id DESC; The big benefit here is that in the last case PostgreSQL can't properly use all available indexes. In particular it ends up performing a sequence scan on the "label_links" table (processing around 290 000 rows). The new query is roughly 2x as fast as the old query.

Use a JOIN in IssuableFinder#by_project
8591cc02 · Yorick Peterse · e9cd58f5 · 8591cc02 · 8591cc02 · 8591cc02
Commit 8591cc02 authored Nov 11, 2015 by Yorick Peterse
Hide whitespace changes
Inline Side-by-side

Showing with 13 additions and 3 deletions

issuable_finder.rb app/finders/issuable_finder.rb +7 -3

issuable.rb app/models/concerns/issuable.rb +3 -0

merge_request.rb app/models/merge_request.rb +3 -0

No files found.
--- a/app/finders/issuable_finder.rb
+++ b/app/finders/issuable_finder.rb
@@ -190,8 +190,10 @@ class IssuableFinder

  def by_project(items)
    items =
-      if projects
-        items.of_projects(projects).references(:project)
+      if project?
+        items.of_projects(projects).references_project
+      elsif projects
+        items.merge(projects.reorder(nil)).join_project
      else
        items.none
      end
@@ -206,7 +208,9 @@ class IssuableFinder
  end

  def sort(items)
-    items.sort(params[:sort])
+    # Ensure we always have an explicit sort order (instead of inheriting
+    # multiple orders when combining ActiveRecord::Relation objects).
+    params[:sort] ? items.sort(params[:sort]) : items.reorder(id: :desc)
  end

  def by_assignee(items)

--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -35,6 +35,9 @@ module Issuable
    scope :order_milestone_due_desc, -> { joins(:milestone).reorder('milestones.due_date DESC, milestones.id DESC') }
    scope :order_milestone_due_asc, -> { joins(:milestone).reorder('milestones.due_date ASC, milestones.id ASC') }

+    scope :join_project, -> { joins(:project) }
+    scope :references_project, -> { references(:project) }
+
    delegate :name,
             :email,
             to: :author,

--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -134,6 +134,9 @@ class MergeRequest < ActiveRecord::Base
  scope :closed, -> { with_state(:closed) }
  scope :closed_and_merged, -> { with_states(:closed, :merged) }

+  scope :join_project, -> { joins(:target_project) }
+  scope :references_project, -> { references(:target_project) }
+
  def self.reference_prefix
    '!'
  end