From: Brad Jorsch Date: Fri, 6 Oct 2017 17:03:55 +0000 (-0400) Subject: Replace selectFields() methods with getQueryInfo() X-Git-Tag: 1.31.0-rc.0~1645 X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=3488f495321a01f808eebf87e90b6fb334817d81 Replace selectFields() methods with getQueryInfo() Several classes have a "selectFields()" static method to tell callers which fields to select from the database. With the recent comment table change and the upcoming actor table change, this pattern has become too simplistic as a SELECT will need to join several tables to be able to retrieve all the needed fields. Thus, we deprecate the selectFields() methods in favor of getQueryInfo() methods that return tables and join conditions in addition to the fields. Change-Id: Idcfd15568489d9f03a7ba4460e96610d33bc4089 --- diff --git a/RELEASE-NOTES-1.31 b/RELEASE-NOTES-1.31 index b5ec0d6b2b..a7edba436e 100644 --- a/RELEASE-NOTES-1.31 +++ b/RELEASE-NOTES-1.31 @@ -64,6 +64,28 @@ changes to languages because of Phabricator reports. in 1.24, were removed. * Installer::locateExecutable() and Installer::locateExecutableInDefaultPaths() were removed, use ExecutableFinder::findInDefaultPaths() instead. +* Several methods for returning lists of fields to select from the database + have been deprecated in favor of similar methods that also return the tables + to select from and the join conditions for those tables. + * Block::selectFields() → Block::getQueryInfo() + * RecentChange::selectFields() → RecentChange::getQueryInfo() + * ArchivedFile::selectFields() → ArchivedFile::getQueryInfo() + * LocalFile::selectFields() → LocalFile::getQueryInfo() + * LocalFile::getCacheFields() with a prefix no longer works + * LocalFile::getLazyCacheFields() with a prefix no longer works + * OldLocalFile::selectFields() → OldLocalFile::getQueryInfo() + * RecentChange::selectFields() → RecentChange::getQueryInfo() + * Revision::userJoinCond() → Revision::getQueryInfo( [ 'user' ] ) + * Revision::selectUserFields() → Revision::getQueryInfo( [ 'user' ] ) + * Revision::pageJoinCond() → Revision::getQueryInfo( [ 'page' ] ) + * Revision::selectPageFields() → Revision::getQueryInfo( [ 'page' ] ) + * Revision::selectTextFields() → Revision::getQueryInfo( [ 'text' ] ) + * Revision::selectFields() → Revision::getQueryInfo() + * Revision::selectArchiveFields() → Revision::getArchiveQueryInfo() + * User::selectFields() → User::getQueryInfo() + * WikiPage::selectFields() → WikiPage::getQueryInfo() +* Due to significant refactoring, method ContribsPager::getUserCond() that had + no access restriction has been removed. == Compatibility == MediaWiki 1.31 requires PHP 5.5.9 or later. There is experimental support for diff --git a/docs/hooks.txt b/docs/hooks.txt index a19e9fc0e2..effc6d9ee2 100644 --- a/docs/hooks.txt +++ b/docs/hooks.txt @@ -689,6 +689,8 @@ $destTitle: destination title (object) 'ArticlePageDataBefore': Before loading data of an article from the database. &$wikiPage: WikiPage (object) that data will be loaded &$fields: fields (array) to load from the database +&$tables: tables (array) to load from the database +&$joinConds: join conditions (array) to load from the database 'ArticlePrepareTextForEdit': Called when preparing text to be saved. $wikiPage: the WikiPage being saved diff --git a/includes/Block.php b/includes/Block.php index 8d69d9a914..d1e78bb6cf 100644 --- a/includes/Block.php +++ b/includes/Block.php @@ -183,11 +183,14 @@ class Block { */ public static function newFromID( $id ) { $dbr = wfGetDB( DB_REPLICA ); + $blockQuery = self::getQueryInfo(); $res = $dbr->selectRow( - 'ipblocks', - self::selectFields(), + $blockQuery['tables'], + $blockQuery['fields'], [ 'ipb_id' => $id ], - __METHOD__ + __METHOD__, + [], + $blockQuery['joins'] ); if ( $res ) { return self::newFromRow( $res ); @@ -199,11 +202,11 @@ class Block { /** * Return the list of ipblocks fields that should be selected to create * a new block. - * @todo Deprecate this in favor of a method that returns tables and joins - * as well, and use CommentStore::getJoin(). + * @deprecated since 1.31, use self::getQueryInfo() instead. * @return array */ public static function selectFields() { + wfDeprecated( __METHOD__, '1.31' ); return [ 'ipb_id', 'ipb_address', @@ -222,6 +225,39 @@ class Block { ] + CommentStore::newKey( 'ipb_reason' )->getFields(); } + /** + * Return the tables, fields, and join conditions to be selected to create + * a new block object. + * @since 1.31 + * @return array With three keys: + * - tables: (string[]) to include in the `$table` to `IDatabase->select()` + * - fields: (string[]) to include in the `$vars` to `IDatabase->select()` + * - joins: (array) to include in the `$join_conds` to `IDatabase->select()` + */ + public static function getQueryInfo() { + $commentQuery = CommentStore::newKey( 'ipb_reason' )->getJoin(); + return [ + 'tables' => [ 'ipblocks' ] + $commentQuery['tables'], + 'fields' => [ + 'ipb_id', + 'ipb_address', + 'ipb_by', + 'ipb_by_text', + 'ipb_timestamp', + 'ipb_auto', + 'ipb_anon_only', + 'ipb_create_account', + 'ipb_enable_autoblock', + 'ipb_expiry', + 'ipb_deleted', + 'ipb_block_email', + 'ipb_allow_usertalk', + 'ipb_parent_block_id', + ] + $commentQuery['fields'], + 'joins' => $commentQuery['joins'], + ]; + } + /** * Check if two blocks are effectively equal. Doesn't check irrelevant things like * the blocking user or the block timestamp, only things which affect the blocked user @@ -295,7 +331,10 @@ class Block { } } - $res = $db->select( 'ipblocks', self::selectFields(), $conds, __METHOD__ ); + $blockQuery = self::getQueryInfo(); + $res = $db->select( + $blockQuery['tables'], $blockQuery['fields'], $conds, __METHOD__, [], $blockQuery['joins'] + ); # This result could contain a block on the user, a block on the IP, and a russian-doll # set of rangeblocks. We want to choose the most specific one, so keep a leader board. @@ -422,7 +461,7 @@ class Block { $db = wfGetDB( DB_REPLICA ); $this->mExpiry = $db->decodeExpiry( $row->ipb_expiry ); $this->mReason = CommentStore::newKey( 'ipb_reason' ) - // Legacy because $row probably came from self::selectFields() + // Legacy because $row may have come from self::selectFields() ->getCommentLegacy( $db, $row )->text; $this->isHardblock( !$row->ipb_anon_only ); @@ -1187,14 +1226,14 @@ class Block { if ( !$isAnon ) { $conds = [ $conds, 'ipb_anon_only' => 0 ]; } - $selectFields = array_merge( - [ 'ipb_range_start', 'ipb_range_end' ], - self::selectFields() - ); - $rows = $db->select( 'ipblocks', - $selectFields, + $blockQuery = self::getQueryInfo(); + $rows = $db->select( + $blockQuery['tables'], + array_merge( [ 'ipb_range_start', 'ipb_range_end' ], $blockQuery['fields'] ), $conds, - __METHOD__ + __METHOD__, + [], + $blockQuery['joins'] ); $blocks = []; diff --git a/includes/FeedUtils.php b/includes/FeedUtils.php index b1c3ce615a..6c343ab39f 100644 --- a/includes/FeedUtils.php +++ b/includes/FeedUtils.php @@ -89,9 +89,7 @@ class FeedUtils { $timestamp, $row->rc_deleted & Revision::DELETED_COMMENT ? wfMessage( 'rev-deleted-comment' )->escaped() - : CommentStore::newKey( 'rc_comment' ) - // Legacy from RecentChange::selectFields() via ChangesListSpecialPage::doMainQuery() - ->getCommentLegacy( wfGetDB( DB_REPLICA ), $row )->text, + : CommentStore::newKey( 'rc_comment' )->getComment( $row )->text, $actiontext ); } diff --git a/includes/Revision.php b/includes/Revision.php index dd3ee782fd..f9810a0354 100644 --- a/includes/Revision.php +++ b/includes/Revision.php @@ -193,7 +193,7 @@ class Revision implements IDBAccessObject { 'page' => isset( $row->ar_page_id ) ? $row->ar_page_id : null, 'id' => isset( $row->ar_rev_id ) ? $row->ar_rev_id : null, 'comment' => CommentStore::newKey( 'ar_comment' ) - // Legacy because $row probably came from self::selectArchiveFields() + // Legacy because $row may have come from self::selectArchiveFields() ->getCommentLegacy( wfGetDB( DB_REPLICA ), $row, true )->text, 'user' => $row->ar_user, 'user_text' => $row->ar_user_text, @@ -403,22 +403,18 @@ class Revision implements IDBAccessObject { * @return stdClass */ private static function fetchFromConds( $db, $conditions, $flags = 0 ) { - $fields = array_merge( - self::selectFields(), - self::selectPageFields(), - self::selectUserFields() - ); + $revQuery = self::getQueryInfo( [ 'page', 'user' ] ); $options = []; if ( ( $flags & self::READ_LOCKING ) == self::READ_LOCKING ) { $options[] = 'FOR UPDATE'; } return $db->selectRow( - [ 'revision', 'page', 'user' ], - $fields, + $revQuery['tables'], + $revQuery['fields'], $conditions, __METHOD__, $options, - [ 'page' => self::pageJoinCond(), 'user' => self::userJoinCond() ] + $revQuery['joins'] ); } @@ -426,6 +422,7 @@ class Revision implements IDBAccessObject { * Return the value of a select() JOIN conds array for the user table. * This will get user table rows for logged-in users. * @since 1.19 + * @deprecated since 1.31, use self::getQueryInfo( [ 'user' ] ) instead. * @return array */ public static function userJoinCond() { @@ -436,6 +433,7 @@ class Revision implements IDBAccessObject { * Return the value of a select() page conds array for the page table. * This will assure that the revision(s) are not orphaned from live pages. * @since 1.19 + * @deprecated since 1.31, use self::getQueryInfo( [ 'page' ] ) instead. * @return array */ public static function pageJoinCond() { @@ -445,8 +443,7 @@ class Revision implements IDBAccessObject { /** * Return the list of revision fields that should be selected to create * a new revision. - * @todo Deprecate this in favor of a method that returns tables and joins - * as well, and use CommentStore::getJoin(). + * @deprecated since 1.31, use self::getQueryInfo() instead. * @return array */ public static function selectFields() { @@ -479,8 +476,7 @@ class Revision implements IDBAccessObject { /** * Return the list of revision fields that should be selected to create * a new revision from an archive row. - * @todo Deprecate this in favor of a method that returns tables and joins - * as well, and use CommentStore::getJoin(). + * @deprecated since 1.31, use self::getArchiveQueryInfo() instead. * @return array */ public static function selectArchiveFields() { @@ -513,6 +509,7 @@ class Revision implements IDBAccessObject { /** * Return the list of text fields that should be selected to read the * revision text + * @deprecated since 1.31, use self::getQueryInfo( [ 'text' ] ) instead. * @return array */ public static function selectTextFields() { @@ -524,6 +521,7 @@ class Revision implements IDBAccessObject { /** * Return the list of page fields that should be selected from page table + * @deprecated since 1.31, use self::getQueryInfo( [ 'page' ] ) instead. * @return array */ public static function selectPageFields() { @@ -539,12 +537,127 @@ class Revision implements IDBAccessObject { /** * Return the list of user fields that should be selected from user table + * @deprecated since 1.31, use self::getQueryInfo( [ 'user' ] ) instead. * @return array */ public static function selectUserFields() { return [ 'user_name' ]; } + /** + * Return the tables, fields, and join conditions to be selected to create + * a new revision object. + * @since 1.31 + * @param array $options Any combination of the following strings + * - 'page': Join with the page table, and select fields to identify the page + * - 'user': Join with the user table, and select the user name + * - 'text': Join with the text table, and select fields to load page text + * @return array With three keys: + * - tables: (string[]) to include in the `$table` to `IDatabase->select()` + * - fields: (string[]) to include in the `$vars` to `IDatabase->select()` + * - joins: (array) to include in the `$join_conds` to `IDatabase->select()` + */ + public static function getQueryInfo( $options = [] ) { + global $wgContentHandlerUseDB; + + $commentQuery = CommentStore::newKey( 'rev_comment' )->getJoin(); + $ret = [ + 'tables' => [ 'revision' ] + $commentQuery['tables'], + 'fields' => [ + 'rev_id', + 'rev_page', + 'rev_text_id', + 'rev_timestamp', + 'rev_user_text', + 'rev_user', + 'rev_minor_edit', + 'rev_deleted', + 'rev_len', + 'rev_parent_id', + 'rev_sha1', + ] + $commentQuery['fields'], + 'joins' => $commentQuery['joins'], + ]; + + if ( $wgContentHandlerUseDB ) { + $ret['fields'][] = 'rev_content_format'; + $ret['fields'][] = 'rev_content_model'; + } + + if ( in_array( 'page', $options, true ) ) { + $ret['tables'][] = 'page'; + $ret['fields'] = array_merge( $ret['fields'], [ + 'page_namespace', + 'page_title', + 'page_id', + 'page_latest', + 'page_is_redirect', + 'page_len', + ] ); + $ret['joins']['page'] = [ 'INNER JOIN', [ 'page_id = rev_page' ] ]; + } + + if ( in_array( 'user', $options, true ) ) { + $ret['tables'][] = 'user'; + $ret['fields'] = array_merge( $ret['fields'], [ + 'user_name', + ] ); + $ret['joins']['user'] = [ 'LEFT JOIN', [ 'rev_user != 0', 'user_id = rev_user' ] ]; + } + + if ( in_array( 'text', $options, true ) ) { + $ret['tables'][] = 'text'; + $ret['fields'] = array_merge( $ret['fields'], [ + 'old_text', + 'old_flags' + ] ); + $ret['joins']['text'] = [ 'INNER JOIN', [ 'rev_text_id=old_id' ] ]; + } + + return $ret; + } + + /** + * Return the tables, fields, and join conditions to be selected to create + * a new archived revision object. + * @since 1.31 + * @return array With three keys: + * - tables: (string[]) to include in the `$table` to `IDatabase->select()` + * - fields: (string[]) to include in the `$vars` to `IDatabase->select()` + * - joins: (array) to include in the `$join_conds` to `IDatabase->select()` + */ + public static function getArchiveQueryInfo() { + global $wgContentHandlerUseDB; + + $commentQuery = CommentStore::newKey( 'ar_comment' )->getJoin(); + $ret = [ + 'tables' => [ 'archive' ] + $commentQuery['tables'], + 'fields' => [ + 'ar_id', + 'ar_page_id', + 'ar_rev_id', + 'ar_text', + 'ar_text_id', + 'ar_timestamp', + 'ar_user_text', + 'ar_user', + 'ar_minor_edit', + 'ar_deleted', + 'ar_len', + 'ar_parent_id', + 'ar_sha1', + ] + $commentQuery['fields'], + 'joins' => $commentQuery['joins'], + ]; + + if ( $wgContentHandlerUseDB ) { + $ret['fields'][] = 'ar_content_format'; + $ret['fields'][] = 'ar_content_model'; + } + + return $ret; + } + /** * Do a batched query to get the parent revision lengths * @param IDatabase $db @@ -590,7 +703,7 @@ class Revision implements IDBAccessObject { $this->mPage = intval( $row->rev_page ); $this->mTextId = intval( $row->rev_text_id ); $this->mComment = CommentStore::newKey( 'rev_comment' ) - // Legacy because $row probably came from self::selectFields() + // Legacy because $row may have come from self::selectFields() ->getCommentLegacy( wfGetDB( DB_REPLICA ), $row, true )->text; $this->mUser = intval( $row->rev_user ); $this->mMinorEdit = intval( $row->rev_minor_edit ); @@ -837,11 +950,21 @@ class Revision implements IDBAccessObject { // rev_id is defined as NOT NULL, but this revision may not yet have been inserted. if ( $this->mId !== null ) { $dbr = wfGetLB( $this->mWiki )->getConnectionRef( DB_REPLICA, [], $this->mWiki ); + // @todo: Title::getSelectFields(), or Title::getQueryInfo(), or something like that $row = $dbr->selectRow( - [ 'page', 'revision' ], - self::selectPageFields(), - [ 'page_id=rev_page', 'rev_id' => $this->mId ], - __METHOD__ + [ 'revision', 'page' ], + [ + 'page_namespace', + 'page_title', + 'page_id', + 'page_latest', + 'page_is_redirect', + 'page_len', + ], + [ 'rev_id' => $this->mId ], + __METHOD__, + [], + [ 'page' => [ 'JOIN', 'page_id=rev_page' ] ] ); if ( $row ) { // @TODO: better foreign title handling diff --git a/includes/RevisionList.php b/includes/RevisionList.php index b0bc60a188..fa454e07e3 100644 --- a/includes/RevisionList.php +++ b/includes/RevisionList.php @@ -296,15 +296,14 @@ class RevisionList extends RevisionListBase { if ( $this->ids !== null ) { $conds['rev_id'] = array_map( 'intval', $this->ids ); } + $revQuery = Revision::getQueryInfo( [ 'page', 'user' ] ); return $db->select( - [ 'revision', 'page', 'user' ], - array_merge( Revision::selectFields(), Revision::selectUserFields() ), + $revQuery['tables'], + $revQuery['fields'], $conds, __METHOD__, [ 'ORDER BY' => 'rev_id DESC' ], - [ - 'page' => Revision::pageJoinCond(), - 'user' => Revision::userJoinCond() ] + $revQuery['joins'] ); } diff --git a/includes/Title.php b/includes/Title.php index 718239dbeb..d043b442ac 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -3628,19 +3628,20 @@ class Title implements LinkTarget { $blNamespace = "{$prefix}_namespace"; $blTitle = "{$prefix}_title"; + $pageQuery = WikiPage::getQueryInfo(); $res = $db->select( - [ $table, 'page' ], + [ $table, 'nestpage' => $pageQuery['tables'] ], array_merge( [ $blNamespace, $blTitle ], - WikiPage::selectFields() + $pageQuery['fields'] ), [ "{$prefix}_from" => $id ], __METHOD__, $options, - [ 'page' => [ + [ 'nestpage' => [ 'LEFT JOIN', [ "page_namespace=$blNamespace", "page_title=$blTitle" ] - ] ] + ] ] + $pageQuery['joins'] ); $retVal = []; @@ -4193,13 +4194,15 @@ class Title implements LinkTarget { $pageId = $this->getArticleID( $flags ); if ( $pageId ) { $db = ( $flags & self::GAID_FOR_UPDATE ) ? wfGetDB( DB_MASTER ) : wfGetDB( DB_REPLICA ); - $row = $db->selectRow( 'revision', Revision::selectFields(), + $revQuery = Revision::getQueryInfo(); + $row = $db->selectRow( $revQuery['tables'], $revQuery['fields'], [ 'rev_page' => $pageId ], __METHOD__, [ 'ORDER BY' => 'rev_timestamp ASC, rev_id ASC', - 'IGNORE INDEX' => 'rev_timestamp', // See T159319 - ] + 'IGNORE INDEX' => [ 'revision' => 'rev_timestamp' ], // See T159319 + ], + $revQuery['joins'] ); if ( $row ) { return new Revision( $row ); diff --git a/includes/actions/HistoryAction.php b/includes/actions/HistoryAction.php index a9a504d13a..a9e3d6accd 100644 --- a/includes/actions/HistoryAction.php +++ b/includes/actions/HistoryAction.php @@ -258,12 +258,18 @@ class HistoryAction extends FormlessAction { $page_id = $this->page->getId(); - return $dbr->select( 'revision', - Revision::selectFields(), + $revQuery = Revision::getQueryInfo(); + return $dbr->select( + $revQuery['tables'], + $revQuery['fields'], array_merge( [ 'rev_page' => $page_id ], $offsets ), __METHOD__, - [ 'ORDER BY' => "rev_timestamp $dirs", - 'USE INDEX' => 'page_timestamp', 'LIMIT' => $limit ] + [ + 'ORDER BY' => "rev_timestamp $dirs", + 'USE INDEX' => [ 'revision' => 'page_timestamp' ], + 'LIMIT' => $limit + ], + $revQuery['joins'] ); } @@ -418,14 +424,15 @@ class HistoryPager extends ReverseChronologicalPager { } function getQueryInfo() { + $revQuery = Revision::getQueryInfo( [ 'user' ] ); $queryInfo = [ - 'tables' => [ 'revision', 'user' ], - 'fields' => array_merge( Revision::selectFields(), Revision::selectUserFields() ), + 'tables' => $revQuery['tables'], + 'fields' => $revQuery['fields'], 'conds' => array_merge( [ 'rev_page' => $this->getWikiPage()->getId() ], $this->conds ), 'options' => [ 'USE INDEX' => [ 'revision' => 'page_timestamp' ] ], - 'join_conds' => [ 'user' => Revision::userJoinCond() ], + 'join_conds' => $revQuery['joins'], ]; ChangeTags::modifyDisplayQuery( $queryInfo['tables'], diff --git a/includes/api/ApiComparePages.php b/includes/api/ApiComparePages.php index 953bc10cc3..eb67babf2a 100644 --- a/includes/api/ApiComparePages.php +++ b/includes/api/ApiComparePages.php @@ -175,14 +175,17 @@ class ApiComparePages extends ApiBase { $rev = Revision::newFromId( $revId ); if ( !$rev ) { // Titles of deleted revisions aren't secret, per T51088 + $arQuery = Revision::getArchiveQueryInfo(); $row = $this->getDB()->selectRow( - 'archive', + $arQuery['tables'], array_merge( - Revision::selectArchiveFields(), + $arQuery['fields'], [ 'ar_namespace', 'ar_title' ] ), [ 'ar_rev_id' => $revId ], - __METHOD__ + __METHOD__, + [], + $arQuery['joins'] ); if ( $row ) { $rev = Revision::newFromArchiveRow( $row ); @@ -285,14 +288,17 @@ class ApiComparePages extends ApiBase { $rev = Revision::newFromId( $revId ); if ( !$rev && $this->getUser()->isAllowedAny( 'deletedtext', 'undelete' ) ) { // Try the 'archive' table + $arQuery = Revision::getArchiveQueryInfo(); $row = $this->getDB()->selectRow( - 'archive', + $arQuery['tables'], array_merge( - Revision::selectArchiveFields(), + $arQuery['fields'], [ 'ar_namespace', 'ar_title' ] ), [ 'ar_rev_id' => $revId ], - __METHOD__ + __METHOD__, + [], + $arQuery['joins'] ); if ( $row ) { $rev = Revision::newFromArchiveRow( $row ); diff --git a/includes/api/ApiQueryAllDeletedRevisions.php b/includes/api/ApiQueryAllDeletedRevisions.php index b22bb1ff15..765b5c7f1d 100644 --- a/includes/api/ApiQueryAllDeletedRevisions.php +++ b/includes/api/ApiQueryAllDeletedRevisions.php @@ -103,13 +103,16 @@ class ApiQueryAllDeletedRevisions extends ApiQueryRevisionsBase { } } - $this->addTables( 'archive' ); if ( $resultPageSet === null ) { $this->parseParameters( $params ); - $this->addFields( Revision::selectArchiveFields() ); + $arQuery = Revision::getArchiveQueryInfo(); + $this->addTables( $arQuery['tables'] ); + $this->addJoinConds( $arQuery['joins'] ); + $this->addFields( $arQuery['fields'] ); $this->addFields( [ 'ar_title', 'ar_namespace' ] ); } else { $this->limit = $this->getParameter( 'limit' ) ?: 10; + $this->addTables( 'archive' ); $this->addFields( [ 'ar_title', 'ar_namespace' ] ); if ( $optimizeGenerateTitles ) { $this->addOption( 'DISTINCT' ); diff --git a/includes/api/ApiQueryAllImages.php b/includes/api/ApiQueryAllImages.php index 250bee667f..6f497b1996 100644 --- a/includes/api/ApiQueryAllImages.php +++ b/includes/api/ApiQueryAllImages.php @@ -90,10 +90,12 @@ class ApiQueryAllImages extends ApiQueryGeneratorBase { $userId = !is_null( $params['user'] ) ? User::idFromName( $params['user'] ) : null; // Table and return fields - $this->addTables( 'image' ); - $prop = array_flip( $params['prop'] ); - $this->addFields( LocalFile::selectFields() ); + + $fileQuery = LocalFile::getQueryInfo(); + $this->addTables( $fileQuery['tables'] ); + $this->addFields( $fileQuery['fields'] ); + $this->addJoinConds( $fileQuery['joins'] ); $ascendingOrder = true; if ( $params['dir'] == 'descending' || $params['dir'] == 'older' ) { diff --git a/includes/api/ApiQueryAllRevisions.php b/includes/api/ApiQueryAllRevisions.php index 8f7d6eb28f..c8db6a1189 100644 --- a/includes/api/ApiQueryAllRevisions.php +++ b/includes/api/ApiQueryAllRevisions.php @@ -63,20 +63,20 @@ class ApiQueryAllRevisions extends ApiQueryRevisionsBase { } } - $this->addTables( 'revision' ); if ( $resultPageSet === null ) { $this->parseParameters( $params ); - $this->addTables( 'page' ); - $this->addJoinConds( - [ 'page' => [ 'INNER JOIN', [ 'rev_page = page_id' ] ] ] + $revQuery = Revision::getQueryInfo( + $this->fetchContent ? [ 'page', 'text' ] : [ 'page' ] ); - $this->addFields( Revision::selectFields() ); - $this->addFields( Revision::selectPageFields() ); + $this->addTables( $revQuery['tables'] ); + $this->addFields( $revQuery['fields'] ); + $this->addJoinConds( $revQuery['joins'] ); // Review this depeneding on the outcome of T113901 $this->addOption( 'STRAIGHT_JOIN' ); } else { $this->limit = $this->getParameter( 'limit' ) ?: 10; + $this->addTables( 'revision' ); $this->addFields( [ 'rev_timestamp', 'rev_id' ] ); if ( $params['generatetitles'] ) { $this->addFields( [ 'rev_page' ] ); @@ -105,15 +105,6 @@ class ApiQueryAllRevisions extends ApiQueryRevisionsBase { $this->addFields( 'ts_tags' ); } - if ( $this->fetchContent ) { - $this->addTables( 'text' ); - $this->addJoinConds( - [ 'text' => [ 'INNER JOIN', [ 'rev_text_id=old_id' ] ] ] - ); - $this->addFields( 'old_id' ); - $this->addFields( Revision::selectTextFields() ); - } - if ( $params['user'] !== null ) { $id = User::idFromName( $params['user'] ); if ( $id ) { diff --git a/includes/api/ApiQueryDeletedRevisions.php b/includes/api/ApiQueryDeletedRevisions.php index 8e4752e8cf..3339fec5e5 100644 --- a/includes/api/ApiQueryDeletedRevisions.php +++ b/includes/api/ApiQueryDeletedRevisions.php @@ -60,13 +60,16 @@ class ApiQueryDeletedRevisions extends ApiQueryRevisionsBase { $this->requireMaxOneParameter( $params, 'user', 'excludeuser' ); - $this->addTables( 'archive' ); if ( $resultPageSet === null ) { $this->parseParameters( $params ); - $this->addFields( Revision::selectArchiveFields() ); + $arQuery = Revision::getArchiveQueryInfo(); + $this->addTables( $arQuery['tables'] ); + $this->addFields( $arQuery['fields'] ); + $this->addJoinConds( $arQuery['joins'] ); $this->addFields( [ 'ar_title', 'ar_namespace' ] ); } else { $this->limit = $this->getParameter( 'limit' ) ?: 10; + $this->addTables( 'archive' ); $this->addFields( [ 'ar_title', 'ar_namespace', 'ar_timestamp', 'ar_rev_id', 'ar_id' ] ); } diff --git a/includes/api/ApiQueryFilearchive.php b/includes/api/ApiQueryFilearchive.php index 212b61340a..838fc2bda4 100644 --- a/includes/api/ApiQueryFilearchive.php +++ b/includes/api/ApiQueryFilearchive.php @@ -60,25 +60,10 @@ class ApiQueryFilearchive extends ApiQueryBase { $fld_bitdepth = isset( $prop['bitdepth'] ); $fld_archivename = isset( $prop['archivename'] ); - $this->addTables( 'filearchive' ); - - $this->addFields( ArchivedFile::selectFields() ); - $this->addFields( [ 'fa_id', 'fa_name', 'fa_timestamp', 'fa_deleted' ] ); - $this->addFieldsIf( 'fa_sha1', $fld_sha1 ); - $this->addFieldsIf( [ 'fa_user', 'fa_user_text' ], $fld_user ); - $this->addFieldsIf( [ 'fa_height', 'fa_width', 'fa_size' ], $fld_dimensions || $fld_size ); - $this->addFieldsIf( [ 'fa_major_mime', 'fa_minor_mime' ], $fld_mime ); - $this->addFieldsIf( 'fa_media_type', $fld_mediatype ); - $this->addFieldsIf( 'fa_metadata', $fld_metadata ); - $this->addFieldsIf( 'fa_bits', $fld_bitdepth ); - $this->addFieldsIf( 'fa_archive_name', $fld_archivename ); - - if ( $fld_description ) { - $commentQuery = $commentStore->getJoin(); - $this->addTables( $commentQuery['tables'] ); - $this->addFields( $commentQuery['fields'] ); - $this->addJoinConds( $commentQuery['joins'] ); - } + $fileQuery = ArchivedFile::getQueryInfo(); + $this->addTables( $fileQuery['tables'] ); + $this->addFields( $fileQuery['fields'] ); + $this->addJoinConds( $fileQuery['joins'] ); if ( !is_null( $params['continue'] ) ) { $cont = explode( '|', $params['continue'] ); diff --git a/includes/api/ApiQueryRevisions.php b/includes/api/ApiQueryRevisions.php index 2dfa42a3a4..a04b7c545c 100644 --- a/includes/api/ApiQueryRevisions.php +++ b/includes/api/ApiQueryRevisions.php @@ -129,20 +129,31 @@ class ApiQueryRevisions extends ApiQueryRevisionsBase { } $db = $this->getDB(); - $this->addTables( [ 'revision', 'page' ] ); - $this->addJoinConds( - [ 'page' => [ 'INNER JOIN', [ 'page_id = rev_page' ] ] ] - ); if ( $resultPageSet === null ) { $this->parseParameters( $params ); $this->token = $params['token']; - $this->addFields( Revision::selectFields() ); + $opts = []; if ( $this->token !== null || $pageCount > 0 ) { - $this->addFields( Revision::selectPageFields() ); + $opts[] = 'page'; } + if ( $this->fetchContent ) { + $opts[] = 'text'; + } + if ( $this->fld_user ) { + $opts[] = 'user'; + } + $revQuery = Revision::getQueryInfo( $opts ); + $this->addTables( $revQuery['tables'] ); + $this->addFields( $revQuery['fields'] ); + $this->addJoinConds( $revQuery['joins'] ); } else { $this->limit = $this->getParameter( 'limit' ) ?: 10; + // Always join 'page' so orphaned revisions are filtered out + $this->addTables( [ 'revision', 'page' ] ); + $this->addJoinConds( + [ 'page' => [ 'INNER JOIN', [ 'page_id = rev_page' ] ] ] + ); $this->addFields( [ 'rev_id', 'rev_timestamp', 'rev_page' ] ); } @@ -162,7 +173,7 @@ class ApiQueryRevisions extends ApiQueryRevisionsBase { $this->addWhereFld( 'ct_tag', $params['tag'] ); } - if ( $this->fetchContent ) { + if ( $resultPageSet === null && $this->fetchContent ) { // For each page we will request, the user must have read rights for that page $user = $this->getUser(); $status = Status::newGood(); @@ -178,20 +189,6 @@ class ApiQueryRevisions extends ApiQueryRevisionsBase { if ( !$status->isGood() ) { $this->dieStatus( $status ); } - - $this->addTables( 'text' ); - $this->addJoinConds( - [ 'text' => [ 'INNER JOIN', [ 'rev_text_id=old_id' ] ] ] - ); - $this->addFields( 'old_id' ); - $this->addFields( Revision::selectTextFields() ); - } - - // add user name, if needed - if ( $this->fld_user ) { - $this->addTables( 'user' ); - $this->addJoinConds( [ 'user' => Revision::userJoinCond() ] ); - $this->addFields( Revision::selectUserFields() ); } if ( $enumRevMode ) { diff --git a/includes/api/ApiQueryUsers.php b/includes/api/ApiQueryUsers.php index fbf1f9ebfb..8fc99bbcf9 100644 --- a/includes/api/ApiQueryUsers.php +++ b/includes/api/ApiQueryUsers.php @@ -144,8 +144,10 @@ class ApiQueryUsers extends ApiQueryBase { $result = $this->getResult(); if ( count( $parameters ) ) { - $this->addTables( 'user' ); - $this->addFields( User::selectFields() ); + $userQuery = User::getQueryInfo(); + $this->addTables( $userQuery['tables'] ); + $this->addFields( $userQuery['fields'] ); + $this->addJoinConds( $userQuery['joins'] ); if ( $useNames ) { $this->addWhereFld( 'user_name', $goodNames ); } else { diff --git a/includes/changes/RecentChange.php b/includes/changes/RecentChange.php index fd789a6421..bb2412074c 100644 --- a/includes/changes/RecentChange.php +++ b/includes/changes/RecentChange.php @@ -192,7 +192,10 @@ class RecentChange { $dbType = DB_REPLICA ) { $db = wfGetDB( $dbType ); - $row = $db->selectRow( 'recentchanges', self::selectFields(), $conds, $fname ); + $rcQuery = self::getQueryInfo(); + $row = $db->selectRow( + $rcQuery['tables'], $rcQuery['fields'], $conds, $fname, [], $rcQuery['joins'] + ); if ( $row !== false ) { return self::newFromRow( $row ); } else { @@ -203,8 +206,7 @@ class RecentChange { /** * Return the list of recentchanges fields that should be selected to create * a new recentchanges object. - * @todo Deprecate this in favor of a method that returns tables and joins - * as well, and use CommentStore::getJoin(). + * @deprecated since 1.31, use self::getQueryInfo() instead. * @return array */ public static function selectFields() { @@ -235,6 +237,48 @@ class RecentChange { ] + CommentStore::newKey( 'rc_comment' )->getFields(); } + /** + * Return the tables, fields, and join conditions to be selected to create + * a new recentchanges object. + * @since 1.31 + * @return array With three keys: + * - tables: (string[]) to include in the `$table` to `IDatabase->select()` + * - fields: (string[]) to include in the `$vars` to `IDatabase->select()` + * - joins: (array) to include in the `$join_conds` to `IDatabase->select()` + */ + public static function getQueryInfo() { + $commentQuery = CommentStore::newKey( 'rc_comment' )->getJoin(); + return [ + 'tables' => [ 'recentchanges' ] + $commentQuery['tables'], + 'fields' => [ + 'rc_id', + 'rc_timestamp', + 'rc_user', + 'rc_user_text', + 'rc_namespace', + 'rc_title', + 'rc_minor', + 'rc_bot', + 'rc_new', + 'rc_cur_id', + 'rc_this_oldid', + 'rc_last_oldid', + 'rc_type', + 'rc_source', + 'rc_patrolled', + 'rc_ip', + 'rc_old_len', + 'rc_new_len', + 'rc_deleted', + 'rc_logid', + 'rc_log_type', + 'rc_log_action', + 'rc_params', + ] + $commentQuery['fields'], + 'joins' => $commentQuery['joins'], + ]; + } + # Accessors /** @@ -951,7 +995,7 @@ class RecentChange { } $comment = CommentStore::newKey( 'rc_comment' ) - // Legacy because $row probably came from self::selectFields() + // Legacy because $row may have come from self::selectFields() ->getCommentLegacy( wfGetDB( DB_REPLICA ), $row, true )->text; $this->mAttribs['rc_comment'] = &$comment; $this->mAttribs['rc_comment_text'] = &$comment; diff --git a/includes/changetags/ChangeTagsRevisionList.php b/includes/changetags/ChangeTagsRevisionList.php index 91193b0ecd..61259b4192 100644 --- a/includes/changetags/ChangeTagsRevisionList.php +++ b/includes/changetags/ChangeTagsRevisionList.php @@ -36,18 +36,16 @@ class ChangeTagsRevisionList extends ChangeTagsList { */ public function doQuery( $db ) { $ids = array_map( 'intval', $this->ids ); + $revQuery = Revision::getQueryInfo( [ 'user' ] ); $queryInfo = [ - 'tables' => [ 'revision', 'user' ], - 'fields' => array_merge( Revision::selectFields(), Revision::selectUserFields() ), + 'tables' => $revQuery['tables'], + 'fields' => $revQuery['fields'], 'conds' => [ 'rev_page' => $this->title->getArticleID(), 'rev_id' => $ids, ], 'options' => [ 'ORDER BY' => 'rev_id DESC' ], - 'join_conds' => [ - 'page' => Revision::pageJoinCond(), - 'user' => Revision::userJoinCond(), - ], + 'join_conds' => $revQuery['joins'], ]; ChangeTags::modifyDisplayQuery( $queryInfo['tables'], diff --git a/includes/diff/DifferenceEngine.php b/includes/diff/DifferenceEngine.php index ef67477c66..6a2837b43a 100644 --- a/includes/diff/DifferenceEngine.php +++ b/includes/diff/DifferenceEngine.php @@ -182,13 +182,15 @@ class DifferenceEngine extends ContextSource { public function deletedLink( $id ) { if ( $this->getUser()->isAllowed( 'deletedhistory' ) ) { $dbr = wfGetDB( DB_REPLICA ); - $row = $dbr->selectRow( 'archive', - array_merge( - Revision::selectArchiveFields(), - [ 'ar_namespace', 'ar_title' ] - ), + $arQuery = Revision::getArchiveQueryInfo(); + $row = $dbr->selectRow( + $arQuery['tables'], + array_merge( $arQuery['fields'], [ 'ar_namespace', 'ar_title' ] ), [ 'ar_rev_id' => $id ], - __METHOD__ ); + __METHOD__, + [], + $arQuery['joins'] + ); if ( $row ) { $rev = Revision::newFromArchiveRow( $row ); $title = Title::makeTitleSafe( $row->ar_namespace, $row->ar_title ); diff --git a/includes/filerepo/LocalRepo.php b/includes/filerepo/LocalRepo.php index ed00793508..f5b83ae5cc 100644 --- a/includes/filerepo/LocalRepo.php +++ b/includes/filerepo/LocalRepo.php @@ -310,8 +310,9 @@ class LocalRepo extends FileRepo { } if ( count( $imgNames ) ) { - $res = $dbr->select( 'image', - LocalFile::selectFields(), [ 'img_name' => $imgNames ], __METHOD__ ); + $fileQuery = LocalFile::getQueryInfo(); + $res = $dbr->select( $fileQuery['tables'], $fileQuery['fields'], [ 'img_name' => $imgNames ], + __METHOD__, [], $fileQuery['joins'] ); $applyMatchingFiles( $res, $searchSet, $finalFiles ); } @@ -330,8 +331,10 @@ class LocalRepo extends FileRepo { } if ( count( $oiConds ) ) { - $res = $dbr->select( 'oldimage', - OldLocalFile::selectFields(), $dbr->makeList( $oiConds, LIST_OR ), __METHOD__ ); + $fileQuery = OldLocalFile::getQueryInfo(); + $res = $dbr->select( $fileQuery['tables'], $fileQuery['fields'], + $dbr->makeList( $oiConds, LIST_OR ), + __METHOD__, [], $fileQuery['joins'] ); $applyMatchingFiles( $res, $searchSet, $finalFiles ); } @@ -372,12 +375,14 @@ class LocalRepo extends FileRepo { */ function findBySha1( $hash ) { $dbr = $this->getReplicaDB(); + $fileQuery = LocalFile::getQueryInfo(); $res = $dbr->select( - 'image', - LocalFile::selectFields(), + $fileQuery['tables'], + $fileQuery['fields'], [ 'img_sha1' => $hash ], __METHOD__, - [ 'ORDER BY' => 'img_name' ] + [ 'ORDER BY' => 'img_name' ], + $fileQuery['joins'] ); $result = []; @@ -404,12 +409,14 @@ class LocalRepo extends FileRepo { } $dbr = $this->getReplicaDB(); + $fileQuery = LocalFile::getQueryInfo(); $res = $dbr->select( - 'image', - LocalFile::selectFields(), + $fileQuery['tables'], + $fileQuery['fields'], [ 'img_sha1' => $hashes ], __METHOD__, - [ 'ORDER BY' => 'img_name' ] + [ 'ORDER BY' => 'img_name' ], + $fileQuery['joins'] ); $result = []; @@ -434,12 +441,14 @@ class LocalRepo extends FileRepo { // Query database $dbr = $this->getReplicaDB(); + $fileQuery = LocalFile::getQueryInfo(); $res = $dbr->select( - 'image', - LocalFile::selectFields(), + $fileQuery['tables'], + $fileQuery['fields'], 'img_name ' . $dbr->buildLike( $prefix, $dbr->anyString() ), __METHOD__, - $selectOptions + $selectOptions, + $fileQuery['joins'] ); // Build file objects diff --git a/includes/filerepo/file/ArchivedFile.php b/includes/filerepo/file/ArchivedFile.php index 758fb4b5a1..7f48659ba3 100644 --- a/includes/filerepo/file/ArchivedFile.php +++ b/includes/filerepo/file/ArchivedFile.php @@ -178,12 +178,14 @@ class ArchivedFile { if ( !$this->title || $this->title->getNamespace() == NS_FILE ) { $this->dataLoaded = true; // set it here, to have also true on miss $dbr = wfGetDB( DB_REPLICA ); + $fileQuery = self::getQueryInfo(); $row = $dbr->selectRow( - 'filearchive', - self::selectFields(), + $fileQuery['tables'], + $fileQuery['fields'], $conds, __METHOD__, - [ 'ORDER BY' => 'fa_timestamp DESC' ] + [ 'ORDER BY' => 'fa_timestamp DESC' ], + $fileQuery['joins'] ); if ( !$row ) { // this revision does not exist? @@ -215,11 +217,11 @@ class ArchivedFile { /** * Fields in the filearchive table - * @todo Deprecate this in favor of a method that returns tables and joins - * as well, and use CommentStore::getJoin(). + * @deprecated since 1.31, use self::getQueryInfo() instead. * @return array */ static function selectFields() { + wfDeprecated( __METHOD__, '1.31' ); return [ 'fa_id', 'fa_name', @@ -243,6 +245,44 @@ class ArchivedFile { ] + CommentStore::newKey( 'fa_description' )->getFields(); } + /** + * Return the tables, fields, and join conditions to be selected to create + * a new archivedfile object. + * @since 1.31 + * @return array With three keys: + * - tables: (string[]) to include in the `$table` to `IDatabase->select()` + * - fields: (string[]) to include in the `$vars` to `IDatabase->select()` + * - joins: (array) to include in the `$join_conds` to `IDatabase->select()` + */ + public static function getQueryInfo() { + $commentQuery = CommentStore::newKey( 'fa_description' )->getJoin(); + return [ + 'tables' => [ 'filearchive' ] + $commentQuery['tables'], + 'fields' => [ + 'fa_id', + 'fa_name', + 'fa_archive_name', + 'fa_storage_key', + 'fa_storage_group', + 'fa_size', + 'fa_bits', + 'fa_width', + 'fa_height', + 'fa_metadata', + 'fa_media_type', + 'fa_major_mime', + 'fa_minor_mime', + 'fa_user', + 'fa_user_text', + 'fa_timestamp', + 'fa_deleted', + 'fa_deleted_timestamp', /* Used by LocalFileRestoreBatch */ + 'fa_sha1', + ] + $commentQuery['fields'], + 'joins' => $commentQuery['joins'], + ]; + } + /** * Load ArchivedFile object fields from a DB row. * @@ -263,7 +303,7 @@ class ArchivedFile { $this->mime = "$row->fa_major_mime/$row->fa_minor_mime"; $this->media_type = $row->fa_media_type; $this->description = CommentStore::newKey( 'fa_description' ) - // Legacy because $row probably came from self::selectFields() + // Legacy because $row may have come from self::selectFields() ->getCommentLegacy( wfGetDB( DB_REPLICA ), $row )->text; $this->user = $row->fa_user; $this->user_text = $row->fa_user_text; diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php index 3271c966b8..0b730f60af 100644 --- a/includes/filerepo/file/LocalFile.php +++ b/includes/filerepo/file/LocalFile.php @@ -183,7 +183,10 @@ class LocalFile extends File { $conds['img_timestamp'] = $dbr->timestamp( $timestamp ); } - $row = $dbr->selectRow( 'image', self::selectFields(), $conds, __METHOD__ ); + $fileQuery = self::getQueryInfo(); + $row = $dbr->selectRow( + $fileQuery['tables'], $fileQuery['fields'], $conds, __METHOD__, [], $fileQuery['joins'] + ); if ( $row ) { return self::newFromRow( $row, $repo ); } else { @@ -193,8 +196,7 @@ class LocalFile extends File { /** * Fields in the image table - * @todo Deprecate this in favor of a method that returns tables and joins - * as well, and use CommentStore::getJoin(). + * @deprecated since 1.31, use self::getQueryInfo() instead. * @return array */ static function selectFields() { @@ -215,6 +217,51 @@ class LocalFile extends File { ] + CommentStore::newKey( 'img_description' )->getFields(); } + /** + * Return the tables, fields, and join conditions to be selected to create + * a new localfile object. + * @since 1.31 + * @param string[] $options + * - omit-lazy: Omit fields that are lazily cached. + * @return array With three keys: + * - tables: (string[]) to include in the `$table` to `IDatabase->select()` + * - fields: (string[]) to include in the `$vars` to `IDatabase->select()` + * - joins: (array) to include in the `$join_conds` to `IDatabase->select()` + */ + public static function getQueryInfo( array $options = [] ) { + $commentQuery = CommentStore::newKey( 'img_description' )->getJoin(); + $ret = [ + 'tables' => [ 'image' ] + $commentQuery['tables'], + 'fields' => [ + 'img_name', + 'img_size', + 'img_width', + 'img_height', + 'img_metadata', + 'img_bits', + 'img_media_type', + 'img_major_mime', + 'img_minor_mime', + 'img_user', + 'img_user_text', + 'img_timestamp', + 'img_sha1', + ] + $commentQuery['fields'], + 'joins' => $commentQuery['joins'], + ]; + + if ( in_array( 'omit-nonlazy', $options, true ) ) { + // Internal use only for getting only the lazy fields + $ret['fields'] = []; + } + if ( !in_array( 'omit-lazy', $options, true ) ) { + // Note: Keep this in sync with self::getLazyCacheFields() + $ret['fields'][] = 'img_metadata'; + } + + return $ret; + } + /** * Do not call this except from inside a repo class. * @param Title $title @@ -341,51 +388,43 @@ class LocalFile extends File { } /** - * @param string $prefix + * Returns the list of object properties that are included as-is in the cache. + * @param string $prefix Must be the empty string * @return array + * @since 1.31 No longer accepts a non-empty $prefix */ - function getCacheFields( $prefix = 'img_' ) { - static $fields = [ 'size', 'width', 'height', 'bits', 'media_type', - 'major_mime', 'minor_mime', 'metadata', 'timestamp', 'sha1', 'user', - 'user_text' ]; - static $results = []; - - if ( $prefix == '' ) { - return array_merge( $fields, [ 'description' ] ); - } - if ( !isset( $results[$prefix] ) ) { - $prefixedFields = []; - foreach ( $fields as $field ) { - $prefixedFields[] = $prefix . $field; - } - $prefixedFields += CommentStore::newKey( "{$prefix}description" )->getFields(); - $results[$prefix] = $prefixedFields; + protected function getCacheFields( $prefix = 'img_' ) { + if ( $prefix !== '' ) { + throw new InvalidArgumentException( + __METHOD__ . ' with a non-empty prefix is no longer supported.' + ); } - return $results[$prefix]; + // See self::getQueryInfo() for the fetching of the data from the DB, + // self::loadFromRow() for the loading of the object from the DB row, + // and self::loadFromCache() for the caching, and self::setProps() for + // populating the object from an array of data. + return [ 'size', 'width', 'height', 'bits', 'media_type', + 'major_mime', 'minor_mime', 'metadata', 'timestamp', 'sha1', 'user', + 'user_text', 'description' ]; } /** - * @param string $prefix + * Returns the list of object properties that are included as-is in the + * cache, only when they're not too big, and are lazily loaded by self::loadExtraFromDB(). + * @param string $prefix Must be the empty string * @return array + * @since 1.31 No longer accepts a non-empty $prefix */ - function getLazyCacheFields( $prefix = 'img_' ) { - static $fields = [ 'metadata' ]; - static $results = []; - - if ( $prefix == '' ) { - return $fields; - } - - if ( !isset( $results[$prefix] ) ) { - $prefixedFields = []; - foreach ( $fields as $field ) { - $prefixedFields[] = $prefix . $field; - } - $results[$prefix] = $prefixedFields; + protected function getLazyCacheFields( $prefix = 'img_' ) { + if ( $prefix !== '' ) { + throw new InvalidArgumentException( + __METHOD__ . ' with a non-empty prefix is no longer supported.' + ); } - return $results[$prefix]; + // Keep this in sync with the omit-lazy option in self::getQueryInfo(). + return [ 'metadata' ]; } /** @@ -403,8 +442,15 @@ class LocalFile extends File { ? $this->repo->getMasterDB() : $this->repo->getReplicaDB(); - $row = $dbr->selectRow( 'image', $this->getCacheFields( 'img_' ), - [ 'img_name' => $this->getName() ], $fname ); + $fileQuery = static::getQueryInfo(); + $row = $dbr->selectRow( + $fileQuery['tables'], + $fileQuery['fields'], + [ 'img_name' => $this->getName() ], + $fname, + [], + $fileQuery['joins'] + ); if ( $row ) { $this->loadFromRow( $row ); @@ -423,9 +469,9 @@ class LocalFile extends File { # Unconditionally set loaded=true, we don't want the accessors constantly rechecking $this->extraDataLoaded = true; - $fieldMap = $this->loadFieldsWithTimestamp( $this->repo->getReplicaDB(), $fname ); + $fieldMap = $this->loadExtraFieldsWithTimestamp( $this->repo->getReplicaDB(), $fname ); if ( !$fieldMap ) { - $fieldMap = $this->loadFieldsWithTimestamp( $this->repo->getMasterDB(), $fname ); + $fieldMap = $this->loadExtraFieldsWithTimestamp( $this->repo->getMasterDB(), $fname ); } if ( $fieldMap ) { @@ -442,26 +488,46 @@ class LocalFile extends File { * @param string $fname * @return array|bool */ - private function loadFieldsWithTimestamp( $dbr, $fname ) { + private function loadExtraFieldsWithTimestamp( $dbr, $fname ) { $fieldMap = false; - $row = $dbr->selectRow( 'image', $this->getLazyCacheFields( 'img_' ), [ + $fileQuery = self::getQueryInfo( [ 'omit-nonlazy' ] ); + $row = $dbr->selectRow( + $fileQuery['tables'], + $fileQuery['fields'], + [ 'img_name' => $this->getName(), - 'img_timestamp' => $dbr->timestamp( $this->getTimestamp() ) - ], $fname ); + 'img_timestamp' => $dbr->timestamp( $this->getTimestamp() ), + ], + $fname, + [], + $fileQuery['joins'] + ); if ( $row ) { $fieldMap = $this->unprefixRow( $row, 'img_' ); } else { # File may have been uploaded over in the meantime; check the old versions - $row = $dbr->selectRow( 'oldimage', $this->getLazyCacheFields( 'oi_' ), [ + $fileQuery = OldLocalFile::getQueryInfo( [ 'omit-nonlazy' ] ); + $row = $dbr->selectRow( + $fileQuery['tables'], + $fileQuery['fields'], + [ 'oi_name' => $this->getName(), - 'oi_timestamp' => $dbr->timestamp( $this->getTimestamp() ) - ], $fname ); + 'oi_timestamp' => $dbr->timestamp( $this->getTimestamp() ), + ], + $fname, + [], + $fileQuery['joins'] + ); if ( $row ) { $fieldMap = $this->unprefixRow( $row, 'oi_' ); } } + if ( isset( $fieldMap['metadata'] ) ) { + $fieldMap['metadata'] = $this->repo->getReplicaDB()->decodeBlob( $fieldMap['metadata'] ); + } + return $fieldMap; } @@ -499,6 +565,9 @@ class LocalFile extends File { function decodeRow( $row, $prefix = 'img_' ) { $decoded = $this->unprefixRow( $row, $prefix ); + $decoded['description'] = CommentStore::newKey( 'description' ) + ->getComment( (object)$decoded )->text; + $decoded['timestamp'] = wfTimestamp( TS_MW, $decoded['timestamp'] ); $decoded['metadata'] = $this->repo->getReplicaDB()->decodeBlob( $decoded['metadata'] ); @@ -536,10 +605,6 @@ class LocalFile extends File { $this->dataLoaded = true; $this->extraDataLoaded = true; - $this->description = CommentStore::newKey( "{$prefix}description" ) - // $row is probably using getFields() from self::getCacheFields() - ->getCommentLegacy( wfGetDB( DB_REPLICA ), $row )->text; - $array = $this->decodeRow( $row, $prefix ); foreach ( $array as $name => $value ) { @@ -1069,9 +1134,12 @@ class LocalFile extends File { */ function getHistory( $limit = null, $start = null, $end = null, $inc = true ) { $dbr = $this->repo->getReplicaDB(); - $tables = [ 'oldimage' ]; - $fields = OldLocalFile::selectFields(); - $conds = $opts = $join_conds = []; + $oldFileQuery = OldLocalFile::getQueryInfo(); + + $tables = $oldFileQuery['tables']; + $fields = $oldFileQuery['fields']; + $join_conds = $oldFileQuery['joins']; + $conds = $opts = []; $eq = $inc ? '=' : ''; $conds[] = "oi_name = " . $dbr->addQuotes( $this->title->getDBkey() ); @@ -1127,13 +1195,16 @@ class LocalFile extends File { $dbr = $this->repo->getReplicaDB(); if ( $this->historyLine == 0 ) { // called for the first time, return line from cur - $this->historyRes = $dbr->select( 'image', - self::selectFields() + [ + $fileQuery = self::getQueryInfo(); + $this->historyRes = $dbr->select( $fileQuery['tables'], + $fileQuery['fields'] + [ 'oi_archive_name' => $dbr->addQuotes( '' ), 'oi_deleted' => 0, ], [ 'img_name' => $this->title->getDBkey() ], - $fname + $fname, + [], + $fileQuery['joins'] ); if ( 0 == $dbr->numRows( $this->historyRes ) ) { @@ -1142,12 +1213,14 @@ class LocalFile extends File { return false; } } elseif ( $this->historyLine == 1 ) { + $fileQuery = OldLocalFile::getQueryInfo(); $this->historyRes = $dbr->select( - 'oldimage', - OldLocalFile::selectFields(), + $fileQuery['tables'], + $fileQuery['fields'], [ 'oi_name' => $this->title->getDBkey() ], $fname, - [ 'ORDER BY' => 'oi_timestamp DESC' ] + [ 'ORDER BY' => 'oi_timestamp DESC' ], + $fileQuery['joins'] ); } $this->historyLine++; @@ -2418,22 +2491,23 @@ class LocalFileDeleteBatch { } if ( count( $oldRels ) ) { + $fileQuery = OldLocalFile::getQueryInfo(); $res = $dbw->select( - 'oldimage', - OldLocalFile::selectFields(), + $fileQuery['tables'], + $fileQuery['fields'], [ 'oi_name' => $this->file->getName(), 'oi_archive_name' => array_keys( $oldRels ) ], __METHOD__, - [ 'FOR UPDATE' ] + [ 'FOR UPDATE' ], + $fileQuery['joins'] ); $rowsInsert = []; if ( $res->numRows() ) { $reason = $commentStoreFaReason->createComment( $dbw, $this->reason ); foreach ( $res as $row ) { - // Legacy from OldLocalFile::selectFields() just above - $comment = $commentStoreOiDesc->getCommentLegacy( $dbw, $row ); + $comment = $commentStoreOiDesc->getComment( $row ); $rowsInsert[] = [ // Deletion-specific fields 'fa_storage_group' => 'deleted', @@ -2680,12 +2754,14 @@ class LocalFileRestoreBatch { $conditions['fa_id'] = $this->ids; } + $arFileQuery = ArchivedFile::getQueryInfo(); $result = $dbw->select( - 'filearchive', - ArchivedFile::selectFields(), + $arFileQuery['tables'], + $arFileQuery['fields'], $conditions, __METHOD__, - [ 'ORDER BY' => 'fa_timestamp DESC' ] + [ 'ORDER BY' => 'fa_timestamp DESC' ], + $arFileQuery['joins'] ); $idsPresent = []; @@ -2745,8 +2821,7 @@ class LocalFileRestoreBatch { ]; } - // Legacy from ArchivedFile::selectFields() just above - $comment = $commentStoreFaDesc->getCommentLegacy( $dbw, $row ); + $comment = $commentStoreFaDesc->getComment( $row ); if ( $first && !$exists ) { // This revision will be published as the new current version $destRel = $this->file->getRel(); diff --git a/includes/filerepo/file/OldLocalFile.php b/includes/filerepo/file/OldLocalFile.php index ee172e1143..6833a3880e 100644 --- a/includes/filerepo/file/OldLocalFile.php +++ b/includes/filerepo/file/OldLocalFile.php @@ -93,7 +93,10 @@ class OldLocalFile extends LocalFile { $conds['oi_timestamp'] = $dbr->timestamp( $timestamp ); } - $row = $dbr->selectRow( 'oldimage', self::selectFields(), $conds, __METHOD__ ); + $fileQuery = self::getQueryInfo(); + $row = $dbr->selectRow( + $fileQuery['tables'], $fileQuery['fields'], $conds, __METHOD__, [], $fileQuery['joins'] + ); if ( $row ) { return self::newFromRow( $row, $repo ); } else { @@ -103,8 +106,7 @@ class OldLocalFile extends LocalFile { /** * Fields in the oldimage table - * @todo Deprecate this in favor of a method that returns tables and joins - * as well, and use CommentStore::getJoin(). + * @deprecated since 1.31, use self::getQueryInfo() instead. * @return array */ static function selectFields() { @@ -127,6 +129,52 @@ class OldLocalFile extends LocalFile { ] + CommentStore::newKey( 'oi_description' )->getFields(); } + /** + * Return the tables, fields, and join conditions to be selected to create + * a new oldlocalfile object. + * @since 1.31 + * @param string[] $options + * - omit-lazy: Omit fields that are lazily cached. + * @return array With three keys: + * - tables: (string[]) to include in the `$table` to `IDatabase->select()` + * - fields: (string[]) to include in the `$vars` to `IDatabase->select()` + * - joins: (array) to include in the `$join_conds` to `IDatabase->select()` + */ + public static function getQueryInfo( array $options = [] ) { + $commentQuery = CommentStore::newKey( 'oi_description' )->getJoin(); + $ret = [ + 'tables' => [ 'oldimage' ] + $commentQuery['tables'], + 'fields' => [ + 'oi_name', + 'oi_archive_name', + 'oi_size', + 'oi_width', + 'oi_height', + 'oi_bits', + 'oi_media_type', + 'oi_major_mime', + 'oi_minor_mime', + 'oi_user', + 'oi_user_text', + 'oi_timestamp', + 'oi_deleted', + 'oi_sha1', + ] + $commentQuery['fields'], + 'joins' => $commentQuery['joins'], + ]; + + if ( in_array( 'omit-nonlazy', $options, true ) ) { + // Internal use only for getting only the lazy fields + $ret['fields'] = []; + } + if ( !in_array( 'omit-lazy', $options, true ) ) { + // Note: Keep this in sync with self::getLazyCacheFields() + $ret['fields'][] = 'oi_metadata'; + } + + return $ret; + } + /** * @param Title $title * @param FileRepo $repo @@ -188,8 +236,15 @@ class OldLocalFile extends LocalFile { } else { $conds['oi_timestamp'] = $dbr->timestamp( $this->requestedTime ); } - $row = $dbr->selectRow( 'oldimage', $this->getCacheFields( 'oi_' ), - $conds, __METHOD__, [ 'ORDER BY' => 'oi_timestamp DESC' ] ); + $fileQuery = static::getQueryInfo(); + $row = $dbr->selectRow( + $fileQuery['tables'], + $fileQuery['fields'], + $conds, + __METHOD__, + [ 'ORDER BY' => 'oi_timestamp DESC' ], + $fileQuery['joins'] + ); if ( $row ) { $this->loadFromRow( $row, 'oi_' ); } else { @@ -209,14 +264,27 @@ class OldLocalFile extends LocalFile { } else { $conds['oi_timestamp'] = $dbr->timestamp( $this->requestedTime ); } + $fileQuery = static::getQueryInfo( [ 'omit-nonlazy' ] ); // In theory the file could have just been renamed/deleted...oh well - $row = $dbr->selectRow( 'oldimage', $this->getLazyCacheFields( 'oi_' ), - $conds, __METHOD__, [ 'ORDER BY' => 'oi_timestamp DESC' ] ); + $row = $dbr->selectRow( + $fileQuery['tables'], + $fileQuery['fields'], + $conds, + __METHOD__, + [ 'ORDER BY' => 'oi_timestamp DESC' ], + $fileQuery['joins'] + ); if ( !$row ) { // fallback to master $dbr = $this->repo->getMasterDB(); - $row = $dbr->selectRow( 'oldimage', $this->getLazyCacheFields( 'oi_' ), - $conds, __METHOD__, [ 'ORDER BY' => 'oi_timestamp DESC' ] ); + $row = $dbr->selectRow( + $fileQuery['tables'], + $fileQuery['fields'], + $conds, + __METHOD__, + [ 'ORDER BY' => 'oi_timestamp DESC' ], + $fileQuery['joins'] + ); } if ( $row ) { @@ -228,11 +296,8 @@ class OldLocalFile extends LocalFile { } } - /** - * @param string $prefix - * @return array - */ - function getCacheFields( $prefix = 'img_' ) { + /** @inheritDoc */ + protected function getCacheFields( $prefix = 'img_' ) { $fields = parent::getCacheFields( $prefix ); $fields[] = $prefix . 'archive_name'; $fields[] = $prefix . 'deleted'; diff --git a/includes/jobqueue/jobs/CategoryMembershipChangeJob.php b/includes/jobqueue/jobs/CategoryMembershipChangeJob.php index 3907fc65ec..55c1367c23 100644 --- a/includes/jobqueue/jobs/CategoryMembershipChangeJob.php +++ b/includes/jobqueue/jobs/CategoryMembershipChangeJob.php @@ -115,16 +115,18 @@ class CategoryMembershipChangeJob extends Job { // Find revisions to this page made around and after this revision which lack category // notifications in recent changes. This lets jobs pick up were the last one left off. $encCutoff = $dbr->addQuotes( $dbr->timestamp( $cutoffUnix ) ); + $revQuery = Revision::getQueryInfo(); $res = $dbr->select( - 'revision', - Revision::selectFields(), + $revQuery['tables'], + $revQuery['fields'], [ 'rev_page' => $page->getId(), "rev_timestamp > $encCutoff" . " OR (rev_timestamp = $encCutoff AND rev_id > $lastRevId)" ], __METHOD__, - [ 'ORDER BY' => 'rev_timestamp ASC, rev_id ASC' ] + [ 'ORDER BY' => 'rev_timestamp ASC, rev_id ASC' ], + $revQuery['joins'] ); // Apply all category updates in revision timestamp order diff --git a/includes/jobqueue/jobs/RecentChangesUpdateJob.php b/includes/jobqueue/jobs/RecentChangesUpdateJob.php index 6f349d4447..a92ae96074 100644 --- a/includes/jobqueue/jobs/RecentChangesUpdateJob.php +++ b/includes/jobqueue/jobs/RecentChangesUpdateJob.php @@ -85,14 +85,17 @@ class RecentChangesUpdateJob extends Job { $factory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory(); $ticket = $factory->getEmptyTransactionTicket( __METHOD__ ); $cutoff = $dbw->timestamp( time() - $wgRCMaxAge ); + $rcQuery = RecentChange::getQueryInfo(); do { $rcIds = []; $rows = []; - $res = $dbw->select( 'recentchanges', - RecentChange::selectFields(), + $res = $dbw->select( + $rcQuery['tables'], + $rcQuery['fields'], [ 'rc_timestamp < ' . $dbw->addQuotes( $cutoff ) ], __METHOD__, - [ 'LIMIT' => $wgUpdateRowsPerQuery ] + [ 'LIMIT' => $wgUpdateRowsPerQuery ], + $rcQuery['joins'] ); foreach ( $res as $row ) { $rcIds[] = $row->rc_id; diff --git a/includes/libs/objectcache/WANObjectCache.php b/includes/libs/objectcache/WANObjectCache.php index 0531d7f709..c6aac9a0ed 100644 --- a/includes/libs/objectcache/WANObjectCache.php +++ b/includes/libs/objectcache/WANObjectCache.php @@ -1119,7 +1119,15 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { * $setOpts += Database::getCacheSetOptions( $dbr ); * * // Load the row for this file - * $row = $dbr->selectRow( 'file', File::selectFields(), [ 'id' => $id ], __METHOD__ ); + * $queryInfo = File::getQueryInfo(); + * $row = $dbr->selectRow( + * $queryInfo['tables'], + * $queryInfo['fields'], + * [ 'id' => $id ], + * __METHOD__, + * [], + * $queryInfo['joins'] + * ); * * return $row ? (array)$row : false; * }, @@ -1205,7 +1213,15 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { * * // Load the rows for these files * $rows = []; - * $res = $dbr->select( 'file', File::selectFields(), [ 'id' => $ids ], __METHOD__ ); + * $queryInfo = File::getQueryInfo(); + * $res = $dbr->select( + * $queryInfo['tables'], + * $queryInfo['fields'], + * [ 'id' => $ids ], + * __METHOD__, + * [], + * $queryInfo['joins'] + * ); * foreach ( $res as $row ) { * $rows[$row->id] = $row; * $mtime = wfTimestamp( TS_UNIX, $row->timestamp ); diff --git a/includes/logging/LogEntry.php b/includes/logging/LogEntry.php index 8b51932be7..bf35d78d25 100644 --- a/includes/logging/LogEntry.php +++ b/includes/logging/LogEntry.php @@ -383,7 +383,7 @@ class RCDatabaseLogEntry extends DatabaseLogEntry { public function getComment() { return CommentStore::newKey( 'rc_comment' ) - // Legacy because the row probably used RecentChange::selectFields() + // Legacy because the row may have used RecentChange::selectFields() ->getCommentLegacy( wfGetDB( DB_REPLICA ), $this->row )->text; } diff --git a/includes/page/PageArchive.php b/includes/page/PageArchive.php index af936cc730..209551b296 100644 --- a/includes/page/PageArchive.php +++ b/includes/page/PageArchive.php @@ -231,12 +231,14 @@ class PageArchive { } $dbr = wfGetDB( DB_REPLICA ); + $fileQuery = ArchivedFile::getQueryInfo(); return $dbr->select( - 'filearchive', - ArchivedFile::selectFields(), + $fileQuery['tables'], + $fileQuery['fields'], [ 'fa_name' => $this->title->getDBkey() ], __METHOD__, - [ 'ORDER BY' => 'fa_timestamp DESC' ] + [ 'ORDER BY' => 'fa_timestamp DESC' ], + $fileQuery['joins'] ); } @@ -249,34 +251,11 @@ class PageArchive { */ public function getRevision( $timestamp ) { $dbr = wfGetDB( DB_REPLICA ); - $commentQuery = CommentStore::newKey( 'ar_comment' )->getJoin(); - - $tables = [ 'archive' ] + $commentQuery['tables']; - - $fields = [ - 'ar_rev_id', - 'ar_text', - 'ar_user', - 'ar_user_text', - 'ar_timestamp', - 'ar_minor_edit', - 'ar_flags', - 'ar_text_id', - 'ar_deleted', - 'ar_len', - 'ar_sha1', - ] + $commentQuery['fields']; - - if ( $this->config->get( 'ContentHandlerUseDB' ) ) { - $fields[] = 'ar_content_format'; - $fields[] = 'ar_content_model'; - } - - $join_conds = [] + $commentQuery['joins']; + $arQuery = Revision::getArchiveQueryInfo(); $row = $dbr->selectRow( - $tables, - $fields, + $arQuery['tables'], + $arQuery['fields'], [ 'ar_namespace' => $this->title->getNamespace(), 'ar_title' => $this->title->getDBkey(), @@ -284,7 +263,7 @@ class PageArchive { ], __METHOD__, [], - $join_conds + $arQuery['joins'] ); if ( $row ) { diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php index 146c054cd3..946c7f062b 100644 --- a/includes/page/WikiPage.php +++ b/includes/page/WikiPage.php @@ -158,8 +158,11 @@ class WikiPage implements Page, IDBAccessObject { $from = self::convertSelectType( $from ); $db = wfGetDB( $from === self::READ_LATEST ? DB_MASTER : DB_REPLICA ); + $pageQuery = self::getQueryInfo(); $row = $db->selectRow( - 'page', self::selectFields(), [ 'page_id' => $id ], __METHOD__ ); + $pageQuery['tables'], $pageQuery['fields'], [ 'page_id' => $id ], __METHOD__, + [], $pageQuery['joins'] + ); if ( !$row ) { return null; } @@ -277,6 +280,7 @@ class WikiPage implements Page, IDBAccessObject { * Return the list of revision fields that should be selected to create * a new page. * + * @deprecated since 1.31, use self::getQueryInfo() instead. * @return array */ public static function selectFields() { @@ -307,6 +311,47 @@ class WikiPage implements Page, IDBAccessObject { return $fields; } + /** + * Return the tables, fields, and join conditions to be selected to create + * a new page object. + * @since 1.31 + * @return array With three keys: + * - tables: (string[]) to include in the `$table` to `IDatabase->select()` + * - fields: (string[]) to include in the `$vars` to `IDatabase->select()` + * - joins: (array) to include in the `$join_conds` to `IDatabase->select()` + */ + public static function getQueryInfo() { + global $wgContentHandlerUseDB, $wgPageLanguageUseDB; + + $ret = [ + 'tables' => [ 'page' ], + 'fields' => [ + 'page_id', + 'page_namespace', + 'page_title', + 'page_restrictions', + 'page_is_redirect', + 'page_is_new', + 'page_random', + 'page_touched', + 'page_links_updated', + 'page_latest', + 'page_len', + ], + 'joins' => [], + ]; + + if ( $wgContentHandlerUseDB ) { + $ret['fields'][] = 'page_content_model'; + } + + if ( $wgPageLanguageUseDB ) { + $ret['fields'][] = 'page_lang'; + } + + return $ret; + } + /** * Fetch a page record with the given conditions * @param IDatabase $dbr @@ -315,14 +360,23 @@ class WikiPage implements Page, IDBAccessObject { * @return object|bool Database result resource, or false on failure */ protected function pageData( $dbr, $conditions, $options = [] ) { - $fields = self::selectFields(); + $pageQuery = self::getQueryInfo(); // Avoid PHP 7.1 warning of passing $this by reference $wikiPage = $this; - Hooks::run( 'ArticlePageDataBefore', [ &$wikiPage, &$fields ] ); + Hooks::run( 'ArticlePageDataBefore', [ + &$wikiPage, &$pageQuery['fields'], &$pageQuery['tables'], &$pageQuery['joins'] + ] ); - $row = $dbr->selectRow( 'page', $fields, $conditions, __METHOD__, $options ); + $row = $dbr->selectRow( + $pageQuery['tables'], + $pageQuery['fields'], + $conditions, + __METHOD__, + $options, + $pageQuery['joins'] + ); Hooks::run( 'ArticlePageDataAfter', [ &$wikiPage, &$row ] ); @@ -2788,13 +2842,13 @@ class WikiPage implements Page, IDBAccessObject { $revCommentStore = new CommentStore( 'rev_comment' ); $arCommentStore = new CommentStore( 'ar_comment' ); - $fields = Revision::selectFields(); + $revQuery = Revision::getQueryInfo(); $bitfield = false; // Bitfields to further suppress the content if ( $suppress ) { $bitfield = Revision::SUPPRESSED_ALL; - $fields = array_diff( $fields, [ 'rev_deleted' ] ); + $revQuery['fields'] = array_diff( $revQuery['fields'], [ 'rev_deleted' ] ); } // For now, shunt the revision data into the archive table. @@ -2805,14 +2859,13 @@ class WikiPage implements Page, IDBAccessObject { // the rev_deleted field, which is reserved for this purpose. // Get all of the page revisions - $commentQuery = $revCommentStore->getJoin(); $res = $dbw->select( - [ 'revision' ] + $commentQuery['tables'], - $fields + $commentQuery['fields'], + $revQuery['tables'], + $revQuery['fields'], [ 'rev_page' => $id ], __METHOD__, 'FOR UPDATE', - $commentQuery['joins'] + $revQuery['joins'] ); // Build their equivalent archive rows diff --git a/includes/revisiondelete/RevDelArchiveList.php b/includes/revisiondelete/RevDelArchiveList.php index 9afaf404c8..4f66cdae55 100644 --- a/includes/revisiondelete/RevDelArchiveList.php +++ b/includes/revisiondelete/RevDelArchiveList.php @@ -43,14 +43,15 @@ class RevDelArchiveList extends RevDelRevisionList { $timestamps[] = $db->timestamp( $id ); } - $tables = [ 'archive' ]; - $fields = Revision::selectArchiveFields(); + $arQuery = Revision::getArchiveQueryInfo(); + $tables = $arQuery['tables']; + $fields = $arQuery['fields']; $conds = [ 'ar_namespace' => $this->title->getNamespace(), 'ar_title' => $this->title->getDBkey(), 'ar_timestamp' => $timestamps, ]; - $join_conds = []; + $join_conds = $arQuery['joins']; $options = [ 'ORDER BY' => 'ar_timestamp DESC' ]; ChangeTags::modifyDisplayQuery( diff --git a/includes/revisiondelete/RevDelArchivedFileList.php b/includes/revisiondelete/RevDelArchivedFileList.php index 1d80d8696c..1ed87263f1 100644 --- a/includes/revisiondelete/RevDelArchivedFileList.php +++ b/includes/revisiondelete/RevDelArchivedFileList.php @@ -40,15 +40,17 @@ class RevDelArchivedFileList extends RevDelFileList { public function doQuery( $db ) { $ids = array_map( 'intval', $this->ids ); + $fileQuery = ArchivedFile::getQueryInfo(); return $db->select( - 'filearchive', - ArchivedFile::selectFields(), + $fileQuery['tables'], + $fileQuery['fields'], [ 'fa_name' => $this->title->getDBkey(), 'fa_id' => $ids ], __METHOD__, - [ 'ORDER BY' => 'fa_id DESC' ] + [ 'ORDER BY' => 'fa_id DESC' ], + $fileQuery['joins'] ); } diff --git a/includes/revisiondelete/RevDelFileList.php b/includes/revisiondelete/RevDelFileList.php index 77cf976762..6a6b86c099 100644 --- a/includes/revisiondelete/RevDelFileList.php +++ b/includes/revisiondelete/RevDelFileList.php @@ -60,15 +60,17 @@ class RevDelFileList extends RevDelList { $archiveNames[] = $timestamp . '!' . $this->title->getDBkey(); } + $oiQuery = OldLocalFile::getQueryInfo(); return $db->select( - 'oldimage', - OldLocalFile::selectFields(), + $oiQuery['tables'], + $oiQuery['fields'], [ 'oi_name' => $this->title->getDBkey(), 'oi_archive_name' => $archiveNames ], __METHOD__, - [ 'ORDER BY' => 'oi_timestamp DESC' ] + [ 'ORDER BY' => 'oi_timestamp DESC' ], + $oiQuery['joins'] ); } diff --git a/includes/revisiondelete/RevDelRevisionList.php b/includes/revisiondelete/RevDelRevisionList.php index 1ea6a381b5..07362c422f 100644 --- a/includes/revisiondelete/RevDelRevisionList.php +++ b/includes/revisiondelete/RevDelRevisionList.php @@ -62,9 +62,10 @@ class RevDelRevisionList extends RevDelList { */ public function doQuery( $db ) { $ids = array_map( 'intval', $this->ids ); + $revQuery = Revision::getQueryInfo( [ 'page', 'user' ] ); $queryInfo = [ - 'tables' => [ 'revision', 'page', 'user' ], - 'fields' => array_merge( Revision::selectFields(), Revision::selectUserFields() ), + 'tables' => $revQuery['tables'], + 'fields' => $revQuery['fields'], 'conds' => [ 'rev_page' => $this->title->getArticleID(), 'rev_id' => $ids, @@ -73,10 +74,7 @@ class RevDelRevisionList extends RevDelList { 'ORDER BY' => 'rev_id DESC', 'USE INDEX' => [ 'revision' => 'PRIMARY' ] // workaround for MySQL bug (T104313) ], - 'join_conds' => [ - 'page' => Revision::pageJoinCond(), - 'user' => Revision::userJoinCond(), - ], + 'join_conds' => $revQuery['joins'], ]; ChangeTags::modifyDisplayQuery( $queryInfo['tables'], @@ -100,14 +98,15 @@ class RevDelRevisionList extends RevDelList { return $live; } + $arQuery = Revision::getArchiveQueryInfo(); $archiveQueryInfo = [ - 'tables' => [ 'archive' ], - 'fields' => Revision::selectArchiveFields(), + 'tables' => $arQuery['tables'], + 'fields' => $arQuery['fields'], 'conds' => [ 'ar_rev_id' => $ids, ], 'options' => [ 'ORDER BY' => 'ar_rev_id DESC' ], - 'join_conds' => [], + 'join_conds' => $arQuery['joins'], ]; ChangeTags::modifyDisplayQuery( diff --git a/includes/specialpage/ChangesListSpecialPage.php b/includes/specialpage/ChangesListSpecialPage.php index eab31bcdbc..5194983964 100644 --- a/includes/specialpage/ChangesListSpecialPage.php +++ b/includes/specialpage/ChangesListSpecialPage.php @@ -1412,8 +1412,10 @@ abstract class ChangesListSpecialPage extends SpecialPage { protected function doMainQuery( $tables, $fields, $conds, $query_options, $join_conds, FormOptions $opts ) { - $tables[] = 'recentchanges'; - $fields = array_merge( RecentChange::selectFields(), $fields ); + $rcQuery = RecentChange::getQueryInfo(); + $tables = array_merge( $tables, $rcQuery['tables'] ); + $fields = array_merge( $rcQuery['fields'], $fields ); + $join_conds = array_merge( $join_conds, $rcQuery['joins'] ); ChangeTags::modifyDisplayQuery( $tables, diff --git a/includes/specials/SpecialRecentchanges.php b/includes/specials/SpecialRecentchanges.php index c3ce78e301..dfa13b6cd7 100644 --- a/includes/specials/SpecialRecentchanges.php +++ b/includes/specials/SpecialRecentchanges.php @@ -288,8 +288,10 @@ class SpecialRecentChanges extends ChangesListSpecialPage { $dbr = $this->getDB(); $user = $this->getUser(); - $tables[] = 'recentchanges'; - $fields = array_merge( RecentChange::selectFields(), $fields ); + $rcQuery = RecentChange::getQueryInfo(); + $tables = array_merge( $tables, $rcQuery['tables'] ); + $fields = array_merge( $rcQuery['fields'], $fields ); + $join_conds = array_merge( $join_conds, $rcQuery['joins'] ); // JOIN on watchlist for users if ( $user->isLoggedIn() && $user->isAllowed( 'viewmywatchlist' ) ) { diff --git a/includes/specials/SpecialRecentchangeslinked.php b/includes/specials/SpecialRecentchangeslinked.php index a13af55de5..99880de786 100644 --- a/includes/specials/SpecialRecentchangeslinked.php +++ b/includes/specials/SpecialRecentchangeslinked.php @@ -84,8 +84,10 @@ class SpecialRecentChangesLinked extends SpecialRecentChanges { $ns = $title->getNamespace(); $dbkey = $title->getDBkey(); - $tables[] = 'recentchanges'; - $select = array_merge( RecentChange::selectFields(), $select ); + $rcQuery = RecentChange::getQueryInfo(); + $tables = array_merge( $tables, $rcQuery['tables'] ); + $select = array_merge( $rcQuery['fields'], $select ); + $join_conds = array_merge( $join_conds, $rcQuery['joins'] ); // left join with watchlist table to highlight watched rows $uid = $this->getUser()->getId(); diff --git a/includes/specials/SpecialWatchlist.php b/includes/specials/SpecialWatchlist.php index 921a6dd465..ff62e9e603 100644 --- a/includes/specials/SpecialWatchlist.php +++ b/includes/specials/SpecialWatchlist.php @@ -357,8 +357,9 @@ class SpecialWatchlist extends ChangesListSpecialPage { $dbr = $this->getDB(); $user = $this->getUser(); - $tables = array_merge( [ 'recentchanges', 'watchlist' ], $tables ); - $fields = array_merge( RecentChange::selectFields(), $fields ); + $rcQuery = RecentChange::getQueryInfo(); + $tables = array_merge( $tables, $rcQuery['tables'], [ 'watchlist' ] ); + $fields = array_merge( $rcQuery['fields'], $fields ); $join_conds = array_merge( [ @@ -371,6 +372,7 @@ class SpecialWatchlist extends ChangesListSpecialPage { ], ], ], + $rcQuery['joins'], $join_conds ); diff --git a/includes/specials/pagers/ContribsPager.php b/includes/specials/pagers/ContribsPager.php index 979460cf8a..5936423128 100644 --- a/includes/specials/pagers/ContribsPager.php +++ b/includes/specials/pagers/ContribsPager.php @@ -175,79 +175,25 @@ class ContribsPager extends RangeChronologicalPager { } function getQueryInfo() { - list( $tables, $index, $userCond, $join_cond ) = $this->getUserCond(); - - $user = $this->getUser(); - $conds = array_merge( $userCond, $this->getNamespaceCond() ); - - // Paranoia: avoid brute force searches (T19342) - if ( !$user->isAllowed( 'deletedhistory' ) ) { - $conds[] = $this->mDb->bitAnd( 'rev_deleted', Revision::DELETED_USER ) . ' = 0'; - } elseif ( !$user->isAllowedAny( 'suppressrevision', 'viewsuppressed' ) ) { - $conds[] = $this->mDb->bitAnd( 'rev_deleted', Revision::SUPPRESSED_USER ) . - ' != ' . Revision::SUPPRESSED_USER; - } - - # Don't include orphaned revisions - $join_cond['page'] = Revision::pageJoinCond(); - # Get the current user name for accounts - $join_cond['user'] = Revision::userJoinCond(); - - $options = []; - if ( $index ) { - $options['USE INDEX'] = [ 'revision' => $index ]; - } - + $revQuery = Revision::getQueryInfo( [ 'page', 'user' ] ); $queryInfo = [ - 'tables' => $tables, - 'fields' => array_merge( - Revision::selectFields(), - Revision::selectUserFields(), - [ 'page_namespace', 'page_title', 'page_is_new', - 'page_latest', 'page_is_redirect', 'page_len' ] - ), - 'conds' => $conds, - 'options' => $options, - 'join_conds' => $join_cond + 'tables' => $revQuery['tables'], + 'fields' => $revQuery['fields'], + 'conds' => [], + 'options' => [], + 'join_conds' => $revQuery['joins'], ]; - // For IPv6, we use ipc_rev_timestamp on ip_changes as the index field, - // which will be referenced when parsing the results of a query. - if ( self::isQueryableRange( $this->target ) ) { - $queryInfo['fields'][] = 'ipc_rev_timestamp'; - } - - ChangeTags::modifyDisplayQuery( - $queryInfo['tables'], - $queryInfo['fields'], - $queryInfo['conds'], - $queryInfo['join_conds'], - $queryInfo['options'], - $this->tagFilter - ); - - // Avoid PHP 7.1 warning from passing $this by reference - $pager = $this; - Hooks::run( 'ContribsPager::getQueryInfo', [ &$pager, &$queryInfo ] ); - - return $queryInfo; - } - - function getUserCond() { - $condition = []; - $join_conds = []; - $tables = [ 'revision', 'page', 'user' ]; - $index = false; if ( $this->contribs == 'newbie' ) { $max = $this->mDb->selectField( 'user', 'max(user_id)', false, __METHOD__ ); - $condition[] = 'rev_user >' . (int)( $max - $max / 100 ); + $queryInfo['conds'][] = 'rev_user >' . (int)( $max - $max / 100 ); # ignore local groups with the bot right # @todo FIXME: Global groups may have 'bot' rights $groupsWithBotPermission = User::getGroupsWithPermission( 'bot' ); if ( count( $groupsWithBotPermission ) ) { - $tables[] = 'user_groups'; - $condition[] = 'ug_group IS NULL'; - $join_conds['user_groups'] = [ + $queryInfo['tables'][] = 'user_groups'; + $queryInfo['conds'][] = 'ug_group IS NULL'; + $queryInfo['join_conds']['user_groups'] = [ 'LEFT JOIN', [ 'ug_user = rev_user', 'ug_group' => $groupsWithBotPermission, @@ -259,46 +205,76 @@ class ContribsPager extends RangeChronologicalPager { // (T140537) Disallow looking too far in the past for 'newbies' queries. If the user requested // a timestamp offset far in the past such that there are no edits by users with user_ids in // the range, we would end up scanning all revisions from that offset until start of time. - $condition[] = 'rev_timestamp > ' . + $queryInfo['conds'][] = 'rev_timestamp > ' . $this->mDb->addQuotes( $this->mDb->timestamp( wfTimestamp() - 30 * 24 * 60 * 60 ) ); } else { $uid = User::idFromName( $this->target ); if ( $uid ) { - $condition['rev_user'] = $uid; - $index = 'user_timestamp'; + $queryInfo['conds']['rev_user'] = $uid; + $queryInfo['options']['USE INDEX']['revision'] = 'user_timestamp'; } else { $ipRangeConds = $this->getIpRangeConds( $this->mDb, $this->target ); if ( $ipRangeConds ) { - $tables[] = 'ip_changes'; - $join_conds['ip_changes'] = [ + $queryInfo['tables'][] = 'ip_changes'; + $queryInfo['join_conds']['ip_changes'] = [ 'LEFT JOIN', [ 'ipc_rev_id = rev_id' ] ]; - $condition[] = $ipRangeConds; + $queryInfo['conds'][] = $ipRangeConds; } else { - $condition['rev_user_text'] = $this->target; - $index = 'usertext_timestamp'; + $queryInfo['conds']['rev_user_text'] = $this->target; + $queryInfo['options']['USE INDEX']['revision'] = 'usertext_timestamp'; } } } if ( $this->deletedOnly ) { - $condition[] = 'rev_deleted != 0'; + $queryInfo['conds'][] = 'rev_deleted != 0'; } if ( $this->topOnly ) { - $condition[] = 'rev_id = page_latest'; + $queryInfo['conds'][] = 'rev_id = page_latest'; } if ( $this->newOnly ) { - $condition[] = 'rev_parent_id = 0'; + $queryInfo['conds'][] = 'rev_parent_id = 0'; } if ( $this->hideMinor ) { - $condition[] = 'rev_minor_edit = 0'; + $queryInfo['conds'][] = 'rev_minor_edit = 0'; + } + + $user = $this->getUser(); + $queryInfo['conds'] = array_merge( $queryInfo['conds'], $this->getNamespaceCond() ); + + // Paranoia: avoid brute force searches (T19342) + if ( !$user->isAllowed( 'deletedhistory' ) ) { + $queryInfo['conds'][] = $this->mDb->bitAnd( 'rev_deleted', Revision::DELETED_USER ) . ' = 0'; + } elseif ( !$user->isAllowedAny( 'suppressrevision', 'viewsuppressed' ) ) { + $queryInfo['conds'][] = $this->mDb->bitAnd( 'rev_deleted', Revision::SUPPRESSED_USER ) . + ' != ' . Revision::SUPPRESSED_USER; } - return [ $tables, $index, $condition, $join_conds ]; + // For IPv6, we use ipc_rev_timestamp on ip_changes as the index field, + // which will be referenced when parsing the results of a query. + if ( self::isQueryableRange( $this->target ) ) { + $queryInfo['fields'][] = 'ipc_rev_timestamp'; + } + + ChangeTags::modifyDisplayQuery( + $queryInfo['tables'], + $queryInfo['fields'], + $queryInfo['conds'], + $queryInfo['join_conds'], + $queryInfo['options'], + $this->tagFilter + ); + + // Avoid PHP 7.1 warning from passing $this by reference + $pager = $this; + Hooks::run( 'ContribsPager::getQueryInfo', [ &$pager, &$queryInfo ] ); + + return $queryInfo; } function getNamespaceCond() { diff --git a/includes/specials/pagers/MergeHistoryPager.php b/includes/specials/pagers/MergeHistoryPager.php index bbf97e13cb..6a8f7da74e 100644 --- a/includes/specials/pagers/MergeHistoryPager.php +++ b/includes/specials/pagers/MergeHistoryPager.php @@ -85,13 +85,12 @@ class MergeHistoryPager extends ReverseChronologicalPager { $conds['rev_page'] = $this->articleID; $conds[] = "rev_timestamp < " . $this->mDb->addQuotes( $this->maxTimestamp ); + $revQuery = Revision::getQueryInfo( [ 'page', 'user' ] ); return [ - 'tables' => [ 'revision', 'page', 'user' ], - 'fields' => array_merge( Revision::selectFields(), Revision::selectUserFields() ), + 'tables' => $revQuery['tables'], + 'fields' => $revQuery['fields'], 'conds' => $conds, - 'join_conds' => [ - 'page' => Revision::pageJoinCond(), - 'user' => Revision::userJoinCond() ] + 'join_conds' => $revQuery['joins'] ]; } diff --git a/includes/user/PasswordReset.php b/includes/user/PasswordReset.php index dd16fb78ba..faf09eefef 100644 --- a/includes/user/PasswordReset.php +++ b/includes/user/PasswordReset.php @@ -288,11 +288,14 @@ class PasswordReset implements LoggerAwareInterface { * @throws MWException On unexpected database errors */ protected function getUsersByEmail( $email ) { + $userQuery = User::getQueryInfo(); $res = wfGetDB( DB_REPLICA )->select( - 'user', - User::selectFields(), + $userQuery['tables'], + $userQuery['fields'], [ 'user_email' => $email ], - __METHOD__ + __METHOD__, + [], + $userQuery['joins'] ); if ( !$res ) { diff --git a/includes/user/User.php b/includes/user/User.php index 1c894a0f34..d397962ded 100644 --- a/includes/user/User.php +++ b/includes/user/User.php @@ -688,20 +688,25 @@ class User implements IDBAccessObject { } $dbr = wfGetDB( DB_REPLICA ); + $userQuery = self::getQueryInfo(); $row = $dbr->selectRow( - 'user', - self::selectFields(), + $userQuery['tables'], + $userQuery['fields'], [ 'user_name' => $name ], - __METHOD__ + __METHOD__, + [], + $userQuery['joins'] ); if ( !$row ) { // Try the master database... $dbw = wfGetDB( DB_MASTER ); $row = $dbw->selectRow( - 'user', - self::selectFields(), + $userQuery['tables'], + $userQuery['fields'], [ 'user_name' => $name ], - __METHOD__ + __METHOD__, + [], + $userQuery['joins'] ); } @@ -1278,12 +1283,14 @@ class User implements IDBAccessObject { list( $index, $options ) = DBAccessObjectUtils::getDBOptions( $flags ); $db = wfGetDB( $index ); + $userQuery = self::getQueryInfo(); $s = $db->selectRow( - 'user', - self::selectFields(), + $userQuery['tables'], + $userQuery['fields'], [ 'user_id' => $this->mId ], __METHOD__, - $options + $options, + $userQuery['joins'] ); $this->queryFlagsUsed = $flags; @@ -5497,6 +5504,7 @@ class User implements IDBAccessObject { /** * Return the list of user fields that should be selected to create * a new user object. + * @deprecated since 1.31, use self::getQueryInfo() instead. * @return array */ public static function selectFields() { @@ -5515,6 +5523,35 @@ class User implements IDBAccessObject { ]; } + /** + * Return the tables, fields, and join conditions to be selected to create + * a new user object. + * @since 1.31 + * @return array With three keys: + * - tables: (string[]) to include in the `$table` to `IDatabase->select()` + * - fields: (string[]) to include in the `$vars` to `IDatabase->select()` + * - joins: (array) to include in the `$join_conds` to `IDatabase->select()` + */ + public static function getQueryInfo() { + return [ + 'tables' => [ 'user' ], + 'fields' => [ + 'user_id', + 'user_name', + 'user_real_name', + 'user_email', + 'user_touched', + 'user_token', + 'user_email_authenticated', + 'user_email_token', + 'user_email_token_expires', + 'user_registration', + 'user_editcount', + ], + 'joins' => [], + ]; + } + /** * Factory function for fatal permission-denied errors * diff --git a/includes/user/UserArray.php b/includes/user/UserArray.php index ab6683b297..f3a7f9f236 100644 --- a/includes/user/UserArray.php +++ b/includes/user/UserArray.php @@ -49,11 +49,14 @@ abstract class UserArray implements Iterator { return new ArrayIterator( [] ); } $dbr = wfGetDB( DB_REPLICA ); + $userQuery = User::getQueryInfo(); $res = $dbr->select( - 'user', - User::selectFields(), + $userQuery['tables'], + $userQuery['fields'], [ 'user_id' => array_unique( $ids ) ], - __METHOD__ + __METHOD__, + [], + $userQuery['joins'] ); return self::newFromResult( $res ); } @@ -70,11 +73,14 @@ abstract class UserArray implements Iterator { return new ArrayIterator( [] ); } $dbr = wfGetDB( DB_REPLICA ); + $userQuery = User::getQueryInfo(); $res = $dbr->select( - 'user', - User::selectFields(), + $userQuery['tables'], + $userQuery['fields'], [ 'user_name' => array_unique( $names ) ], - __METHOD__ + __METHOD__, + [], + $userQuery['joins'] ); return self::newFromResult( $res ); } diff --git a/maintenance/checkImages.php b/maintenance/checkImages.php index 2df0a0954c..c2f0b2713b 100644 --- a/maintenance/checkImages.php +++ b/maintenance/checkImages.php @@ -43,10 +43,11 @@ class CheckImages extends Maintenance { $numGood = 0; $repo = RepoGroup::singleton()->getLocalRepo(); + $fileQuery = LocalFile::getQueryInfo(); do { - $res = $dbr->select( 'image', LocalFile::selectFields(), + $res = $dbr->select( $fileQuery['tables'], $fileQuery['fields'], [ 'img_name > ' . $dbr->addQuotes( $start ) ], - __METHOD__, [ 'LIMIT' => $this->mBatchSize ] ); + __METHOD__, [ 'LIMIT' => $this->mBatchSize ], $fileQuery['joins'] ); foreach ( $res as $row ) { $numImages++; $start = $row->img_name; diff --git a/maintenance/cleanupBlocks.php b/maintenance/cleanupBlocks.php index f489333f8f..37417c73d1 100644 --- a/maintenance/cleanupBlocks.php +++ b/maintenance/cleanupBlocks.php @@ -39,6 +39,7 @@ class CleanupBlocks extends Maintenance { public function execute() { $db = $this->getDB( DB_MASTER ); + $blockQuery = Block::getQueryInfo(); $max = $db->selectField( 'ipblocks', 'MAX(ipb_user)' ); @@ -65,11 +66,14 @@ class CleanupBlocks extends Maintenance { foreach ( $res as $row ) { $bestBlock = null; $res2 = $db->select( - 'ipblocks', - Block::selectFields(), + $blockQuery['tables'], + $blockQuery['fields'], [ 'ipb_user' => $row->ipb_user, - ] + ], + __METHOD__, + [], + $blockQuery['joins'] ); foreach ( $res2 as $row2 ) { $block = Block::newFromRow( $row2 ); diff --git a/maintenance/eraseArchivedFile.php b/maintenance/eraseArchivedFile.php index 05fbbbcb49..d94d49b72b 100644 --- a/maintenance/eraseArchivedFile.php +++ b/maintenance/eraseArchivedFile.php @@ -55,9 +55,10 @@ class EraseArchivedFile extends Maintenance { $afile = false; } else { // specified version $dbw = $this->getDB( DB_MASTER ); - $row = $dbw->selectRow( 'filearchive', ArchivedFile::selectFields(), + $fileQuery = ArchivedFile::getQueryInfo(); + $row = $dbw->selectRow( $fileQuery['tables'], $fileQuery['fields'], [ 'fa_storage_group' => 'deleted', 'fa_storage_key' => $filekey ], - __METHOD__ ); + __METHOD__, [], $fileQuery['joins'] ); if ( !$row ) { $this->error( "No deleted file exists with key '$filekey'.", 1 ); } @@ -85,9 +86,10 @@ class EraseArchivedFile extends Maintenance { protected function scrubAllVersions( $name ) { $dbw = $this->getDB( DB_MASTER ); - $res = $dbw->select( 'filearchive', ArchivedFile::selectFields(), + $fileQuery = ArchivedFile::getQueryInfo(); + $res = $dbw->select( $fileQuery['tables'], $fileQuery['fields'], [ 'fa_name' => $name, 'fa_storage_group' => 'deleted' ], - __METHOD__ ); + __METHOD__, [], $fileQuery['joins'] ); foreach ( $res as $row ) { $this->scrubVersion( ArchivedFile::newFromRow( $row ) ); } diff --git a/maintenance/populateRevisionLength.php b/maintenance/populateRevisionLength.php index a9457c2a1c..0cb14c42c8 100644 --- a/maintenance/populateRevisionLength.php +++ b/maintenance/populateRevisionLength.php @@ -54,10 +54,10 @@ class PopulateRevisionLength extends LoggedUpdateMaintenance { } $this->output( "Populating rev_len column\n" ); - $rev = $this->doLenUpdates( 'revision', 'rev_id', 'rev', Revision::selectFields() ); + $rev = $this->doLenUpdates( 'revision', 'rev_id', 'rev', Revision::getQueryInfo() ); $this->output( "Populating ar_len column\n" ); - $ar = $this->doLenUpdates( 'archive', 'ar_id', 'ar', Revision::selectArchiveFields() ); + $ar = $this->doLenUpdates( 'archive', 'ar_id', 'ar', Revision::getArchiveQueryInfo() ); $this->output( "rev_len and ar_len population complete " . "[$rev revision rows, $ar archive rows].\n" ); @@ -69,10 +69,10 @@ class PopulateRevisionLength extends LoggedUpdateMaintenance { * @param string $table * @param string $idCol * @param string $prefix - * @param array $fields + * @param array $queryInfo * @return int */ - protected function doLenUpdates( $table, $idCol, $prefix, $fields ) { + protected function doLenUpdates( $table, $idCol, $prefix, $queryInfo ) { $dbr = $this->getDB( DB_REPLICA ); $dbw = $this->getDB( DB_MASTER ); $start = $dbw->selectField( $table, "MIN($idCol)", false, __METHOD__ ); @@ -91,14 +91,16 @@ class PopulateRevisionLength extends LoggedUpdateMaintenance { while ( $blockStart <= $end ) { $this->output( "...doing $idCol from $blockStart to $blockEnd\n" ); $res = $dbr->select( - $table, - $fields, + $queryInfo['tables'], + $queryInfo['fields'], [ "$idCol >= $blockStart", "$idCol <= $blockEnd", "{$prefix}_len IS NULL" ], - __METHOD__ + __METHOD__, + [], + $queryInfo['joins'] ); if ( $res->numRows() > 0 ) { diff --git a/maintenance/populateRevisionSha1.php b/maintenance/populateRevisionSha1.php index c06f1e85df..f96c2eca36 100644 --- a/maintenance/populateRevisionSha1.php +++ b/maintenance/populateRevisionSha1.php @@ -55,10 +55,10 @@ class PopulateRevisionSha1 extends LoggedUpdateMaintenance { } $this->output( "Populating rev_sha1 column\n" ); - $rc = $this->doSha1Updates( 'revision', 'rev_id', Revision::selectFields(), 'rev' ); + $rc = $this->doSha1Updates( 'revision', 'rev_id', Revision::getQueryInfo(), 'rev' ); $this->output( "Populating ar_sha1 column\n" ); - $ac = $this->doSha1Updates( 'archive', 'ar_rev_id', Revision::selectArchiveFields(), 'ar' ); + $ac = $this->doSha1Updates( 'archive', 'ar_rev_id', Revision::getArchiveQueryInfo(), 'ar' ); $this->output( "Populating ar_sha1 column legacy rows\n" ); $ac += $this->doSha1LegacyUpdates(); @@ -71,10 +71,11 @@ class PopulateRevisionSha1 extends LoggedUpdateMaintenance { /** * @param string $table * @param string $idCol + * @param array $queryInfo * @param string $prefix * @return int Rows changed */ - protected function doSha1Updates( $table, $idCol, $fields, $prefix ) { + protected function doSha1Updates( $table, $idCol, $queryInfo, $prefix ) { $db = $this->getDB( DB_MASTER ); $start = $db->selectField( $table, "MIN($idCol)", false, __METHOD__ ); $end = $db->selectField( $table, "MAX($idCol)", false, __METHOD__ ); @@ -93,7 +94,9 @@ class PopulateRevisionSha1 extends LoggedUpdateMaintenance { $this->output( "...doing $idCol from $blockStart to $blockEnd\n" ); $cond = "$idCol BETWEEN $blockStart AND $blockEnd AND $idCol IS NOT NULL AND {$prefix}_sha1 = ''"; - $res = $db->select( $table, $fields, $cond, __METHOD__ ); + $res = $db->select( + $queryInfo['tables'], $queryInfo['fields'], $cond, __METHOD__, [], $queryInfo['joins'] + ); $this->beginTransaction( $db, __METHOD__ ); foreach ( $res as $row ) { @@ -117,8 +120,9 @@ class PopulateRevisionSha1 extends LoggedUpdateMaintenance { protected function doSha1LegacyUpdates() { $count = 0; $db = $this->getDB( DB_MASTER ); - $res = $db->select( 'archive', Revision::selectArchiveFields(), - [ 'ar_rev_id IS NULL', 'ar_sha1' => '' ], __METHOD__ ); + $arQuery = Revision::getArchiveQueryInfo(); + $res = $db->select( $arQuery['tables'], $arQuery['fields'], + [ 'ar_rev_id IS NULL', 'ar_sha1' => '' ], __METHOD__, [], $arQuery['joins'] ); $updateSize = 0; $this->beginTransaction( $db, __METHOD__ ); diff --git a/maintenance/rebuildImages.php b/maintenance/rebuildImages.php index a8fb9a3bd0..8fceedb28c 100644 --- a/maintenance/rebuildImages.php +++ b/maintenance/rebuildImages.php @@ -125,12 +125,14 @@ class ImageBuilder extends Maintenance { flush(); } - function buildTable( $table, $key, $fields, $callback ) { + function buildTable( $table, $key, $queryInfo, $callback ) { $count = $this->dbw->selectField( $table, 'count(*)', '', __METHOD__ ); $this->init( $count, $table ); $this->output( "Processing $table...\n" ); - $result = $this->getDB( DB_REPLICA )->select( $table, $fields, [], __METHOD__ ); + $result = $this->getDB( DB_REPLICA )->select( + $queryInfo['tables'], $queryInfo['fields'], [], __METHOD__, [], $queryInfo['joins'] + ); foreach ( $result as $row ) { $update = call_user_func( $callback, $row, null ); @@ -145,7 +147,7 @@ class ImageBuilder extends Maintenance { function buildImage() { $callback = [ $this, 'imageCallback' ]; - $this->buildTable( 'image', 'img_name', LocalFile::selectFields(), $callback ); + $this->buildTable( 'image', 'img_name', LocalFile::getQueryInfo(), $callback ); } function imageCallback( $row, $copy ) { @@ -157,7 +159,7 @@ class ImageBuilder extends Maintenance { } function buildOldImage() { - $this->buildTable( 'oldimage', 'oi_archive_name', OldLocalFile::selectFields(), + $this->buildTable( 'oldimage', 'oi_archive_name', OldLocalFile::getQueryInfo(), [ $this, 'oldimageCallback' ] ); } diff --git a/maintenance/rebuildtextindex.php b/maintenance/rebuildtextindex.php index c786925440..5971d5e9f7 100644 --- a/maintenance/rebuildtextindex.php +++ b/maintenance/rebuildtextindex.php @@ -93,11 +93,7 @@ class RebuildTextIndex extends Maintenance { $this->output( "Rebuilding index fields for {$count} pages...\n" ); $n = 0; - $fields = array_merge( - Revision::selectPageFields(), - Revision::selectFields(), - Revision::selectTextFields() - ); + $revQuery = Revision::getQueryInfo( [ 'page', 'text' ] ); while ( $n < $count ) { if ( $n ) { @@ -105,7 +101,7 @@ class RebuildTextIndex extends Maintenance { } $end = $n + self::RTI_CHUNK_SIZE - 1; - $res = $this->db->select( [ 'page', 'revision', 'text' ], $fields, + $res = $this->db->select( $revQuery['tables'], $revQuery['fields'], [ "page_id BETWEEN $n AND $end", 'page_latest = rev_id', 'rev_text_id = old_id' ], __METHOD__ ); diff --git a/maintenance/refreshFileHeaders.php b/maintenance/refreshFileHeaders.php index 16702350f6..e123de7fdb 100644 --- a/maintenance/refreshFileHeaders.php +++ b/maintenance/refreshFileHeaders.php @@ -57,6 +57,8 @@ class RefreshFileHeaders extends Maintenance { $count = 0; $dbr = $this->getDB( DB_REPLICA ); + $fileQuery = LocalFile::getQueryInfo(); + do { $conds = [ "img_name > {$dbr->addQuotes( $start )}" ]; @@ -76,8 +78,9 @@ class RefreshFileHeaders extends Maintenance { $conds[] = "img_minor_mime = {$dbr->addQuotes( $minor_mime )}"; } - $res = $dbr->select( 'image', LocalFile::selectFields(), $conds, - __METHOD__, [ 'LIMIT' => $this->mBatchSize, 'ORDER BY' => 'img_name ASC' ] ); + $res = $dbr->select( $fileQuery['tables'], $fileQuery['fields'], $conds, + __METHOD__, [ 'LIMIT' => $this->mBatchSize, 'ORDER BY' => 'img_name ASC' ], $fileQuery['joins'] + ); if ( $res->numRows() > 0 ) { $row1 = $res->current(); diff --git a/maintenance/refreshImageMetadata.php b/maintenance/refreshImageMetadata.php index 41da6b46e4..b7f03d9bb6 100644 --- a/maintenance/refreshImageMetadata.php +++ b/maintenance/refreshImageMetadata.php @@ -124,13 +124,16 @@ class RefreshImageMetadata extends Maintenance { 'ORDER BY' => 'img_name ASC', ]; + $fileQuery = LocalFile::getQueryInfo(); + do { $res = $dbw->select( - 'image', - LocalFile::selectFields(), + $fileQuery['tables'], + $fileQuery['fields'], array_merge( $conds, $conds2 ), __METHOD__, - $options + $options, + $fileQuery['joins'] ); if ( $res->numRows() > 0 ) { diff --git a/maintenance/storage/testCompression.php b/maintenance/storage/testCompression.php index deb2ca60d5..028f11cc6d 100644 --- a/maintenance/storage/testCompression.php +++ b/maintenance/storage/testCompression.php @@ -48,20 +48,18 @@ if ( isset( $options['limit'] ) ) { $type = isset( $options['type'] ) ? $options['type'] : 'ConcatenatedGzipHistoryBlob'; $dbr = $this->getDB( DB_REPLICA ); +$revQuery = Revision::getQueryInfo( [ 'page', 'text' ] ); $res = $dbr->select( - [ 'page', 'revision', 'text' ], - array_merge( - Revision::selectFields(), - Revision::selectPageFields(), - Revision::selectTextFields() - ), + $revQuery['tables'], + $revQuery['fields'], [ 'page_namespace' => $title->getNamespace(), 'page_title' => $title->getDBkey(), - 'page_id=rev_page', 'rev_timestamp > ' . $dbr->addQuotes( $dbr->timestamp( $start ) ), - 'rev_text_id=old_id' - ], __FILE__, [ 'LIMIT' => $limit ] + ], + __FILE__, + [ 'LIMIT' => $limit ], + $revQuery['joins'] ); $blob = new $type; diff --git a/tests/phpunit/includes/RevisionIntegrationTest.php b/tests/phpunit/includes/RevisionIntegrationTest.php index 10186edf28..96ce7660c4 100644 --- a/tests/phpunit/includes/RevisionIntegrationTest.php +++ b/tests/phpunit/includes/RevisionIntegrationTest.php @@ -265,7 +265,9 @@ class RevisionIntegrationTest extends MediaWikiTestCase { $orig = $this->makeRevisionWithProps(); $dbr = wfGetDB( DB_REPLICA ); - $res = $dbr->select( 'revision', Revision::selectFields(), [ 'rev_id' => $orig->getId() ] ); + $revQuery = Revision::getQueryInfo(); + $res = $dbr->select( $revQuery['tables'], $revQuery['fields'], [ 'rev_id' => $orig->getId() ], + __METHOD__, [], $revQuery['joins'] ); $this->assertTrue( is_object( $res ), 'query failed' ); $row = $res->fetchObject(); @@ -333,9 +335,11 @@ class RevisionIntegrationTest extends MediaWikiTestCase { $page->doDeleteArticle( 'test Revision::newFromArchiveRow' ); $dbr = wfGetDB( DB_REPLICA ); - $selectFields = $selectModifier( Revision::selectArchiveFields() ); + $arQuery = Revision::getArchiveQueryInfo(); + $arQuery['fields'] = $selectModifier( $arQuery['fields'] ); $res = $dbr->select( - 'archive', $selectFields, [ 'ar_rev_id' => $orig->getId() ] + $arQuery['tables'], $arQuery['fields'], [ 'ar_rev_id' => $orig->getId() ], + __METHOD__, [], $arQuery['joins'] ); $this->assertTrue( is_object( $res ), 'query failed' ); @@ -360,8 +364,10 @@ class RevisionIntegrationTest extends MediaWikiTestCase { $page->doDeleteArticle( 'test Revision::newFromArchiveRow' ); $dbr = wfGetDB( DB_REPLICA ); + $arQuery = Revision::getArchiveQueryInfo(); $res = $dbr->select( - 'archive', Revision::selectArchiveFields(), [ 'ar_rev_id' => $orig->getId() ] + $arQuery['tables'], $arQuery['fields'], [ 'ar_rev_id' => $orig->getId() ], + __METHOD__, [], $arQuery['joins'] ); $this->assertTrue( is_object( $res ), 'query failed' ); diff --git a/tests/phpunit/includes/RevisionUnitTest.php b/tests/phpunit/includes/RevisionUnitTest.php index 53725bd173..7b8d316f47 100644 --- a/tests/phpunit/includes/RevisionUnitTest.php +++ b/tests/phpunit/includes/RevisionUnitTest.php @@ -361,178 +361,6 @@ class RevisionUnitTest extends MediaWikiTestCase { Revision::getRevisionText( $row ), "getRevisionText" ); } - /** - * @covers Revision::userJoinCond - */ - public function testUserJoinCond() { - $this->assertEquals( - [ 'LEFT JOIN', [ 'rev_user != 0', 'user_id = rev_user' ] ], - Revision::userJoinCond() - ); - } - - /** - * @covers Revision::pageJoinCond - */ - public function testPageJoinCond() { - $this->assertEquals( - [ 'INNER JOIN', [ 'page_id = rev_page' ] ], - Revision::pageJoinCond() - ); - } - - public function provideSelectFields() { - yield [ - true, - [ - 'rev_id', - 'rev_page', - 'rev_text_id', - 'rev_timestamp', - 'rev_user_text', - 'rev_user', - 'rev_minor_edit', - 'rev_deleted', - 'rev_len', - 'rev_parent_id', - 'rev_sha1', - 'rev_comment_text' => 'rev_comment', - 'rev_comment_data' => 'NULL', - 'rev_comment_cid' => 'NULL', - 'rev_content_format', - 'rev_content_model', - ] - ]; - yield [ - false, - [ - 'rev_id', - 'rev_page', - 'rev_text_id', - 'rev_timestamp', - 'rev_user_text', - 'rev_user', - 'rev_minor_edit', - 'rev_deleted', - 'rev_len', - 'rev_parent_id', - 'rev_sha1', - 'rev_comment_text' => 'rev_comment', - 'rev_comment_data' => 'NULL', - 'rev_comment_cid' => 'NULL', - ] - ]; - } - - /** - * @dataProvider provideSelectFields - * @covers Revision::selectFields - * @todo a true unit test would mock CommentStore - */ - public function testSelectFields( $contentHandlerUseDB, $expected ) { - $this->setMwGlobals( 'wgContentHandlerUseDB', $contentHandlerUseDB ); - $this->assertEquals( $expected, Revision::selectFields() ); - } - - public function provideSelectArchiveFields() { - yield [ - true, - [ - 'ar_id', - 'ar_page_id', - 'ar_rev_id', - 'ar_text', - 'ar_text_id', - 'ar_timestamp', - 'ar_user_text', - 'ar_user', - 'ar_minor_edit', - 'ar_deleted', - 'ar_len', - 'ar_parent_id', - 'ar_sha1', - 'ar_comment_text' => 'ar_comment', - 'ar_comment_data' => 'NULL', - 'ar_comment_cid' => 'NULL', - 'ar_content_format', - 'ar_content_model', - ] - ]; - yield [ - false, - [ - 'ar_id', - 'ar_page_id', - 'ar_rev_id', - 'ar_text', - 'ar_text_id', - 'ar_timestamp', - 'ar_user_text', - 'ar_user', - 'ar_minor_edit', - 'ar_deleted', - 'ar_len', - 'ar_parent_id', - 'ar_sha1', - 'ar_comment_text' => 'ar_comment', - 'ar_comment_data' => 'NULL', - 'ar_comment_cid' => 'NULL', - ] - ]; - } - - /** - * @dataProvider provideSelectArchiveFields - * @covers Revision::selectArchiveFields - * @todo a true unit test would mock CommentStore - */ - public function testSelectArchiveFields( $contentHandlerUseDB, $expected ) { - $this->setMwGlobals( 'wgContentHandlerUseDB', $contentHandlerUseDB ); - $this->assertEquals( $expected, Revision::selectArchiveFields() ); - } - - /** - * @covers Revision::selectTextFields - */ - public function testSelectTextFields() { - $this->assertEquals( - [ - 'old_text', - 'old_flags', - ], - Revision::selectTextFields() - ); - } - - /** - * @covers Revision::selectPageFields - */ - public function testSelectPageFields() { - $this->assertEquals( - [ - 'page_namespace', - 'page_title', - 'page_id', - 'page_latest', - 'page_is_redirect', - 'page_len', - ], - Revision::selectPageFields() - ); - } - - /** - * @covers Revision::selectUserFields - */ - public function testSelectUserFields() { - $this->assertEquals( - [ - 'user_name', - ], - Revision::selectUserFields() - ); - } - public function provideFetchFromConds() { yield [ 0, [] ]; yield [ Revision::READ_LOCKING, [ 'FOR UPDATE' ] ];