mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-23 05:50:55 +01:00
Rewrite CommitQuery to use UNION for performance
Summary: Ref T12680. See PHI167. See that task for discussion. Rewrite `DiffusionCommitQuery` to work more like `DifferentialRevisionQuery`, and use a UNION to find "all revisions you need to audit OR respond to". I tried to get this working a little more cleanly than RevisionQuery does, and can probably simplify that now. Test Plan: Poked at the UI locally without hitting any apparent issues, but my local data is pretty garbage at this point. I'll take a look at how the query plans work on `secure`. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T12680 Differential Revision: https://secure.phabricator.com/D18722
This commit is contained in:
parent
63e6b2553e
commit
157f47cd14
2 changed files with 85 additions and 33 deletions
|
@ -177,7 +177,63 @@ final class DiffusionCommitQuery
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function loadPage() {
|
protected function loadPage() {
|
||||||
return $this->loadStandardPage($this->newResultObject());
|
$table = $this->newResultObject();
|
||||||
|
$conn = $table->establishConnection('r');
|
||||||
|
|
||||||
|
$subqueries = array();
|
||||||
|
if ($this->responsiblePHIDs) {
|
||||||
|
$base_authors = $this->authorPHIDs;
|
||||||
|
$base_auditors = $this->auditorPHIDs;
|
||||||
|
|
||||||
|
$responsible_phids = $this->responsiblePHIDs;
|
||||||
|
if ($base_authors) {
|
||||||
|
$all_authors = array_merge($base_authors, $responsible_phids);
|
||||||
|
} else {
|
||||||
|
$all_authors = $responsible_phids;
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($base_auditors) {
|
||||||
|
$all_auditors = array_merge($base_auditors, $responsible_phids);
|
||||||
|
} else {
|
||||||
|
$all_auditors = $responsible_phids;
|
||||||
|
}
|
||||||
|
|
||||||
|
$this->authorPHIDs = $all_authors;
|
||||||
|
$this->auditorPHIDs = $base_auditors;
|
||||||
|
$subqueries[] = $this->buildStandardPageQuery(
|
||||||
|
$conn,
|
||||||
|
$table->getTableName());
|
||||||
|
|
||||||
|
$this->authorPHIDs = $base_authors;
|
||||||
|
$this->auditorPHIDs = $all_auditors;
|
||||||
|
$subqueries[] = $this->buildStandardPageQuery(
|
||||||
|
$conn,
|
||||||
|
$table->getTableName());
|
||||||
|
} else {
|
||||||
|
$subqueries[] = $this->buildStandardPageQuery(
|
||||||
|
$conn,
|
||||||
|
$table->getTableName());
|
||||||
|
}
|
||||||
|
|
||||||
|
if (count($subqueries) > 1) {
|
||||||
|
foreach ($subqueries as $key => $subquery) {
|
||||||
|
$subqueries[$key] = '('.$subquery.')';
|
||||||
|
}
|
||||||
|
|
||||||
|
$query = qsprintf(
|
||||||
|
$conn,
|
||||||
|
'%Q %Q %Q',
|
||||||
|
implode(' UNION DISTINCT ', $subqueries),
|
||||||
|
$this->buildOrderClause($conn, true),
|
||||||
|
$this->buildLimitClause($conn));
|
||||||
|
} else {
|
||||||
|
$query = head($subqueries);
|
||||||
|
}
|
||||||
|
|
||||||
|
$rows = queryfx_all($conn, '%Q', $query);
|
||||||
|
$rows = $this->didLoadRawRows($rows);
|
||||||
|
|
||||||
|
return $table->loadAllFromArray($rows);
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function willFilterPage(array $commits) {
|
protected function willFilterPage(array $commits) {
|
||||||
|
@ -487,18 +543,10 @@ final class DiffusionCommitQuery
|
||||||
$this->auditorPHIDs);
|
$this->auditorPHIDs);
|
||||||
}
|
}
|
||||||
|
|
||||||
if ($this->responsiblePHIDs !== null) {
|
|
||||||
$where[] = qsprintf(
|
|
||||||
$conn,
|
|
||||||
'(audit.auditorPHID IN (%Ls) OR commit.authorPHID IN (%Ls))',
|
|
||||||
$this->responsiblePHIDs,
|
|
||||||
$this->responsiblePHIDs);
|
|
||||||
}
|
|
||||||
|
|
||||||
if ($this->statuses !== null) {
|
if ($this->statuses !== null) {
|
||||||
$where[] = qsprintf(
|
$where[] = qsprintf(
|
||||||
$conn,
|
$conn,
|
||||||
'commit.auditStatus IN (%Ls)',
|
'commit.auditStatus IN (%Ld)',
|
||||||
$this->statuses);
|
$this->statuses);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -541,10 +589,6 @@ final class DiffusionCommitQuery
|
||||||
return ($this->auditIDs || $this->auditorPHIDs);
|
return ($this->auditIDs || $this->auditorPHIDs);
|
||||||
}
|
}
|
||||||
|
|
||||||
private function shouldJoinAudit() {
|
|
||||||
return (bool)$this->responsiblePHIDs;
|
|
||||||
}
|
|
||||||
|
|
||||||
private function shouldJoinOwners() {
|
private function shouldJoinOwners() {
|
||||||
return (bool)$this->packagePHIDs;
|
return (bool)$this->packagePHIDs;
|
||||||
}
|
}
|
||||||
|
@ -560,13 +604,6 @@ final class DiffusionCommitQuery
|
||||||
$audit_request->getTableName());
|
$audit_request->getTableName());
|
||||||
}
|
}
|
||||||
|
|
||||||
if ($this->shouldJoinAudit()) {
|
|
||||||
$join[] = qsprintf(
|
|
||||||
$conn,
|
|
||||||
'LEFT JOIN %T audit ON commit.phid = audit.commitPHID',
|
|
||||||
$audit_request->getTableName());
|
|
||||||
}
|
|
||||||
|
|
||||||
if ($this->shouldJoinOwners()) {
|
if ($this->shouldJoinOwners()) {
|
||||||
$join[] = qsprintf(
|
$join[] = qsprintf(
|
||||||
$conn,
|
$conn,
|
||||||
|
@ -584,10 +621,6 @@ final class DiffusionCommitQuery
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
if ($this->shouldJoinAudit()) {
|
|
||||||
return true;
|
|
||||||
}
|
|
||||||
|
|
||||||
if ($this->shouldJoinOwners()) {
|
if ($this->shouldJoinOwners()) {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
|
@ -111,7 +111,19 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
|
||||||
AphrontDatabaseConnection $conn,
|
AphrontDatabaseConnection $conn,
|
||||||
$table_name) {
|
$table_name) {
|
||||||
|
|
||||||
$rows = queryfx_all(
|
$query = $this->buildStandardPageQuery($conn, $table_name);
|
||||||
|
|
||||||
|
$rows = queryfx_all($conn, '%Q', $query);
|
||||||
|
$rows = $this->didLoadRawRows($rows);
|
||||||
|
|
||||||
|
return $rows;
|
||||||
|
}
|
||||||
|
|
||||||
|
protected function buildStandardPageQuery(
|
||||||
|
AphrontDatabaseConnection $conn,
|
||||||
|
$table_name) {
|
||||||
|
|
||||||
|
return qsprintf(
|
||||||
$conn,
|
$conn,
|
||||||
'%Q FROM %T %Q %Q %Q %Q %Q %Q %Q',
|
'%Q FROM %T %Q %Q %Q %Q %Q %Q %Q',
|
||||||
$this->buildSelectClause($conn),
|
$this->buildSelectClause($conn),
|
||||||
|
@ -123,10 +135,6 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
|
||||||
$this->buildHavingClause($conn),
|
$this->buildHavingClause($conn),
|
||||||
$this->buildOrderClause($conn),
|
$this->buildOrderClause($conn),
|
||||||
$this->buildLimitClause($conn));
|
$this->buildLimitClause($conn));
|
||||||
|
|
||||||
$rows = $this->didLoadRawRows($rows);
|
|
||||||
|
|
||||||
return $rows;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function didLoadRawRows(array $rows) {
|
protected function didLoadRawRows(array $rows) {
|
||||||
|
@ -1032,7 +1040,10 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
|
||||||
/**
|
/**
|
||||||
* @task order
|
* @task order
|
||||||
*/
|
*/
|
||||||
final protected function buildOrderClause(AphrontDatabaseConnection $conn) {
|
final protected function buildOrderClause(
|
||||||
|
AphrontDatabaseConnection $conn,
|
||||||
|
$for_union = false) {
|
||||||
|
|
||||||
$orderable = $this->getOrderableColumns();
|
$orderable = $this->getOrderableColumns();
|
||||||
$vector = $this->getOrderVector();
|
$vector = $this->getOrderVector();
|
||||||
|
|
||||||
|
@ -1045,7 +1056,7 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
|
||||||
$parts[] = $part;
|
$parts[] = $part;
|
||||||
}
|
}
|
||||||
|
|
||||||
return $this->formatOrderClause($conn, $parts);
|
return $this->formatOrderClause($conn, $parts, $for_union);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@ -1054,7 +1065,8 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
|
||||||
*/
|
*/
|
||||||
protected function formatOrderClause(
|
protected function formatOrderClause(
|
||||||
AphrontDatabaseConnection $conn,
|
AphrontDatabaseConnection $conn,
|
||||||
array $parts) {
|
array $parts,
|
||||||
|
$for_union = false) {
|
||||||
|
|
||||||
$is_query_reversed = false;
|
$is_query_reversed = false;
|
||||||
if ($this->getBeforeID()) {
|
if ($this->getBeforeID()) {
|
||||||
|
@ -1075,6 +1087,13 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
|
||||||
}
|
}
|
||||||
|
|
||||||
$table = idx($part, 'table');
|
$table = idx($part, 'table');
|
||||||
|
|
||||||
|
// When we're building an ORDER BY clause for a sequence of UNION
|
||||||
|
// statements, we can't refer to tables from the subqueries.
|
||||||
|
if ($for_union) {
|
||||||
|
$table = null;
|
||||||
|
}
|
||||||
|
|
||||||
$column = $part['column'];
|
$column = $part['column'];
|
||||||
|
|
||||||
if ($table !== null) {
|
if ($table !== null) {
|
||||||
|
|
Loading…
Reference in a new issue