From 8591cc02be6b12ed60f763a5e0147f2cbbca99e1 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Wed, 11 Nov 2015 12:50:36 +0100
Subject: 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.
---
 app/finders/issuable_finder.rb  | 10 +++++++---
 app/models/concerns/issuable.rb |  3 +++
 app/models/merge_request.rb     |  3 +++
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb
index 15b5d6ab34c..3d5e8b6fbe7 100644
--- 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)
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 492a026add9..97d3cb18f4e 100644
--- 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,
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 2eb03b8ba5b..1e8d9908f0a 100644
--- 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
-- 
cgit v1.2.1