From 5e276df9c00e818bc4fe605dadb245e49b03cd1a Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 28 Sep 2017 12:48:19 -0700
Subject: [PATCH] (stable) Allow the fulltext index to select only transactions
 with comments

Summary:
Ref T12997. Although we can't query by transaction type (since we can't easily enumerate all possible types which may have comments -- inline types may also have comments), we //can// just check if there's a comment row or not.

This reduces the amount of garbage we need to load to rebuild indexes for unusual objects with hundreds and hundreds of mentions.

Test Plan:
  - Used batch editor to mention a task 700 times.
  - Indexed it before and after this change, saw index time drop from {nav 1600ms > 160ms}.
  - Made some new comments on it, verified that they still indexed/queried properly.
  - Browsed around, made normal transactions, made inline comments.
  - Added a unique word to an inline comment, indexed revision, searched for word, found revision.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T12997

Differential Revision: https://secure.phabricator.com/D18660
---
 ...torTransactionsFulltextEngineExtension.php |  1 +
 ...PhabricatorApplicationTransactionQuery.php | 95 ++++++++++++-------
 2 files changed, 63 insertions(+), 33 deletions(-)

diff --git a/src/applications/transactions/engineextension/PhabricatorTransactionsFulltextEngineExtension.php b/src/applications/transactions/engineextension/PhabricatorTransactionsFulltextEngineExtension.php
index 701ad34ca7..c9f43f49b2 100644
--- a/src/applications/transactions/engineextension/PhabricatorTransactionsFulltextEngineExtension.php
+++ b/src/applications/transactions/engineextension/PhabricatorTransactionsFulltextEngineExtension.php
@@ -25,6 +25,7 @@ final class PhabricatorTransactionsFulltextEngineExtension
     $xactions = $query
       ->setViewer($this->getViewer())
       ->withObjectPHIDs(array($object->getPHID()))
+      ->withComments(true)
       ->needComments(true)
       ->execute();
 
diff --git a/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php b/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php
index da8454c7fc..abe1498fc0 100644
--- a/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php
+++ b/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php
@@ -7,6 +7,7 @@ abstract class PhabricatorApplicationTransactionQuery
   private $objectPHIDs;
   private $authorPHIDs;
   private $transactionTypes;
+  private $withComments;
 
   private $needComments = true;
   private $needHandles  = true;
@@ -34,10 +35,6 @@ abstract class PhabricatorApplicationTransactionQuery
 
   abstract public function getTemplateApplicationTransaction();
 
-  protected function buildMoreWhereClauses(AphrontDatabaseConnection $conn_r) {
-    return array();
-  }
-
   public function withPHIDs(array $phids) {
     $this->phids = $phids;
     return $this;
@@ -58,6 +55,11 @@ abstract class PhabricatorApplicationTransactionQuery
     return $this;
   }
 
+  public function withComments($with_comments) {
+    $this->withComments = $with_comments;
+    return $this;
+  }
+
   public function needComments($need) {
     $this->needComments = $need;
     return $this;
@@ -70,17 +72,8 @@ abstract class PhabricatorApplicationTransactionQuery
 
   protected function loadPage() {
     $table = $this->getTemplateApplicationTransaction();
-    $conn_r = $table->establishConnection('r');
 
-    $data = queryfx_all(
-      $conn_r,
-      'SELECT * FROM %T x %Q %Q %Q',
-      $table->getTableName(),
-      $this->buildWhereClause($conn_r),
-      $this->buildOrderClause($conn_r),
-      $this->buildLimitClause($conn_r));
-
-    $xactions = $table->loadAllFromArray($data);
+    $xactions = $this->loadStandardPage($table);
 
     foreach ($xactions as $xaction) {
       $xaction->attachViewer($this->getViewer());
@@ -161,50 +154,86 @@ abstract class PhabricatorApplicationTransactionQuery
     return $xactions;
   }
 
-  protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {
-    $where = array();
+  protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
+    $where = parent::buildWhereClauseParts($conn);
 
-    if ($this->phids) {
+    if ($this->phids !== null) {
       $where[] = qsprintf(
-        $conn_r,
-        'phid IN (%Ls)',
+        $conn,
+        'x.phid IN (%Ls)',
         $this->phids);
     }
 
-    if ($this->objectPHIDs) {
+    if ($this->objectPHIDs !== null) {
       $where[] = qsprintf(
-        $conn_r,
-        'objectPHID IN (%Ls)',
+        $conn,
+        'x.objectPHID IN (%Ls)',
         $this->objectPHIDs);
     }
 
-    if ($this->authorPHIDs) {
+    if ($this->authorPHIDs !== null) {
       $where[] = qsprintf(
-        $conn_r,
-        'authorPHID IN (%Ls)',
+        $conn,
+        'x.authorPHID IN (%Ls)',
         $this->authorPHIDs);
     }
 
-    if ($this->transactionTypes) {
+    if ($this->transactionTypes !== null) {
       $where[] = qsprintf(
-        $conn_r,
-        'transactionType IN (%Ls)',
+        $conn,
+        'x.transactionType IN (%Ls)',
         $this->transactionTypes);
     }
 
-    foreach ($this->buildMoreWhereClauses($conn_r) as $clause) {
-      $where[] = $clause;
+    if ($this->withComments !== null) {
+      if (!$this->withComments) {
+        $where[] = qsprintf(
+          $conn,
+          'c.id IS NULL');
+      }
     }
 
-    $where[] = $this->buildPagingClause($conn_r);
-
-    return $this->formatWhereClause($where);
+    return $where;
   }
 
+  protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) {
+    $joins = parent::buildJoinClauseParts($conn);
+
+    if ($this->withComments !== null) {
+      $xaction = $this->getTemplateApplicationTransaction();
+      $comment = $xaction->getApplicationTransactionCommentObject();
+
+      if ($this->withComments) {
+        $joins[] = qsprintf(
+          $conn,
+          'JOIN %T c ON x.phid = c.transactionPHID',
+          $comment->getTableName());
+      } else {
+        $joins[] = qsprintf(
+          $conn,
+          'LEFT JOIN %T c ON x.phid = c.transactionPHID',
+          $comment->getTableName());
+      }
+    }
+
+    return $joins;
+  }
+
+  protected function shouldGroupQueryResultRows() {
+    if ($this->withComments !== null) {
+      return true;
+    }
+
+    return parent::shouldGroupQueryResultRows();
+  }
 
   public function getQueryApplicationClass() {
     // TODO: Sort this out?
     return null;
   }
 
+  protected function getPrimaryTableAlias() {
+    return 'x';
+  }
+
 }