From e6f22cd42eb077ef8dd8102743c6fcba8e5e8e46 Mon Sep 17 00:00:00 2001 From: Tim Starling Date: Mon, 25 Mar 2024 16:13:49 +1100 Subject: [PATCH] Fix index usage when searching for page titles When searching for a specific page title, it's necessary to specify page_namespace, not just linter_namespace, so that the relevant index in the page table can be used. Submitting the form with an empty namespace box led to a search for namespace zero, because getCheck() returns true for an empty string. It's not easy to search for a title part in all namespaces. So drop that hidden feature and interpret a title part with a missing namespace as being a search for namespace 0. It's possible to search for a category with an empty title and zero or more namespaces. Implement the namespace filter in this case using the linter_namespace field. But ignore the namespace filter if there is no category, since there is no index on linter_namespace alone. Bug: T360865 Change-Id: I00934eaaf1a99e4098f177166b43069d33d9f137 (cherry picked from commit 4dd75df2e8dae51c62ae6082204260c95dad1616) --- includes/LintErrorsPager.php | 38 ++++++++++++++++--------- tests/phpunit/SpecialLintErrorsTest.php | 19 ++++++------- 2 files changed, 33 insertions(+), 24 deletions(-) diff --git a/includes/LintErrorsPager.php b/includes/LintErrorsPager.php index 5b3448c5..b6e3f3f8 100644 --- a/includes/LintErrorsPager.php +++ b/includes/LintErrorsPager.php @@ -114,7 +114,8 @@ class LintErrorsPager extends TablePager { private function fillQueryBuilder( SelectQueryBuilder $queryBuilder ): void { $mainConfig = MediaWikiServices::getInstance()->getMainConfig(); $queryBuilder - ->table( 'page' )->leftJoin( 'linter', null, 'page_id=linter_page' ) + ->table( 'page' ) + ->join( 'linter', null, 'page_id=linter_page' ) ->fields( LinkCache::getSelectFields() ) ->fields( [ 'page_namespace', 'page_title', @@ -127,21 +128,30 @@ class LintErrorsPager extends TablePager { $queryBuilder->where( [ 'linter_cat' => $this->categoryId ] ); } - if ( $this->namespaces ) { - $namespaceCol = $mainConfig->get( 'LinterUseNamespaceColumnStage' ) - ? "linter_namespace" : "page_namespace"; - $queryBuilder->where( [ $namespaceCol => $this->namespaces ] ); + if ( $this->title !== '' ) { + $namespaces = $this->namespaces ?: [ NS_MAIN ]; + // Specify page_namespace so that the index can be used (T360865) + $queryBuilder->where( [ 'page_namespace' => $namespaces ] ); + if ( $mainConfig->get( 'LinterUseNamespaceColumnStage' ) ) { + // Also put a condition on linter_namespace, in case the DB + // decides to put the linter table first + $queryBuilder->where( [ 'linter_namespace' => $namespaces ] ); + } + if ( $this->exactMatch ) { + $queryBuilder->where( [ + 'page_title' => $this->title + ] ); + } else { + $queryBuilder->where( $this->mDb->expr( + 'page_title', IExpression::LIKE, new LikeValue( $this->title, $this->mDb->anyString() ) + ) ); + } + } elseif ( $this->namespaces ) { + $namespaceCol = $mainConfig->get( 'LinterUseNamespaceColumnStage' ) + ? "linter_namespace" : "page_namespace"; + $queryBuilder->where( [ $namespaceCol => $this->namespaces ] ); } - if ( $this->exactMatch ) { - if ( $this->title !== '' ) { - $queryBuilder->where( [ "page_title" => $this->title ] ); - } - } else { - $queryBuilder->where( $this->mDb->expr( - 'page_title', IExpression::LIKE, new LikeValue( $this->title, $this->mDb->anyString() ) - ) ); - } if ( $mainConfig->get( 'LinterUserInterfaceTagAndTemplateStage' ) ) { if ( $this->throughTemplate !== 'all' ) { $op = ( $this->throughTemplate === 'with' ) ? '!=' : '='; diff --git a/tests/phpunit/SpecialLintErrorsTest.php b/tests/phpunit/SpecialLintErrorsTest.php index b98669ec..b1c1a59f 100644 --- a/tests/phpunit/SpecialLintErrorsTest.php +++ b/tests/phpunit/SpecialLintErrorsTest.php @@ -268,35 +268,35 @@ class SpecialLintErrorsTest extends SpecialPageTestBase { ] ]; $testConfigurations[ 3 ] = [ - 'namespaces' => [ 0, 1, 3, null ], + 'namespaces' => [ 0, 1, 3 ], 'titles' => [ 'L', 'Lint Error One', 'LintErrorTwo', 'User talk:L', 'User talk:LintErrorTwo', 'Talk:L' ], 'cases' => [ - [ 'iterations' => [ 1, 2, 3, 37, 38, 39, 43, 47 ], + [ 'iterations' => [ 1, 2, 3 ], 'message' => 'Lint Error One' ], - [ 'iterations' => [ 25, 28, 29, 31, 32, 33, 37, 40, 41, 43, 44, 45, 47 ], + [ 'iterations' => [ 25, 28, 29, 31, 32, 33 ], 'message' => 'LintErrorTwo' ], - [ 'iterations' => [ 0, 4, 5, 12, 13, 14, 15, 16, 17, 22, 23, 24, 26, 27, 30, 36, 42, 46 ], + [ 'iterations' => [ 0, 4, 5, 12, 13, 14, 15, 16, 17, 22, 23, 24, 26, 27, 30 ], 'message' => 'table_pager_empty' ], [ 'iterations' => [ 6, 7, 8, 9, 10, 11, 18, 19, 20, 21, 34, 35 ], 'message' => 'linter-namespace-mismatch' ] ] ]; $testConfigurations[ 4 ] = [ - 'namespaces' => [ 0, 3, null ], + 'namespaces' => [ 0, 3 ], 'titles' => [ ':Lint Error One' ], 'cases' => [ - [ 'iterations' => [ 0, 1, 4, 5 ], + [ 'iterations' => [ 0, 1 ], 'message' => 'Lint Error One' ], [ 'iterations' => [ 2, 3 ], 'message' => 'linter-namespace-mismatch' ] ] ]; $testConfigurations[ 5 ] = [ - 'namespaces' => [ 0, 3, null ], + 'namespaces' => [ 0, 3 ], 'titles' => [ 'FooBar:ErrorFive' ], 'cases' => [ - [ 'iterations' => [ 2, 3, 4, 5 ], + [ 'iterations' => [ 2, 3 ], 'message' => 'FooBar:ErrorFive' ], [ 'iterations' => [ 0, 1 ], 'message' => 'table_pager_empty' ], @@ -377,8 +377,7 @@ class SpecialLintErrorsTest extends SpecialPageTestBase { foreach ( $group[ 'cases' ] as $caseIndex => $case ) { $exactString = [ 'prefix', 'exact' ][ $exact ]; $message = $case[ 'message' ]; - $descriptionNamespace = $namespace === null ? 'all' : - implode( ',', (array)$namespace ); + $descriptionNamespace = implode( ',', (array)$namespace ); $description = "On group [$groupIndex], namespace [$descriptionNamespace], " . "case [$caseIndex], iteration [$testIndex] " . "for a [$exactString] match with search title [$title] and test text [$message] ";