diff --git a/src/applications/differential/query/DifferentialChangesetQuery.php b/src/applications/differential/query/DifferentialChangesetQuery.php --- a/src/applications/differential/query/DifferentialChangesetQuery.php +++ b/src/applications/differential/query/DifferentialChangesetQuery.php @@ -150,8 +150,4 @@ return 'PhabricatorDifferentialApplication'; } - protected function getReversePaging() { - return true; - } - } diff --git a/src/applications/maniphest/query/ManiphestTaskQuery.php b/src/applications/maniphest/query/ManiphestTaskQuery.php --- a/src/applications/maniphest/query/ManiphestTaskQuery.php +++ b/src/applications/maniphest/query/ManiphestTaskQuery.php @@ -969,13 +969,11 @@ } protected function buildPagingClause(AphrontDatabaseConnection $conn_r) { - $default = parent::buildPagingClause($conn_r); - $before_id = $this->getBeforeID(); $after_id = $this->getAfterID(); if (!$before_id && !$after_id) { - return $default; + return ''; } $cursor_id = nonempty($before_id, $after_id); @@ -1004,28 +1002,24 @@ ); break; case self::GROUP_OWNER: - $columns[] = array( - 'table' => 'task', - 'column' => 'ownerOrdering', - 'value' => strlen($group_id), - 'type' => 'null', - ); + $value = null; if ($group_id) { $paging_users = id(new PhabricatorPeopleQuery()) ->setViewer($this->getViewer()) ->withPHIDs(array($group_id)) ->execute(); - if (!$paging_users) { - return null; + if ($paging_users) { + $value = head($paging_users)->getUsername(); } - $columns[] = array( - 'table' => 'task', - 'column' => 'ownerOrdering', - 'value' => head($paging_users)->getUsername(), - 'type' => 'string', - 'reverse' => true, - ); } + $columns[] = array( + 'table' => 'task', + 'column' => 'ownerOrdering', + 'value' => $value, + 'type' => 'string', + 'null' => 'head', + 'reverse' => true, + ); break; case self::GROUP_STATUS: $columns[] = array( @@ -1036,28 +1030,25 @@ ); break; case self::GROUP_PROJECT: - $columns[] = array( - 'table' => 'projectGroupName', - 'column' => 'indexedObjectName', - 'value' => strlen($group_id), - 'type' => 'null', - ); + $value = null; if ($group_id) { $paging_projects = id(new PhabricatorProjectQuery()) ->setViewer($this->getViewer()) ->withPHIDs(array($group_id)) ->execute(); - if (!$paging_projects) { - return null; + if ($paging_projects) { + $value = head($paging_projects)->getName(); } - $columns[] = array( - 'table' => 'projectGroupName', - 'column' => 'indexedObjectName', - 'value' => head($paging_projects)->getName(), - 'type' => 'string', - 'reverse' => true, - ); } + + $columns[] = array( + 'table' => 'projectGroupName', + 'column' => 'indexedObjectName', + 'value' => $value, + 'type' => 'string', + 'null' => 'head', + 'reverse' => true, + ); break; default: throw new Exception("Unknown group query '{$this->groupBy}'!"); diff --git a/src/applications/phrequent/query/PhrequentUserTimeQuery.php b/src/applications/phrequent/query/PhrequentUserTimeQuery.php --- a/src/applications/phrequent/query/PhrequentUserTimeQuery.php +++ b/src/applications/phrequent/query/PhrequentUserTimeQuery.php @@ -56,10 +56,10 @@ $this->setOrderVector(array('start', 'id')); break; case self::ORDER_ENDED_ASC: - $this->setOrderVector(array('-ongoing', '-end', '-id')); + $this->setOrderVector(array('-end', '-id')); break; case self::ORDER_ENDED_DESC: - $this->setOrderVector(array('ongoing', 'end', 'id')); + $this->setOrderVector(array('end', 'id')); break; default: throw new Exception(pht('Unknown order "%s".', $order)); @@ -125,13 +125,10 @@ 'column' => 'dateStarted', 'type' => 'int', ), - 'ongoing' => array( - 'column' => 'dateEnded', - 'type' => 'null', - ), 'end' => array( 'column' => 'dateEnded', 'type' => 'int', + 'null' => 'head', ), ); } @@ -141,7 +138,6 @@ return array( 'id' => $usertime->getID(), 'start' => $usertime->getDateStarted(), - 'ongoing' => $usertime->getDateEnded(), 'end' => $usertime->getDateEnded(), ); } diff --git a/src/applications/repository/query/PhabricatorRepositoryQuery.php b/src/applications/repository/query/PhabricatorRepositoryQuery.php --- a/src/applications/repository/query/PhabricatorRepositoryQuery.php +++ b/src/applications/repository/query/PhabricatorRepositoryQuery.php @@ -28,7 +28,6 @@ const ORDER_CALLSIGN = 'order-callsign'; const ORDER_NAME = 'order-name'; const ORDER_SIZE = 'order-size'; - private $order = self::ORDER_CREATED; const HOSTED_PHABRICATOR = 'hosted-phab'; const HOSTED_REMOTE = 'hosted-remote'; @@ -126,7 +125,25 @@ } public function setOrder($order) { - $this->order = $order; + switch ($order) { + case self::ORDER_CREATED: + $this->setOrderVector(array('id')); + break; + case self::ORDER_COMMITTED: + $this->setOrderVector(array('committed', 'id')); + break; + case self::ORDER_CALLSIGN: + $this->setOrderVector(array('callsign')); + break; + case self::ORDER_NAME: + $this->setOrderVector(array('name', 'id')); + break; + case self::ORDER_SIZE: + $this->setOrderVector(array('size', 'id')); + break; + default: + throw new Exception(pht('Unknown order "%s".', $order)); + } return $this; } @@ -152,7 +169,7 @@ $table->getTableName(), $this->buildJoinsClause($conn_r), $this->buildWhereClause($conn_r), - $this->buildCustomOrderClause($conn_r), + $this->buildOrderClause($conn_r), $this->buildLimitClause($conn_r)); $repositories = $table->loadAllFromArray($data); @@ -295,162 +312,97 @@ return $repositories; } - protected function buildCustomOrderClause(AphrontDatabaseConnection $conn) { - $parts = array(); - - $order = $this->order; - switch ($order) { - case self::ORDER_CREATED: - break; - case self::ORDER_COMMITTED: - $parts[] = array( - 'table' => 's', - 'column' => 'epoch', - ); - break; - case self::ORDER_CALLSIGN: - $parts[] = array( - 'table' => 'r', - 'column' => 'callsign', - 'reverse' => true, - ); - break; - case self::ORDER_NAME: - $parts[] = array( - 'table' => 'r', - 'column' => 'name', - 'reverse' => true, - ); - break; - case self::ORDER_SIZE: - $parts[] = array( - 'table' => 's', - 'column' => 'size', - ); - break; - default: - throw new Exception("Unknown order '{$order}!'"); - } + public function getPrimaryTableAlias() { + return 'r'; + } - $parts[] = array( - 'table' => 'r', - 'column' => 'id', + public function getOrderableColumns() { + return parent::getOrderableColumns() + array( + 'committed' => array( + 'table' => 's', + 'column' => 'epoch', + 'type' => 'int', + 'null' => 'tail', + ), + 'callsign' => array( + 'table' => 'r', + 'column' => 'callsign', + 'type' => 'string', + 'unique' => true, + 'reverse' => true, + ), + 'name' => array( + 'table' => 'r', + 'column' => 'name', + 'type' => 'string', + 'reverse' => true, + ), + 'size' => array( + 'table' => 's', + 'column' => 'size', + 'type' => 'int', + 'null' => 'tail', + ), ); - - return $this->formatOrderClause($conn, $parts); } - protected function loadCursorObject($id) { - $query = id(new PhabricatorRepositoryQuery()) - ->setViewer($this->getPagingViewer()) - ->withIDs(array((int)$id)); + protected function willExecuteCursorQuery( + PhabricatorCursorPagedPolicyAwareQuery $query) { + $vector = $this->getOrderVector(); - if ($this->order == self::ORDER_COMMITTED) { + if ($vector->containsKey('committed')) { $query->needMostRecentCommits(true); } - if ($this->order == self::ORDER_SIZE) { + if ($vector->containsKey('size')) { $query->needCommitCounts(true); } - - $results = $query->execute(); - return head($results); } - protected function buildPagingClause(AphrontDatabaseConnection $conn_r) { - $default = parent::buildPagingClause($conn_r); + protected function getPagingValueMap($cursor, array $keys) { + $repository = $this->loadCursorObject($cursor); - $before_id = $this->getBeforeID(); - $after_id = $this->getAfterID(); - - if (!$before_id && !$after_id) { - return $default; - } - - $order = $this->order; - if ($order == self::ORDER_CREATED) { - return $default; - } - - if ($before_id) { - $cursor = $this->loadCursorObject($before_id); - } else { - $cursor = $this->loadCursorObject($after_id); - } - - if (!$cursor) { - return null; - } - - $id_column = array( - 'table' => 'r', - 'column' => 'id', - 'type' => 'int', - 'value' => $cursor->getID(), + $map = array( + 'id' => $repository->getID(), + 'callsign' => $repository->getCallsign(), + 'name' => $repository->getName(), ); - $columns = array(); - switch ($order) { - case self::ORDER_COMMITTED: - $commit = $cursor->getMostRecentCommit(); - if (!$commit) { - return null; - } - $columns[] = array( - 'table' => 's', - 'column' => 'epoch', - 'type' => 'int', - 'value' => $commit->getEpoch(), - ); - $columns[] = $id_column; - break; - case self::ORDER_CALLSIGN: - $columns[] = array( - 'table' => 'r', - 'column' => 'callsign', - 'type' => 'string', - 'value' => $cursor->getCallsign(), - 'reverse' => true, - ); - break; - case self::ORDER_NAME: - $columns[] = array( - 'table' => 'r', - 'column' => 'name', - 'type' => 'string', - 'value' => $cursor->getName(), - 'reverse' => true, - ); - $columns[] = $id_column; - break; - case self::ORDER_SIZE: - $columns[] = array( - 'table' => 's', - 'column' => 'size', - 'type' => 'int', - 'value' => $cursor->getCommitCount(), - ); - $columns[] = $id_column; - break; - default: - throw new Exception("Unknown order '{$order}'!"); + foreach ($keys as $key) { + switch ($key) { + case 'committed': + $commit = $repository->getMostRecentCommit(); + if ($commit) { + $map[$key] = $commit->getEpoch(); + } else { + $map[$key] = null; + } + break; + case 'size': + $count = $repository->getCommitCount(); + if ($count) { + $map[$key] = $count; + } else { + $map[$key] = null; + } + break; + } } - return $this->buildPagingClauseFromMultipleColumns( - $conn_r, - $columns, - array( - 'reversed' => ($this->getReversePaging() xor (bool)($before_id)), - )); + return $map; } private function buildJoinsClause(AphrontDatabaseConnection $conn_r) { $joins = array(); $join_summary_table = $this->needCommitCounts || - $this->needMostRecentCommits || - ($this->order == self::ORDER_COMMITTED) || - ($this->order == self::ORDER_SIZE); + $this->needMostRecentCommits; + + $vector = $this->getOrderVector(); + if ($vector->containsKey('committed') || + $vector->containsKey('size')) { + $join_summary_table = true; + } if ($join_summary_table) { $joins[] = qsprintf( @@ -554,6 +506,8 @@ return $this->formatWhereClause($where); } + + public function getQueryApplicationClass() { return 'PhabricatorDiffusionApplication'; } diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php --- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php @@ -320,6 +320,7 @@ 'type' => 'string', 'reverse' => 'optional bool', 'unique' => 'optional bool', + 'null' => 'optional string|null', )); } @@ -336,21 +337,36 @@ $last_key = last_key($columns); foreach ($columns as $key => $column) { $type = $column['type']; - switch ($type) { - case 'null': - $value = qsprintf($conn, '%d', ($column['value'] ? 0 : 1)); - break; - case 'int': - $value = qsprintf($conn, '%d', $column['value']); - break; - case 'float': - $value = qsprintf($conn, '%f', $column['value']); - break; - case 'string': - $value = qsprintf($conn, '%s', $column['value']); - break; - default: - throw new Exception("Unknown column type '{$type}'!"); + + $null = idx($column, 'null'); + if ($column['value'] === null) { + if ($null) { + $value = null; + } else { + throw new Exception( + pht( + 'Column "%s" has null value, but does not specify a null '. + 'behavior.', + $key)); + } + } else { + switch ($type) { + case 'int': + $value = qsprintf($conn, '%d', $column['value']); + break; + case 'float': + $value = qsprintf($conn, '%f', $column['value']); + break; + case 'string': + $value = qsprintf($conn, '%s', $column['value']); + break; + default: + throw new Exception( + pht( + 'Column "%s" has unknown column type "%s".', + $column['column'], + $type)); + } } $is_column_reversed = idx($column, 'reverse', false); @@ -366,23 +382,67 @@ $field = qsprintf($conn, '%T', $column_name); } - if ($type == 'null') { - $field = qsprintf($conn, '(%Q IS NULL)', $field); + $parts = array(); + if ($null) { + $can_page_if_null = ($null === 'head'); + $can_page_if_nonnull = ($null === 'tail'); + + if ($reverse) { + $can_page_if_null = !$can_page_if_null; + $can_page_if_nonnull = !$can_page_if_nonnull; + } + + $subclause = null; + if ($can_page_if_null && $value === null) { + $parts[] = qsprintf( + $conn, + '(%Q IS NOT NULL)', + $field); + } else if ($can_page_if_nonnull && $value !== null) { + $parts[] = qsprintf( + $conn, + '(%Q IS NULL)', + $field); + } + } + + if ($value !== null) { + $parts[] = qsprintf( + $conn, + '%Q %Q %Q', + $field, + $reverse ? '>' : '<', + $value); + } + + if ($parts) { + if (count($parts) > 1) { + $clause[] = '('.implode(') OR (', $parts).')'; + } else { + $clause[] = head($parts); + } } - $clause[] = qsprintf( - $conn, - '%Q %Q %Q', - $field, - $reverse ? '>' : '<', - $value); - $clauses[] = '('.implode(') AND (', $clause).')'; + if ($clause) { + if (count($clause) > 1) { + $clauses[] = '('.implode(') AND (', $clause).')'; + } else { + $clauses[] = head($clause); + } + } - $accumulated[] = qsprintf( - $conn, - '%Q = %Q', - $field, - $value); + if ($value === null) { + $accumulated[] = qsprintf( + $conn, + '%Q IS NULL', + $field); + } else { + $accumulated[] = qsprintf( + $conn, + '%Q = %Q', + $field, + $value); + } } return '('.implode(') OR (', $clauses).')'; @@ -576,8 +636,28 @@ $field = qsprintf($conn, '%T', $column); } - if (idx($part, 'type') === 'null') { - $field = qsprintf($conn, '(%Q IS NULL)', $field); + $null = idx($part, 'null'); + if ($null) { + switch ($null) { + case 'head': + $null_field = qsprintf($conn, '(%Q IS NULL)', $field); + break; + case 'tail': + $null_field = qsprintf($conn, '(%Q IS NOT NULL)', $field); + break; + default: + throw new Exception( + pht( + 'NULL value "%s" is invalid. Valid values are "head" and '. + '"tail".', + $null)); + } + + if ($descending) { + $sql[] = qsprintf($conn, '%Q DESC', $null_field); + } else { + $sql[] = qsprintf($conn, '%Q ASC', $null_field); + } } if ($descending) {