From 00b1c4190c412c365254977e293f5cbbfbc84dd9 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 30 Mar 2019 14:33:19 -0700 Subject: [PATCH] Correct some straggling Ferret/Cursor interactions Summary: See PHI1182. Ref T13266. The recent fixes didn't quite cover the case where you have a query, but order by something other than relevance, and page forward. Refine the tests around building/selecting these columns and paging values a little bit to be more specific about what they care about. Test Plan: Executed queries, then went to "Next Page", for: - query text, non-relevance order. - query text, relevance order. - no query text, non-relevance order. - no query text, relevance order. Also, made an API call similar to the one in PHI1182. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13266 Differential Revision: https://secure.phabricator.com/D20354 --- ...PhabricatorCursorPagedPolicyAwareQuery.php | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php index 378c282e14..f5586fd90f 100644 --- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php @@ -212,7 +212,7 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery } if ($this->supportsFerretEngine()) { - if ($this->getFerretTokens()) { + if ($this->hasFerretOrder()) { $map += array( 'rank' => $cursor->getRawRowProperty(self::FULLTEXT_RANK), @@ -1840,15 +1840,16 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery return $select; } - $vector = $this->getOrderVector(); - if (!$vector->containsKey('rank')) { - // We only need to SELECT the virtual "_ft_rank" column if we're + if (!$this->hasFerretOrder()) { + // We only need to SELECT the virtual rank/relevance columns if we're // actually sorting the results by rank. return $select; } if (!$this->ferretEngine) { $select[] = qsprintf($conn, '0 AS %T', self::FULLTEXT_RANK); + $select[] = qsprintf($conn, '0 AS %T', self::FULLTEXT_CREATED); + $select[] = qsprintf($conn, '0 AS %T', self::FULLTEXT_MODIFIED); return $select; } @@ -3152,4 +3153,22 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery } } + private function hasFerretOrder() { + $vector = $this->getOrderVector(); + + if ($vector->containsKey('rank')) { + return true; + } + + if ($vector->containsKey('fulltext-created')) { + return true; + } + + if ($vector->containsKey('fulltext-modified')) { + return true; + } + + return false; + } + }