Manybubbles has uploaded a new change for review.
https://gerrit.wikimedia.org/r/126975
Change subject: Don't add a filter if not needed
......................................................................
Don't add a filter if not needed
This skips adding the filter if searching a single index containings exactly
the same number of namespaces that the user is searching. This will provide
a marginal improvement on the most common case: searching the main namespace.
It might provide other improvements along the way, but the main namespace
one is the most important because it is so much more common.
Also remove a flaky test case the complains for no reason about almost every
commit even though the feature works. Useless.
Change-Id: I7a8be3bb2dae9753ddff6bf4b7c15fbc2596cd20
---
M includes/Connection.php
M includes/Searcher.php
M tests/browser/features/prefer_recent.feature
A tests/unit/ConnectionTest.php
4 files changed, 103 insertions(+), 5 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/CirrusSearch
refs/changes/75/126975/1
diff --git a/includes/Connection.php b/includes/Connection.php
index edb84ef..389b358 100644
--- a/includes/Connection.php
+++ b/includes/Connection.php
@@ -95,4 +95,26 @@
return MWNamespace::isContent( $namespace ) ?
self::CONTENT_INDEX_TYPE : self::GENERAL_INDEX_TYPE;
}
+
+ /**
+ * Is there more then one namespace in the provided index type?
+ * @var string $indexType an index type
+ * @return false|integer false if the number of indexes is unknown, an
integer if it is known
+ */
+ public static function namespacesInIndexType( $indexType ) {
+ global $wgCirrusSearchNamespaceMappings,
+ $wgContentNamespaces;
+
+ if ( $indexType === self::GENERAL_INDEX_TYPE ) {
+ return false;
+ }
+
+ $count = count( array_keys( $wgCirrusSearchNamespaceMappings,
$indexType ) );
+ if ( $indexType === self::CONTENT_INDEX_TYPE ) {
+ // The content namespace includes everything set in the
mappings to content (count right now)
+ // Plus everything in wgContentNamespaces that isn't
already in namespace mappings
+ $count += count( array_diff( $wgContentNamespaces,
array_keys( $wgCirrusSearchNamespaceMappings ) ) );
+ }
+ return $count;
+ }
}
diff --git a/includes/Searcher.php b/includes/Searcher.php
index b32da14..5724ea6 100644
--- a/includes/Searcher.php
+++ b/includes/Searcher.php
@@ -677,11 +677,12 @@
$query->setParam( 'fields', $this->resultsType->getFields() );
$extraIndexes = array();
+ $indexType = $this->pickIndexTypeFromNamespaces();
if ( $this->namespaces ) {
- if ( count( $this->namespaces ) < count(
MWNamespace::getValidNamespaces() ) ) {
+ $extraIndexes = $this->getAndFilterExtraIndexes();
+ if ( $this->needNsFilter( $extraIndexes, $indexType ) )
{
$this->filters[] = new \Elastica\Filter\Terms(
'namespace', $this->namespaces );
}
- $extraIndexes = $this->getAndFilterExtraIndexes();
}
// Wrap $this->query in a filtered query if there are filters.
@@ -779,8 +780,7 @@
// Setup the search
- $pageType = Connection::getPageType( $this->indexBaseName,
- $this->pickIndexTypeFromNamespaces() );
+ $pageType = Connection::getPageType( $this->indexBaseName,
$indexType );
$search = $pageType->createSearch( $query, $queryOptions );
foreach ( $extraIndexes as $i ) {
$search->addIndex( $i );
@@ -823,6 +823,26 @@
return $result;
}
+ private function needNsFilter( $extraIndexes, $indexType ) {
+ if ( $extraIndexes ) {
+ // We're reaching into another wiki's indexes and we
don't know what is there so be defensive.
+ return true;
+ }
+ $nsCount = count( $this->namespaces );
+ $validNsCount = count( MWNamespace::getValidNamespaces() );
+ if ( $nsCount === $validNsCount ) {
+ // We're only on our wiki and we're searching
_everything_.
+ return false;
+ }
+ if ( !$indexType ) {
+ // We're searching less than everything but we're going
across indexes. Back to the defensive.
+ return true;
+ }
+ $namespacesInIndexType = Connection::namespacesInIndexType(
$indexType );
+ wfDebugLog( 'CirrusSearch' , "ASDFADSFASDF $nsCount ===
$namespacesInIndexType");
+ return $nsCount !== $namespacesInIndexType;
+ }
+
private function buildSearchTextQuery( $fields, $nearMatchFields,
$queryString ) {
// Build one query for the full text fields and one for the
near match fields so that
// the near match analyzer doesn't confuse the full text
analyzers.
diff --git a/tests/browser/features/prefer_recent.feature
b/tests/browser/features/prefer_recent.feature
index cbabb69..26b7d36 100644
--- a/tests/browser/features/prefer_recent.feature
+++ b/tests/browser/features/prefer_recent.feature
@@ -17,7 +17,6 @@
| 1,.0001 |
| .99,.0001 |
| .99,.001 |
- | .8,.0001 |
@prefer_recent
Scenario Outline: You can specify prefer-recent: in such a way that being
super recent isn't enough
diff --git a/tests/unit/ConnectionTest.php b/tests/unit/ConnectionTest.php
new file mode 100644
index 0000000..df9759f
--- /dev/null
+++ b/tests/unit/ConnectionTest.php
@@ -0,0 +1,57 @@
+<?php
+
+namespace CirrusSearch;
+
+use \MediaWikiTestCase;
+
+/**
+ * Make sure cirrus doens't break any hooks.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+class ConnectionTest extends MediaWikiTestCase {
+ /**
+ * @dataProvider provideNamespacesInIndexType
+ */
+ public function testNamespacesInIndexType( $contentNamespaces,
$namespaceMappings, $indexType, $expected ) {
+ global $wgContentNamespaces,
+ $wgCirrusSearchNamespaceMappings;
+
+ $wgContentNamespaces = $contentNamespaces;
+ $wgCirrusSearchNamespaceMappings = $namespaceMappings;
+ $this->assertEquals( $expected,
Connection::namespacesInIndexType( $indexType ) );
+ }
+
+ public static function provideNamespacesInIndexType() {
+ return array(
+ // Standard:
+ array( array( NS_MAIN ), array(), 'content', 1 ),
+ array( array( NS_MAIN ), array(), 'general', false ),
+
+ // Commons:
+ array( array( NS_MAIN ), array( NS_FILE => 'file' ),
'file', 1 ),
+
+ // Funky:
+ array( array( NS_MAIN ), array( NS_FILE => 'file',
NS_FILE_TALK => 'file' ), 'file', 2 ),
+ array( array( NS_MAIN ), array( NS_FILE => 'file',
NS_FILE_TALK => 'file' ), 'conent', false ),
+ array( array( NS_MAIN, NS_FILE ), array(), 'content', 2
),
+ array( array( NS_MAIN, NS_FILE ), array( NS_FILE =>
'file' ), 'file', 1 ),
+ array( array( NS_MAIN, NS_FILE ), array( NS_FILE =>
'file' ), 'content', 1 ),
+ array( array( NS_MAIN, NS_FILE, NS_FILE_TALK ), array(
NS_FILE => 'file' ), 'content', 2 ),
+ array( array( NS_MAIN, NS_FILE, NS_FILE_TALK ),
array(), 'content', 3 ),
+ );
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/126975
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7a8be3bb2dae9753ddff6bf4b7c15fbc2596cd20
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/CirrusSearch
Gerrit-Branch: master
Gerrit-Owner: Manybubbles <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
