mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-09 16:32:39 +01:00
When building audit queries, prefilter possible "authorPHID" values
Summary: Ref T13244. See PHI1057. Currently, if you're a member of a lot of projects/packages, you can end up with a very large `commit.authorPHID IN (...)` clause in part of the "Active Audits" query, since your `alice` token in "Responsible Users: alice" expands into every package and project you can audit on behalf of. It's impossible for a commit to be authored by anything but a user, and evidence in PHI1057 suggests this giant `IN (...)` list can prevent MySQL from making effective utilization of the `<authorPHID, auditStatus, ...>` key on the table. Prefilter the list of PHIDs to only PHIDs which can possibly author a commit. (We'll also eventually need to convert the `authorPHIDs` into `identityPHIDs` anyway, for T12164, and this moves us slightly toward that.) Test Plan: Loaded "Active Audits" before and after change, saw a more streamlined and sensible `authorPHID IN (...)` clause afterwards. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13244 Differential Revision: https://secure.phabricator.com/D20129
This commit is contained in:
parent
a4bab60ad0
commit
509fbb6c20
1 changed files with 44 additions and 7 deletions
|
@ -202,6 +202,7 @@ final class DiffusionCommitQuery
|
|||
$table = $this->newResultObject();
|
||||
$conn = $table->establishConnection('r');
|
||||
|
||||
$empty_exception = null;
|
||||
$subqueries = array();
|
||||
if ($this->responsiblePHIDs) {
|
||||
$base_authors = $this->authorPHIDs;
|
||||
|
@ -222,21 +223,33 @@ final class DiffusionCommitQuery
|
|||
|
||||
$this->authorPHIDs = $all_authors;
|
||||
$this->auditorPHIDs = $base_auditors;
|
||||
$subqueries[] = $this->buildStandardPageQuery(
|
||||
$conn,
|
||||
$table->getTableName());
|
||||
try {
|
||||
$subqueries[] = $this->buildStandardPageQuery(
|
||||
$conn,
|
||||
$table->getTableName());
|
||||
} catch (PhabricatorEmptyQueryException $ex) {
|
||||
$empty_exception = $ex;
|
||||
}
|
||||
|
||||
$this->authorPHIDs = $base_authors;
|
||||
$this->auditorPHIDs = $all_auditors;
|
||||
$subqueries[] = $this->buildStandardPageQuery(
|
||||
$conn,
|
||||
$table->getTableName());
|
||||
try {
|
||||
$subqueries[] = $this->buildStandardPageQuery(
|
||||
$conn,
|
||||
$table->getTableName());
|
||||
} catch (PhabricatorEmptyQueryException $ex) {
|
||||
$empty_exception = $ex;
|
||||
}
|
||||
} else {
|
||||
$subqueries[] = $this->buildStandardPageQuery(
|
||||
$conn,
|
||||
$table->getTableName());
|
||||
}
|
||||
|
||||
if (!$subqueries) {
|
||||
throw $empty_exception;
|
||||
}
|
||||
|
||||
if (count($subqueries) > 1) {
|
||||
$unions = null;
|
||||
foreach ($subqueries as $subquery) {
|
||||
|
@ -642,10 +655,19 @@ final class DiffusionCommitQuery
|
|||
}
|
||||
|
||||
if ($this->authorPHIDs !== null) {
|
||||
$author_phids = $this->authorPHIDs;
|
||||
if ($author_phids) {
|
||||
$author_phids = $this->selectPossibleAuthors($author_phids);
|
||||
if (!$author_phids) {
|
||||
throw new PhabricatorEmptyQueryException(
|
||||
pht('Author PHIDs contain no possible authors.'));
|
||||
}
|
||||
}
|
||||
|
||||
$where[] = qsprintf(
|
||||
$conn,
|
||||
'commit.authorPHID IN (%Ls)',
|
||||
$this->authorPHIDs);
|
||||
$author_phids);
|
||||
}
|
||||
|
||||
if ($this->epochMin !== null) {
|
||||
|
@ -934,5 +956,20 @@ final class DiffusionCommitQuery
|
|||
) + $parent;
|
||||
}
|
||||
|
||||
private function selectPossibleAuthors(array $phids) {
|
||||
// See PHI1057. Select PHIDs which might possibly be commit authors from
|
||||
// a larger list of PHIDs. This primarily filters out packages and projects
|
||||
// from "Responsible Users: ..." queries. Our goal in performing this
|
||||
// filtering is to improve the performance of the final query.
|
||||
|
||||
foreach ($phids as $key => $phid) {
|
||||
if (phid_get_type($phid) !== PhabricatorPeopleUserPHIDType::TYPECONST) {
|
||||
unset($phids[$key]);
|
||||
}
|
||||
}
|
||||
|
||||
return $phids;
|
||||
}
|
||||
|
||||
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue