From 516aaad3410c242b47ee270b4cdca8e56f6b379b Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 6 Mar 2018 20:25:17 -0800
Subject: [PATCH] Use "pathIndex" in some owners package queries to improve
 query plans

Summary: Depends on D19184. Ref T11015. Now that we have a digest index column, we can improve some of the queries a bit.

Test Plan:
  - Ran queries from revision pages before and after with and without EXPLAIN.
  - Saw the same results with much better EXPLAIN plans.
  - Fragment size is now fixed at 12 bytes per fragment, so we can shove more of them in a single query.

Maniphest Tasks: T11015

Differential Revision: https://secure.phabricator.com/D19185
---
 .../query/PhabricatorOwnersPackageQuery.php   | 20 ++++++++++++++-----
 .../storage/PhabricatorOwnersPackage.php      | 11 +++++++---
 .../owners/storage/PhabricatorOwnersPath.php  |  3 +++
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php
index ce411aad35..ff0665b344 100644
--- a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php
+++ b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php
@@ -206,8 +206,8 @@ final class PhabricatorOwnersPackageQuery
     if ($this->paths !== null) {
       $where[] = qsprintf(
         $conn,
-        'rpath.path IN (%Ls)',
-        $this->getFragmentsForPaths($this->paths));
+        'rpath.pathIndex IN (%Ls)',
+        $this->getFragmentIndexesForPaths($this->paths));
     }
 
     if ($this->statuses !== null) {
@@ -220,13 +220,13 @@ final class PhabricatorOwnersPackageQuery
     if ($this->controlMap) {
       $clauses = array();
       foreach ($this->controlMap as $repository_phid => $paths) {
-        $fragments = $this->getFragmentsForPaths($paths);
+        $indexes = $this->getFragmentIndexesForPaths($paths);
 
         $clauses[] = qsprintf(
           $conn,
-          '(rpath.repositoryPHID = %s AND rpath.path IN (%Ls))',
+          '(rpath.repositoryPHID = %s AND rpath.pathIndex IN (%Ls))',
           $repository_phid,
-          $fragments);
+          $indexes);
       }
       $where[] = implode(' OR ', $clauses);
     }
@@ -333,6 +333,16 @@ final class PhabricatorOwnersPackageQuery
     return $fragments;
   }
 
+  private function getFragmentIndexesForPaths(array $paths) {
+    $indexes = array();
+
+    foreach ($this->getFragmentsForPaths($paths) as $fragment) {
+      $indexes[] = PhabricatorHash::digestForIndex($fragment);
+    }
+
+    return $indexes;
+  }
+
 
 /* -(  Path Control  )------------------------------------------------------- */
 
diff --git a/src/applications/owners/storage/PhabricatorOwnersPackage.php b/src/applications/owners/storage/PhabricatorOwnersPackage.php
index 60d244052f..7f155d7c89 100644
--- a/src/applications/owners/storage/PhabricatorOwnersPackage.php
+++ b/src/applications/owners/storage/PhabricatorOwnersPackage.php
@@ -218,15 +218,20 @@ final class PhabricatorOwnersPackage
     // and then merge results in PHP.
 
     $rows = array();
-    foreach (array_chunk(array_keys($fragments), 128) as $chunk) {
+    foreach (array_chunk(array_keys($fragments), 1024) as $chunk) {
+      $indexes = array();
+      foreach ($chunk as $fragment) {
+        $indexes[] = PhabricatorHash::digestForIndex($fragment);
+      }
+
       $rows[] = queryfx_all(
         $conn,
         'SELECT pkg.id, pkg.dominion, p.excluded, p.path
           FROM %T pkg JOIN %T p ON p.packageID = pkg.id
-          WHERE p.path IN (%Ls) AND pkg.status IN (%Ls) %Q',
+          WHERE p.pathIndex IN (%Ls) AND pkg.status IN (%Ls) %Q',
         $package->getTableName(),
         $path->getTableName(),
-        $chunk,
+        $indexes,
         array(
           self::STATUS_ACTIVE,
         ),
diff --git a/src/applications/owners/storage/PhabricatorOwnersPath.php b/src/applications/owners/storage/PhabricatorOwnersPath.php
index 0ef7fce59b..7022cb887c 100644
--- a/src/applications/owners/storage/PhabricatorOwnersPath.php
+++ b/src/applications/owners/storage/PhabricatorOwnersPath.php
@@ -26,6 +26,9 @@ final class PhabricatorOwnersPath extends PhabricatorOwnersDAO {
           'columns' => array('packageID', 'repositoryPHID', 'pathIndex'),
           'unique' => true,
         ),
+        'key_repository' => array(
+          'columns' => array('repositoryPHID', 'pathIndex'),
+        ),
       ),
     ) + parent::getConfiguration();
   }