Modernize Phrequent and Commit query ordering/paging

Summary: Ref T7803. Fixes T3870. Move these away from pagingColumn / reversePaging.

Test Plan:
  - Tested/paged audit query.
  - Poked at Phrequent. Didn't seem any more broken than before.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3870, T7803

Differential Revision: https://secure.phabricator.com/D12363

src/applications/diffusion/query/DiffusionCommitQuery.php

@@ -149,8 +149,8 @@ final class DiffusionCommitQuery
     return $this->identifierMap;
   }
 
-  protected function getPagingColumn() {
+  protected function getPrimaryTableAlias() {
-    return 'commit.id';
+    return 'commit';
   }
 
   protected function willExecute() {

src/applications/phrequent/query/PhrequentUserTimeQuery.php

@@ -9,26 +9,29 @@ final class PhrequentUserTimeQuery
   const ORDER_STARTED_DESC  = 3;
   const ORDER_ENDED_ASC     = 4;
   const ORDER_ENDED_DESC    = 5;
-  const ORDER_DURATION_ASC  = 6;
-  const ORDER_DURATION_DESC = 7;
 
   const ENDED_YES = 0;
   const ENDED_NO  = 1;
   const ENDED_ALL = 2;
 
+  private $ids;
   private $userPHIDs;
   private $objectPHIDs;
-  private $order = self::ORDER_ID_ASC;
   private $ended = self::ENDED_ALL;
 
   private $needPreemptingEvents;
 
-  public function withUserPHIDs($user_phids) {
+  public function withIDs(array $ids) {
+    $this->ids = $ids;
+    return $this;
+  }
+
+  public function withUserPHIDs(array $user_phids) {
     $this->userPHIDs = $user_phids;
     return $this;
   }
 
-  public function withObjectPHIDs($object_phids) {
+  public function withObjectPHIDs(array $object_phids) {
     $this->objectPHIDs = $object_phids;
     return $this;
   }
@@ -39,7 +42,29 @@ final class PhrequentUserTimeQuery
   }
 
   public function setOrder($order) {
-    $this->order = $order;
+    switch ($order) {
+      case self::ORDER_ID_ASC:
+        $this->setOrderVector(array('-id'));
+        break;
+      case self::ORDER_ID_DESC:
+        $this->setOrderVector(array('id'));
+        break;
+      case self::ORDER_STARTED_ASC:
+        $this->setOrderVector(array('-start', '-id'));
+        break;
+      case self::ORDER_STARTED_DESC:
+        $this->setOrderVector(array('start', 'id'));
+        break;
+      case self::ORDER_ENDED_ASC:
+        $this->setOrderVector(array('-ongoing', '-end', '-id'));
+        break;
+      case self::ORDER_ENDED_DESC:
+        $this->setOrderVector(array('ongoing', 'end', 'id'));
+        break;
+      default:
+        throw new Exception(pht('Unknown order "%s".', $order));
+    }
+
     return $this;
   }
 
@@ -51,14 +76,21 @@ final class PhrequentUserTimeQuery
   private function buildWhereClause(AphrontDatabaseConnection $conn) {
     $where = array();
 
-    if ($this->userPHIDs) {
+    if ($this->ids !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'id IN (%Ld)',
+        $this->ids);
+    }
+
+    if ($this->userPHIDs !== null) {
       $where[] = qsprintf(
         $conn,
         'userPHID IN (%Ls)',
         $this->userPHIDs);
     }
 
-    if ($this->objectPHIDs) {
+    if ($this->objectPHIDs !== null) {
       $where[] = qsprintf(
         $conn,
         'objectPHID IN (%Ls)',
@@ -87,59 +119,31 @@ final class PhrequentUserTimeQuery
     return $this->formatWhereClause($where);
   }
 
-  protected function getPagingColumn() {
+  public function getOrderableColumns() {
-    switch ($this->order) {
+    return parent::getOrderableColumns() + array(
-      case self::ORDER_ID_ASC:
+      'start' => array(
-      case self::ORDER_ID_DESC:
+        'column' => 'dateStarted',
-        return 'id';
+        'type' => 'int',
-      case self::ORDER_STARTED_ASC:
+      ),
-      case self::ORDER_STARTED_DESC:
+      'ongoing' => array(
-        return 'dateStarted';
+        'column' => 'dateEnded',
-      case self::ORDER_ENDED_ASC:
+        'type' => 'null',
-      case self::ORDER_ENDED_DESC:
+      ),
-        return 'dateEnded';
+      'end' => array(
-      case self::ORDER_DURATION_ASC:
+        'column' => 'dateEnded',
-      case self::ORDER_DURATION_DESC:
+        'type' => 'int',
-        return 'COALESCE(dateEnded, UNIX_TIMESTAMP()) - dateStarted';
+      ),
-      default:
+    );
-        throw new Exception("Unknown order '{$this->order}'!");
-    }
   }
 
-  protected function getPagingValue($result) {
+  protected function getPagingValueMap($cursor, array $keys) {
-    switch ($this->order) {
+    $usertime = $this->loadCursorObject($cursor);
-      case self::ORDER_ID_ASC:
+    return array(
-      case self::ORDER_ID_DESC:
+      'id' => $usertime->getID(),
-        return $result->getID();
+      'start' => $usertime->getDateStarted(),
-      case self::ORDER_STARTED_ASC:
+      'ongoing' => $usertime->getDateEnded(),
-      case self::ORDER_STARTED_DESC:
+      'end' => $usertime->getDateEnded(),
-        return $result->getDateStarted();
+    );
-      case self::ORDER_ENDED_ASC:
-      case self::ORDER_ENDED_DESC:
-        return $result->getDateEnded();
-      case self::ORDER_DURATION_ASC:
-      case self::ORDER_DURATION_DESC:
-        return ($result->getDateEnded() || time()) - $result->getDateStarted();
-      default:
-        throw new Exception("Unknown order '{$this->order}'!");
-    }
-  }
-
-  protected function getReversePaging() {
-    switch ($this->order) {
-      case self::ORDER_ID_ASC:
-      case self::ORDER_STARTED_ASC:
-      case self::ORDER_ENDED_ASC:
-      case self::ORDER_DURATION_ASC:
-        return true;
-      case self::ORDER_ID_DESC:
-      case self::ORDER_STARTED_DESC:
-      case self::ORDER_ENDED_DESC:
-      case self::ORDER_DURATION_DESC:
-        return false;
-      default:
-        throw new Exception("Unknown order '{$this->order}'!");
-    }
   }
 
   protected function loadPage() {
@@ -249,8 +253,6 @@ final class PhrequentUserTimeQuery
       self::ORDER_STARTED_DESC  => pht('by nearest start date'),
       self::ORDER_ENDED_ASC     => pht('by furthest end date'),
       self::ORDER_ENDED_DESC    => pht('by nearest end date'),
-      self::ORDER_DURATION_ASC  => pht('by smallest duration'),
-      self::ORDER_DURATION_DESC => pht('by largest duration'),
     );
   }
 

src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php

@@ -570,18 +570,20 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
       $table = idx($part, 'table');
       $column = $part['column'];
 
-      if ($descending) {
+      if ($table !== null) {
-        if ($table !== null) {
+        $field = qsprintf($conn, '%T.%T', $table, $column);
-          $sql[] = qsprintf($conn, '%T.%T DESC', $table, $column);
-        } else {
-          $sql[] = qsprintf($conn, '%T DESC', $column);
-        }
       } else {
-        if ($table !== null) {
+        $field = qsprintf($conn, '%T', $column);
-          $sql[] = qsprintf($conn, '%T.%T ASC', $table, $column);
+      }
-        } else {
+
-          $sql[] = qsprintf($conn, '%T ASC', $column);
+      if (idx($part, 'type') === 'null') {
-        }
+        $field = qsprintf($conn, '(%Q IS NULL)', $field);
+      }
+
+      if ($descending) {
+        $sql[] = qsprintf($conn, '%Q DESC', $field);
+      } else {
+        $sql[] = qsprintf($conn, '%Q ASC', $field);
       }
     }