From 2a83df578604cf5229ff95e8108985c2e0b0e97c Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 15 Sep 2020 17:13:21 -0700 Subject: [PATCH] Fix an issue where a GROUP BY was missing when a query matched a revision using multiple hashes Summary: Ref T13581. If you query for revisions by hash and provide multiple hashes (A, B) which match a single revision (e.g., older and newer diffs for that revision), the query omits a GROUP BY clause but should contain one. Add a GROUP BY clause in this case. Test Plan: With a working copy that has multiple hashes corresponding to a single revision, ran `arc branches` before and after the change. Before, got this error: ``` [2020-09-15 17:02:07] EXCEPTION: (ConduitClientException) ERR-CONDUIT-CORE: Rows passed to "loadAllFromArray(...)" include two or more rows with the same ID ("130"). Rows must have unique IDs. An underlying query may be missing a GROUP BY. at [/src/conduit/ConduitFuture.php:65] ``` After, clean execution. Maniphest Tasks: T13581 Differential Revision: https://secure.phabricator.com/D21462 --- .../query/DifferentialRevisionQuery.php | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/applications/differential/query/DifferentialRevisionQuery.php b/src/applications/differential/query/DifferentialRevisionQuery.php index 84f9f07f61..0f6d5d944b 100644 --- a/src/applications/differential/query/DifferentialRevisionQuery.php +++ b/src/applications/differential/query/DifferentialRevisionQuery.php @@ -778,12 +778,19 @@ final class DifferentialRevisionQuery */ protected function shouldGroupQueryResultRows() { - $join_triggers = array_merge( - $this->pathIDs, - $this->ccs, - $this->reviewers); + if (count($this->pathIDs) > 1) { + return true; + } - if (count($join_triggers) > 1) { + if (count($this->ccs) > 1) { + return true; + } + + if (count($this->reviewers) > 1) { + return true; + } + + if (count($this->commitHashes) > 1) { return true; }