From 4fba6e7730aaf97513d35a6f1adf7e17269305b5 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 11 Apr 2015 19:00:53 -0700 Subject: [PATCH] Remove trivial implementations of getPagingColumn() Summary: Ref T7803. Some Query subclasses implement getPagingColumn() in a trivial way, usually to provide a table alias. Formalize the concept of a primary table alias, and remove obsoleted getPagingColumn() implementations. Test Plan: Issued affected queries. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T7803 Differential Revision: https://secure.phabricator.com/D12356 --- .../files/query/PhabricatorFileQuery.php | 4 +-- .../macro/query/PhabricatorMacroQuery.php | 4 +-- .../maniphest/query/ManiphestTaskQuery.php | 4 +-- ...habricatorMetaMTAApplicationEmailQuery.php | 8 ++--- .../people/query/PhabricatorPeopleQuery.php | 8 ++--- .../project/query/PhabricatorProjectQuery.php | 4 +-- .../query/PhabricatorSlowvoteQuery.php | 4 +-- ...PhabricatorCursorPagedPolicyAwareQuery.php | 33 ++++++++++++++++--- 8 files changed, 42 insertions(+), 27 deletions(-) diff --git a/src/applications/files/query/PhabricatorFileQuery.php b/src/applications/files/query/PhabricatorFileQuery.php index b986c0ee6f..ad0bbbac1b 100644 --- a/src/applications/files/query/PhabricatorFileQuery.php +++ b/src/applications/files/query/PhabricatorFileQuery.php @@ -331,8 +331,8 @@ final class PhabricatorFileQuery return $this->formatWhereClause($where); } - protected function getPagingColumn() { - return 'f.id'; + protected function getPrimaryTableAlias() { + return 'f'; } public function getQueryApplicationClass() { diff --git a/src/applications/macro/query/PhabricatorMacroQuery.php b/src/applications/macro/query/PhabricatorMacroQuery.php index c7f9534499..a326f97697 100644 --- a/src/applications/macro/query/PhabricatorMacroQuery.php +++ b/src/applications/macro/query/PhabricatorMacroQuery.php @@ -225,8 +225,8 @@ final class PhabricatorMacroQuery return $macros; } - protected function getPagingColumn() { - return 'm.id'; + protected function getPrimaryTableAlias() { + return 'm'; } public function getQueryApplicationClass() { diff --git a/src/applications/maniphest/query/ManiphestTaskQuery.php b/src/applications/maniphest/query/ManiphestTaskQuery.php index d3f178f3d6..c3e1388425 100644 --- a/src/applications/maniphest/query/ManiphestTaskQuery.php +++ b/src/applications/maniphest/query/ManiphestTaskQuery.php @@ -1137,8 +1137,8 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery { )); } - protected function getApplicationSearchObjectPHIDColumn() { - return 'task.phid'; + protected function getPrimaryTableAlias() { + return 'task'; } public function getQueryApplicationClass() { diff --git a/src/applications/metamta/query/PhabricatorMetaMTAApplicationEmailQuery.php b/src/applications/metamta/query/PhabricatorMetaMTAApplicationEmailQuery.php index 3983e362cc..d2ac2f17a6 100644 --- a/src/applications/metamta/query/PhabricatorMetaMTAApplicationEmailQuery.php +++ b/src/applications/metamta/query/PhabricatorMetaMTAApplicationEmailQuery.php @@ -114,12 +114,8 @@ final class PhabricatorMetaMTAApplicationEmailQuery return $this->formatWhereClause($where); } - protected function getPagingColumn() { - return 'appemail.id'; - } - - protected function getApplicationSearchObjectPHIDColumn() { - return 'appemail.phid'; + protected function getPrimaryTableAlias() { + return 'appemail'; } public function getQueryApplicationClass() { diff --git a/src/applications/people/query/PhabricatorPeopleQuery.php b/src/applications/people/query/PhabricatorPeopleQuery.php index f76ab9e5aa..f79957dfa2 100644 --- a/src/applications/people/query/PhabricatorPeopleQuery.php +++ b/src/applications/people/query/PhabricatorPeopleQuery.php @@ -288,12 +288,8 @@ final class PhabricatorPeopleQuery return $this->formatWhereClause($where); } - protected function getPagingColumn() { - return 'user.id'; - } - - protected function getApplicationSearchObjectPHIDColumn() { - return 'user.phid'; + protected function getPrimaryTableAlias() { + return 'user'; } public function getQueryApplicationClass() { diff --git a/src/applications/project/query/PhabricatorProjectQuery.php b/src/applications/project/query/PhabricatorProjectQuery.php index 486055691a..62db2b2aed 100644 --- a/src/applications/project/query/PhabricatorProjectQuery.php +++ b/src/applications/project/query/PhabricatorProjectQuery.php @@ -380,8 +380,8 @@ final class PhabricatorProjectQuery return 'PhabricatorProjectApplication'; } - protected function getApplicationSearchObjectPHIDColumn() { - return 'p.phid'; + protected function getPrimaryTableAlias() { + return 'p'; } } diff --git a/src/applications/slowvote/query/PhabricatorSlowvoteQuery.php b/src/applications/slowvote/query/PhabricatorSlowvoteQuery.php index f1ec5358f4..d17672b5e3 100644 --- a/src/applications/slowvote/query/PhabricatorSlowvoteQuery.php +++ b/src/applications/slowvote/query/PhabricatorSlowvoteQuery.php @@ -174,8 +174,8 @@ final class PhabricatorSlowvoteQuery return implode(' ', $joins); } - protected function getPagingColumn() { - return 'p.id'; + protected function getPrimaryTableAlias() { + return 'p'; } public function getQueryApplicationClass() { diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php index 2f89890058..af8b492bab 100644 --- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php @@ -147,6 +147,21 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery } + /** + * Return the alias this query uses to identify the primary table. + * + * Some automatic query constructions may need to be qualified with a table + * alias if the query performs joins which make column names ambiguous. If + * this is the case, return the alias for the primary table the query + * uses; generally the object table which has `id` and `phid` columns. + * + * @return string Alias for the primary table. + */ + protected function getPrimaryTableAlias() { + return null; + } + + /* -( Paging )------------------------------------------------------------- */ @@ -450,7 +465,7 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery return array( 'id' => array( - 'table' => null, + 'table' => $this->getPrimaryTableAlias(), 'column' => 'id', 'reverse' => false, 'type' => 'int', @@ -657,15 +672,23 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery /** * Get the name of the query's primary object PHID column, for constructing - * JOIN clauses. Normally (and by default) this is just `"phid"`, but if the - * query construction requires a table alias it may be something like - * `"task.phid"`. + * JOIN clauses. Normally (and by default) this is just `"phid"`, but it may + * be something more exotic. + * + * See @{method:getPrimaryTableAlias} if the column needs to be qualified with + * a table alias. * * @return string Column name. * @task appsearch */ protected function getApplicationSearchObjectPHIDColumn() { - return 'phid'; + if ($this->getPrimaryTableAlias()) { + $prefix = $this->getPrimaryTableAlias().'.'; + } else { + $prefix = ''; + } + + return $prefix.'phid'; }