diff --git a/src/applications/differential/search/DifferentialRevisionFerretEngine.php b/src/applications/differential/search/DifferentialRevisionFerretEngine.php --- a/src/applications/differential/search/DifferentialRevisionFerretEngine.php +++ b/src/applications/differential/search/DifferentialRevisionFerretEngine.php @@ -15,7 +15,7 @@ return new DifferentialRevisionFerretField(); } - protected function newSearchEngine() { + public function newSearchEngine() { return new DifferentialRevisionSearchEngine(); } diff --git a/src/applications/maniphest/search/ManiphestTaskFerretEngine.php b/src/applications/maniphest/search/ManiphestTaskFerretEngine.php --- a/src/applications/maniphest/search/ManiphestTaskFerretEngine.php +++ b/src/applications/maniphest/search/ManiphestTaskFerretEngine.php @@ -15,7 +15,7 @@ return new ManiphestTaskFerretField(); } - protected function newSearchEngine() { + public function newSearchEngine() { return new ManiphestTaskSearchEngine(); } diff --git a/src/applications/search/engineextension/PhabricatorFerretFulltextEngineExtension.php b/src/applications/search/engineextension/PhabricatorFerretFulltextEngineExtension.php --- a/src/applications/search/engineextension/PhabricatorFerretFulltextEngineExtension.php +++ b/src/applications/search/engineextension/PhabricatorFerretFulltextEngineExtension.php @@ -23,11 +23,37 @@ $phid = $document->getPHID(); $engine = $object->newFerretEngine(); + $is_closed = 0; + $author_phid = null; + $owner_phid = null; + foreach ($document->getRelationshipData() as $relationship) { + list($related_type, $related_phid) = $relationship; + switch ($related_type) { + case PhabricatorSearchRelationship::RELATIONSHIP_OPEN: + $is_closed = 0; + break; + case PhabricatorSearchRelationship::RELATIONSHIP_CLOSED: + $is_closed = 1; + break; + case PhabricatorSearchRelationship::RELATIONSHIP_OWNER: + $owner_phid = $related_phid; + break; + case PhabricatorSearchRelationship::RELATIONSHIP_UNOWNED: + $owner_phid = null; + break; + case PhabricatorSearchRelationship::RELATIONSHIP_AUTHOR: + $author_phid = $related_phid; + break; + } + } + $ferret_document = $engine->newDocumentObject() ->setObjectPHID($phid) - ->setIsClosed(0) - ->setEpochCreated(0) - ->setEpochModified(0); + ->setIsClosed($is_closed) + ->setEpochCreated($document->getDocumentCreated()) + ->setEpochModified($document->getDocumentModified()) + ->setAuthorPHID($author_phid) + ->setOwnerPHID($owner_phid); $stemmer = $engine->newStemmer(); diff --git a/src/applications/search/ferret/PhabricatorFerretDocument.php b/src/applications/search/ferret/PhabricatorFerretDocument.php --- a/src/applications/search/ferret/PhabricatorFerretDocument.php +++ b/src/applications/search/ferret/PhabricatorFerretDocument.php @@ -27,6 +27,18 @@ 'columns' => array('objectPHID'), 'unique' => true, ), + 'key_author' => array( + 'columns' => array('authorPHID'), + ), + 'key_owner' => array( + 'columns' => array('ownerPHID'), + ), + 'key_created' => array( + 'columns' => array('epochCreated'), + ), + 'key_modified' => array( + 'columns' => array('epochModified'), + ), ), ) + parent::getConfiguration(); } diff --git a/src/applications/search/ferret/PhabricatorFerretEngine.php b/src/applications/search/ferret/PhabricatorFerretEngine.php --- a/src/applications/search/ferret/PhabricatorFerretEngine.php +++ b/src/applications/search/ferret/PhabricatorFerretEngine.php @@ -5,7 +5,7 @@ abstract public function newNgramsObject(); abstract public function newDocumentObject(); abstract public function newFieldObject(); - abstract protected function newSearchEngine(); + abstract public function newSearchEngine(); public function getDefaultFunctionKey() { return 'all'; @@ -70,60 +70,6 @@ return new PhutilSearchStemmer(); } - public function newConfiguredFulltextQuery( - $object, - PhabricatorSavedQuery $query, - PhabricatorUser $viewer) { - - $local_query = new PhabricatorSavedQuery(); - $local_query->setParameter('query', $query->getParameter('query')); - - // TODO: Modularize this piece. - $project_phids = $query->getParameter('projectPHIDs'); - if ($project_phids) { - $local_query->setParameter('projectPHIDs', $project_phids); - } - - $subscriber_phids = $query->getParameter('subscriberPHIDs'); - if ($subscriber_phids) { - $local_query->setParameter('subscriberPHIDs', $subscriber_phids); - } - - $author_phids = $query->getParameter('authorPHIDs'); - if ($author_phids) { - $local_query->setParameter('authorPHIDs', $author_phids); - } - - $owner_phids = $query->getParameter('ownerPHIDs'); - if ($owner_phids) { - $local_query->setParameter('ownerPHIDs', $owner_phids); - } - - $rel_open = PhabricatorSearchRelationship::RELATIONSHIP_OPEN; - $rel_closed = PhabricatorSearchRelationship::RELATIONSHIP_CLOSED; - - $statuses = $query->getParameter('statuses'); - if ($statuses) { - $statuses = array_fuse($statuses); - if (count($statuses) == 1) { - if (isset($statuses[$rel_open])) { - $local_query->setParameter('statuses', array('open()')); - } - if (isset($statuses[$rel_closed])) { - $local_query->setParameter('statuses', array('closed()')); - } - } - } - - $search_engine = $this->newSearchEngine() - ->setViewer($viewer); - - $engine_query = $search_engine->buildQueryFromSavedQuery($local_query) - ->setViewer($viewer); - - return $engine_query; - } - public function tokenizeString($value) { $value = trim($value, ' '); $value = preg_split('/ +/', $value); diff --git a/src/applications/search/fulltextstorage/PhabricatorFerretFulltextStorageEngine.php b/src/applications/search/fulltextstorage/PhabricatorFerretFulltextStorageEngine.php --- a/src/applications/search/fulltextstorage/PhabricatorFerretFulltextStorageEngine.php +++ b/src/applications/search/fulltextstorage/PhabricatorFerretFulltextStorageEngine.php @@ -47,6 +47,8 @@ $offset = (int)$query->getParameter('offset', 0); $limit = (int)$query->getParameter('limit', 25); + // NOTE: For now, it's okay to query with the omnipotent viewer here + // because we're just returning PHIDs which we'll filter later. $viewer = PhabricatorUser::getOmnipotentUser(); $type_results = array(); @@ -54,19 +56,31 @@ $engine = $spec['engine']; $object = $spec['object']; - // NOTE: For now, it's okay to query with the omnipotent viewer here - // because we're just returning PHIDs which we'll filter later. + $local_query = new PhabricatorSavedQuery(); + $local_query->setParameter('query', $query->getParameter('query')); - $type_query = $engine->newConfiguredFulltextQuery( - $object, - $query, - $viewer); + $project_phids = $query->getParameter('projectPHIDs'); + if ($project_phids) { + $local_query->setParameter('projectPHIDs', $project_phids); + } - $type_query + $subscriber_phids = $query->getParameter('subscriberPHIDs'); + if ($subscriber_phids) { + $local_query->setParameter('subscriberPHIDs', $subscriber_phids); + } + + $search_engine = $engine->newSearchEngine() + ->setViewer($viewer); + + $engine_query = $search_engine->buildQueryFromSavedQuery($local_query) + ->setViewer($viewer); + + $engine_query + ->withFerretQuery($engine, $query) ->setOrder('relevance') ->setLimit($offset + $limit); - $results = $type_query->execute(); + $results = $engine_query->execute(); $results = mpull($results, null, 'getPHID'); $type_results[$type] = $results; } diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php --- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php @@ -28,8 +28,9 @@ private $spaceIsArchived; private $ngrams = array(); private $ferretEngine; - private $ferretTokens; - private $ferretTables; + private $ferretTokens = array(); + private $ferretTables = array(); + private $ferretQuery; protected function getPageCursors(array $page) { return array( @@ -772,7 +773,7 @@ if ($this->supportsFerretEngine()) { $orders['relevance'] = array( - 'vector' => array('rank', 'id'), + 'vector' => array('rank', 'fulltext-modified', 'id'), 'name' => pht('Relevence'), ); } @@ -975,6 +976,16 @@ 'column' => '_ft_rank', 'type' => 'int', ); + $columns['fulltext-created'] = array( + 'table' => 'ft_doc', + 'column' => 'epochCreated', + 'type' => 'int', + ); + $columns['fulltext-modified'] = array( + 'table' => 'ft_doc', + 'column' => 'epochModified', + 'type' => 'int', + ); } $cache->setKey($cache_key, $columns); @@ -1406,6 +1417,22 @@ return ($object instanceof PhabricatorFerretInterface); } + public function withFerretQuery( + PhabricatorFerretEngine $engine, + PhabricatorSavedQuery $query) { + + if (!$this->supportsFerretEngine()) { + throw new Exception( + pht( + 'Query ("%s") does not support the Ferret fulltext engine.', + get_class($this))); + } + + $this->ferretEngine = $engine; + $this->ferretQuery = $query; + + return $this; + } public function withFerretConstraint( PhabricatorFerretEngine $engine, @@ -1538,10 +1565,10 @@ $table_alias, $stem_value); } - - $parts[] = '0'; } + $parts[] = '0'; + $select[] = qsprintf( $conn, '%Q _ft_rank', @@ -1646,7 +1673,7 @@ $joins = array(); $joins[] = qsprintf( $conn, - 'JOIN %T ftdoc ON ftdoc.objectPHID = %Q', + 'JOIN %T ft_doc ON ft_doc.objectPHID = %Q', $document_table->getTableName(), $phid_column); @@ -1655,11 +1682,11 @@ $table = $spec['table']; $ngram = $spec['ngram']; - $alias = 'ft'.$idx++; + $alias = 'ftngram_'.$idx++; $joins[] = qsprintf( $conn, - 'JOIN %T %T ON %T.documentID = ftdoc.id AND %T.ngram = %s', + 'JOIN %T %T ON %T.documentID = ft_doc.id AND %T.ngram = %s', $table, $alias, $alias, @@ -1672,7 +1699,7 @@ $joins[] = qsprintf( $conn, - 'JOIN %T %T ON ftdoc.id = %T.documentID + 'JOIN %T %T ON ft_doc.id = %T.documentID AND %T.fieldKey = %s', $field_table->getTableName(), $alias, @@ -1801,6 +1828,75 @@ } } + if ($this->ferretQuery) { + $query = $this->ferretQuery; + + $author_phids = $query->getParameter('authorPHIDs'); + if ($author_phids) { + $where[] = qsprintf( + $conn, + 'ft_doc.authorPHID IN (%Ls)', + $author_phids); + } + + $with_unowned = $query->getParameter('withUnowned'); + $with_any = $query->getParameter('withAnyOwner'); + + if ($with_any && $with_unowned) { + throw new PhabricatorEmptyQueryException( + pht( + 'This query matches only unowned documents owned by anyone, '. + 'which is impossible.')); + } + + $owner_phids = $query->getParameter('ownerPHIDs'); + if ($owner_phids && !$with_any) { + if ($with_unowned) { + $where[] = qsprintf( + $conn, + 'ft_doc.ownerPHID IN (%Ls) OR ft_doc.ownerPHID IS NULL', + $owner_phids); + } else { + $where[] = qsprintf( + $conn, + 'ft_doc.ownerPHID IN (%Ls)', + $owner_phids); + } + } else if ($with_unowned) { + $where[] = qsprintf( + $conn, + 'ft_doc.ownerPHID IS NULL'); + } + + if ($with_any) { + $where[] = qsprintf( + $conn, + 'ft_doc.ownerPHID IS NOT NULL'); + } + + $rel_open = PhabricatorSearchRelationship::RELATIONSHIP_OPEN; + + $statuses = $query->getParameter('statuses'); + $is_closed = null; + if ($statuses) { + $statuses = array_fuse($statuses); + if (count($statuses) == 1) { + if (isset($statuses[$rel_open])) { + $is_closed = 0; + } else { + $is_closed = 1; + } + } + } + + if ($is_closed !== null) { + $where[] = qsprintf( + $conn, + 'ft_doc.isClosed = %d', + $is_closed); + } + } + return $where; }