From 933462b4873b4830c9b057e3a15d1aa622835744 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 15 Nov 2018 05:53:34 -0800 Subject: [PATCH] Continue cleaning up queries in the wake of changes to "%Q" Summary: Depends on D19810. Ref T13217. Ref T13216. I mostly used `grep implode | grep OR` and `grep implode | grep AND` to find these -- not totally exhaustive but should be a big chunk of the callsites that are missing `%LO` / `%LA`. Test Plan: These are tricky to test exhaustively, but I made an attempt to hit most of them: - Browsed Almanac interfaces. - Created/browsed Calendar events. - Enabled/disabled/showed the lock log. - Browsed repositories. - Loaded Facts UI. - Poked at Multimeter. - Used typeahead for users and projects. - Browsed Phriction. - Ran various fulltext searches. Not sure these are reachable: - All the lint stuff might be dead/unreachable/nonfunctional? Reviewers: amckinley Reviewed By: amckinley Subscribers: yelirekim Maniphest Tasks: T13217, T13216 Differential Revision: https://secure.phabricator.com/D19814 --- .../almanac/query/AlmanacInterfaceQuery.php | 2 +- .../query/PhabricatorCalendarEventQuery.php | 4 +-- .../PhabricatorLockLogManagementWorkflow.php | 4 +-- .../diffusion/DiffusionLintSaveRunner.php | 8 ++--- .../DiffusionBrowseQueryConduitAPIMethod.php | 4 +-- .../controller/DiffusionLintController.php | 12 +++---- .../query/DiffusionCachedResolveRefsQuery.php | 4 +-- .../query/PhabricatorFactDatapointQuery.php | 4 +-- .../controller/MultimeterSampleController.php | 8 ++--- .../query/PhabricatorPackagesQuery.php | 2 +- .../people/query/PhabricatorPeopleQuery.php | 2 +- .../query/PhrequentUserTimeQuery.php | 4 +-- .../query/PhrictionDocumentQuery.php | 8 +++-- .../project/query/PhabricatorProjectQuery.php | 2 +- ...PhabricatorCursorPagedPolicyAwareQuery.php | 35 +++++++++++++------ 15 files changed, 59 insertions(+), 44 deletions(-) diff --git a/src/applications/almanac/query/AlmanacInterfaceQuery.php b/src/applications/almanac/query/AlmanacInterfaceQuery.php index bb6fc2f9d9..d5886761c1 100644 --- a/src/applications/almanac/query/AlmanacInterfaceQuery.php +++ b/src/applications/almanac/query/AlmanacInterfaceQuery.php @@ -121,7 +121,7 @@ final class AlmanacInterfaceQuery $address->getAddress(), $address->getPort()); } - $where[] = implode(' OR ', $parts); + $where[] = qsprintf($conn, '%LO', $parts); } return $where; diff --git a/src/applications/calendar/query/PhabricatorCalendarEventQuery.php b/src/applications/calendar/query/PhabricatorCalendarEventQuery.php index c35622fc0e..fc1399fdb3 100644 --- a/src/applications/calendar/query/PhabricatorCalendarEventQuery.php +++ b/src/applications/calendar/query/PhabricatorCalendarEventQuery.php @@ -444,8 +444,8 @@ final class PhabricatorCalendarEventQuery $where[] = qsprintf( $conn, - '%Q', - implode(' OR ', $sql)); + '%LO', + $sql); } if ($this->isStub !== null) { diff --git a/src/applications/daemon/management/PhabricatorLockLogManagementWorkflow.php b/src/applications/daemon/management/PhabricatorLockLogManagementWorkflow.php index 5602bda309..8bf9be9423 100644 --- a/src/applications/daemon/management/PhabricatorLockLogManagementWorkflow.php +++ b/src/applications/daemon/management/PhabricatorLockLogManagementWorkflow.php @@ -107,9 +107,9 @@ final class PhabricatorLockLogManagementWorkflow } if (!$parts) { - $constraint = '1 = 1'; + $constraint = qsprintf($conn, '1 = 1'); } else { - $constraint = '('.implode(') AND (', $parts).')'; + $constraint = qsprintf($conn, '%LA', $parts); } $logs = $table->loadAllWhere( diff --git a/src/applications/diffusion/DiffusionLintSaveRunner.php b/src/applications/diffusion/DiffusionLintSaveRunner.php index 980234f95d..a1a2fe7d32 100644 --- a/src/applications/diffusion/DiffusionLintSaveRunner.php +++ b/src/applications/diffusion/DiffusionLintSaveRunner.php @@ -229,9 +229,9 @@ final class DiffusionLintSaveRunner extends Phobject { $this->conn, 'INSERT INTO %T (branchID, path, line, code, severity, name, description) - VALUES %Q', + VALUES %LQ', PhabricatorRepository::TABLE_LINTMESSAGE, - implode(', ', $values)); + $values); } $this->conn->saveTransaction(); @@ -295,10 +295,10 @@ final class DiffusionLintSaveRunner extends Phobject { } queryfx( $this->conn, - 'UPDATE %T SET authorPHID = %s WHERE %Q', + 'UPDATE %T SET authorPHID = %s WHERE %LO', PhabricatorRepository::TABLE_LINTMESSAGE, $author, - implode(' OR ', $where)); + $where); } $this->conn->saveTransaction(); diff --git a/src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php index 2e272d69a7..fe99471b0c 100644 --- a/src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php @@ -451,13 +451,13 @@ final class DiffusionBrowseQueryConduitAPIMethod WHERE repositoryID = %d AND parentID = %d AND existed = 1 - AND (%Q) + AND (%LO) ORDER BY pathName', PhabricatorRepository::TABLE_FILESYSTEM, PhabricatorRepository::TABLE_PATH, $repository->getID(), $path_id, - implode(' OR ', $sql)); + $sql); $loadable_commits = array(); foreach ($browse as $key => $file) { diff --git a/src/applications/diffusion/controller/DiffusionLintController.php b/src/applications/diffusion/controller/DiffusionLintController.php index af3ed571ce..704435882d 100644 --- a/src/applications/diffusion/controller/DiffusionLintController.php +++ b/src/applications/diffusion/controller/DiffusionLintController.php @@ -276,13 +276,13 @@ final class DiffusionLintController extends DiffusionController { array_keys($branch), $path->getPath()); if ($path->getExcluded()) { - $where[] = 'NOT '.$condition; + $where[] = qsprintf($conn, 'NOT %Q', $condition); } else { $or[] = $condition; } } } - $where[] = '('.implode(' OR ', $or).')'; + $where[] = qsprintf($conn, '%LO', $or); } return queryfx_all( @@ -296,11 +296,11 @@ final class DiffusionLintController extends DiffusionController { COUNT(DISTINCT path) AS files, COUNT(*) AS n FROM %T - WHERE %Q + WHERE %LA GROUP BY branchID, code ORDER BY n DESC', PhabricatorRepository::TABLE_LINTMESSAGE, - implode(' AND ', $where)); + $where); } protected function buildActionView(DiffusionRequest $drequest) { @@ -526,10 +526,10 @@ final class DiffusionLintController extends DiffusionController { $conn, 'SELECT * FROM %T - WHERE %Q + WHERE %LA ORDER BY path, code, line LIMIT %d OFFSET %d', PhabricatorRepository::TABLE_LINTMESSAGE, - implode(' AND ', $where), + $where, $limit, $offset); } diff --git a/src/applications/diffusion/query/DiffusionCachedResolveRefsQuery.php b/src/applications/diffusion/query/DiffusionCachedResolveRefsQuery.php index 0217c97d58..1666056d63 100644 --- a/src/applications/diffusion/query/DiffusionCachedResolveRefsQuery.php +++ b/src/applications/diffusion/query/DiffusionCachedResolveRefsQuery.php @@ -81,10 +81,10 @@ final class DiffusionCachedResolveRefsQuery $commits = queryfx_all( $conn_r, 'SELECT commitIdentifier FROM %T - WHERE repositoryID = %s AND %Q', + WHERE repositoryID = %s AND %LO', id(new PhabricatorRepositoryCommit())->getTableName(), $repository->getID(), - implode(' OR ', $prefixes)); + $prefixes); foreach ($commits as $commit) { $hash = $commit['commitIdentifier']; diff --git a/src/applications/fact/query/PhabricatorFactDatapointQuery.php b/src/applications/fact/query/PhabricatorFactDatapointQuery.php index fee7c86d12..20945dbbd0 100644 --- a/src/applications/fact/query/PhabricatorFactDatapointQuery.php +++ b/src/applications/fact/query/PhabricatorFactDatapointQuery.php @@ -158,7 +158,7 @@ final class PhabricatorFactDatapointQuery extends Phobject { $this->dimensionMap); } - $where = '('.implode(') AND (', $where).')'; + $where = qsprintf($conn, '%LA', $where); if ($this->limit) { $limit = qsprintf( @@ -166,7 +166,7 @@ final class PhabricatorFactDatapointQuery extends Phobject { 'LIMIT %d', $this->limit); } else { - $limit = ''; + $limit = qsprintf($conn, ''); } return queryfx_all( diff --git a/src/applications/multimeter/controller/MultimeterSampleController.php b/src/applications/multimeter/controller/MultimeterSampleController.php index 0023038185..da09641d22 100644 --- a/src/applications/multimeter/controller/MultimeterSampleController.php +++ b/src/applications/multimeter/controller/MultimeterSampleController.php @@ -52,8 +52,6 @@ final class MultimeterSampleController extends MultimeterController { } } - $where = '('.implode(') AND (', $where).')'; - $data = queryfx_all( $conn, 'SELECT *, @@ -61,13 +59,13 @@ final class MultimeterSampleController extends MultimeterController { SUM(sampleRate * resourceCost) AS totalCost, SUM(sampleRate * resourceCost) / SUM(sampleRate) AS averageCost FROM %T - WHERE %Q - GROUP BY %Q + WHERE %LA + GROUP BY %LC ORDER BY totalCost DESC, MAX(id) DESC LIMIT 100', $table->getTableName(), $where, - implode(', ', array_select_keys($group_map, $group))); + array_select_keys($group_map, $group)); $this->loadDimensions($data); $phids = array(); diff --git a/src/applications/packages/query/PhabricatorPackagesQuery.php b/src/applications/packages/query/PhabricatorPackagesQuery.php index 58b90599eb..e7e882bdad 100644 --- a/src/applications/packages/query/PhabricatorPackagesQuery.php +++ b/src/applications/packages/query/PhabricatorPackagesQuery.php @@ -32,7 +32,7 @@ abstract class PhabricatorPackagesQuery throw new PhabricatorEmptyQueryException(); } - return implode(' OR ', $parts); + return qsprintf($conn, '%LO', $parts); } } diff --git a/src/applications/people/query/PhabricatorPeopleQuery.php b/src/applications/people/query/PhabricatorPeopleQuery.php index 35ff480833..131dc7c588 100644 --- a/src/applications/people/query/PhabricatorPeopleQuery.php +++ b/src/applications/people/query/PhabricatorPeopleQuery.php @@ -268,7 +268,7 @@ final class PhabricatorPeopleQuery 'user.username LIKE %>', $name_prefix); } - $where[] = '('.implode(' OR ', $parts).')'; + $where[] = qsprintf($conn, '%LO', $parts); } if ($this->emails !== null) { diff --git a/src/applications/phrequent/query/PhrequentUserTimeQuery.php b/src/applications/phrequent/query/PhrequentUserTimeQuery.php index 7fb77f0f76..cf5122c020 100644 --- a/src/applications/phrequent/query/PhrequentUserTimeQuery.php +++ b/src/applications/phrequent/query/PhrequentUserTimeQuery.php @@ -177,9 +177,9 @@ final class PhrequentUserTimeQuery $preempting_events = queryfx_all( $conn_r, - 'SELECT * FROM %T WHERE %Q ORDER BY dateStarted ASC, id ASC', + 'SELECT * FROM %T WHERE %LO ORDER BY dateStarted ASC, id ASC', $usertime->getTableName(), - implode(' OR ', $preempt)); + $preempt); $preempting_events = $usertime->loadAllFromArray($preempting_events); $preempting_events = mgroup($preempting_events, 'getUserPHID'); diff --git a/src/applications/phriction/query/PhrictionDocumentQuery.php b/src/applications/phriction/query/PhrictionDocumentQuery.php index 8a0fa325ae..5f508ad804 100644 --- a/src/applications/phriction/query/PhrictionDocumentQuery.php +++ b/src/applications/phriction/query/PhrictionDocumentQuery.php @@ -309,10 +309,14 @@ final class PhrictionDocumentQuery $max); } - $path_clauses[] = '('.implode(') AND (', $parts).')'; + if ($parts) { + $path_clauses[] = qsprintf($conn, '%LA', $parts); + } } - $where[] = '('.implode(') OR (', $path_clauses).')'; + if ($path_clauses) { + $where[] = qsprintf($conn, '%LO', $path_clauses); + } } return $where; diff --git a/src/applications/project/query/PhabricatorProjectQuery.php b/src/applications/project/query/PhabricatorProjectQuery.php index e3a02edbd0..9b051c00dd 100644 --- a/src/applications/project/query/PhabricatorProjectQuery.php +++ b/src/applications/project/query/PhabricatorProjectQuery.php @@ -514,7 +514,7 @@ final class PhabricatorProjectQuery 'name LIKE %>', $name_prefix); } - $where[] = '('.implode(' OR ', $parts).')'; + $where[] = qsprintf($conn, '%LO', $parts); } if ($this->icons !== null) { diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php index 2d91f5e608..931840e8fb 100644 --- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php @@ -1440,7 +1440,11 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery $key); } - return implode(' ', $joins); + if ($joins) { + return qsprintf($conn, '%LJ', $joins); + } else { + return qsprintf($conn, ''); + } } /** @@ -1516,7 +1520,7 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery } if ($constraint_parts) { - $where[] = '('.implode(') OR (', $constraint_parts).')'; + $where[] = qsprintf($conn, '%LO', $constraint_parts); } break; } @@ -1670,7 +1674,7 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery } if (!$this->ferretEngine) { - $select[] = '0 _ft_rank'; + $select[] = qsprintf($conn, '0 _ft_rank'); return $select; } @@ -1736,12 +1740,21 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery } } - $parts[] = '0'; + $parts[] = qsprintf($conn, '%d', 0); + + $sum = array_shift($parts); + foreach ($parts as $part) { + $sum = qsprintf( + $conn, + '%Q + %Q', + $sum, + $part); + } $select[] = qsprintf( $conn, '%Q _ft_rank', - implode(' + ', $parts)); + $sum); return $select; } @@ -2031,20 +2044,20 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery if ($is_not) { $where[] = qsprintf( $conn, - '(%Q)', - implode(' AND ', $term_constraints)); + '%LA', + $term_constraints); } else if ($is_quoted) { $where[] = qsprintf( $conn, - '(%T.rawCorpus LIKE %~ AND (%Q))', + '(%T.rawCorpus LIKE %~ AND %LO)', $table_alias, $value, - implode(' OR ', $term_constraints)); + $term_constraints); } else { $where[] = qsprintf( $conn, - '(%Q)', - implode(' OR ', $term_constraints)); + '%LO', + $term_constraints); } }