Changing lines like this: "extract( $dbw->tableNames( 'page', 'archive' ) );" to be like this: "list ($page, $archive) = $dbw->tableNamesN( 'page', 'archive' );".

Three reasons for this:
1) It's better for analysis tools [which want explicit variable declaration]
2) It's easier for a human to read, as it's completely explicit where the variables came from [which is something you don't get with extract() ]
3) It makes it easier to find everywhere where a variable is used with search/grep [which you can't currently do with $tbl_page variables from things like: "extract($db->tableNames( 'page', 'revision'), EXTR_PREFIX_ALL, 'tbl');"].

Otherwise, from a functionality/efficiency perspective the two forms should be identical.

By doing this have been able run static analysis over the usages of these variables, thus eliminating 5 unneeded table names from calls, plus removing 3 unused calls entirely, and it just feels subjectively slightly nicer to me.
This commit is contained in:
Nick Jenkins 2006-11-27 08:36:57 +00:00
parent f9ca2e9672
commit a474761d9a
33 changed files with 60 additions and 41 deletions

View file

@ -308,7 +308,7 @@ class Block
$now = wfTimestampNow();
extract( $db->tableNames( 'ipblocks', 'user' ) );
list( $ipblocks, $user ) = $db->tableNamesN( 'ipblocks', 'user' );
$sql = "SELECT $ipblocks.*,user_name FROM $ipblocks,$user " .
"WHERE user_id=ipb_by $cond ORDER BY ipb_timestamp DESC $options";

View file

@ -1383,7 +1383,7 @@ class Database {
* $sql = "SELECT wl_namespace,wl_title FROM $watchlist,$user
* WHERE wl_user=user_id AND wl_user=$nameWithQuotes";
*/
function tableNames() {
public function tableNames() {
$inArray = func_get_args();
$retVal = array();
foreach ( $inArray as $name ) {
@ -1391,6 +1391,24 @@ class Database {
}
return $retVal;
}
/**
* @desc: Fetch a number of table names into an zero-indexed numerical array
* This is handy when you need to construct SQL for joins
*
* Example:
* list( $user, $watchlist ) = $dbr->tableNames('user','watchlist');
* $sql = "SELECT wl_namespace,wl_title FROM $watchlist,$user
* WHERE wl_user=user_id AND wl_user=$nameWithQuotes";
*/
public function tableNamesN() {
$inArray = func_get_args();
$retVal = array();
foreach ( $inArray as $name ) {
$retVal[] = $this->tableName( $name );
}
return $retVal;
}
/**
* @private

View file

@ -1702,7 +1702,7 @@ class Image
}
$linkCache =& LinkCache::singleton();
extract( $db->tableNames( 'page', 'imagelinks' ) );
list( $page, $imagelinks ) = $db->tableNamesN( 'page', 'imagelinks' );
$encName = $db->addQuotes( $this->name );
$sql = "SELECT page_namespace,page_title,page_id FROM $page,$imagelinks WHERE page_id=il_from AND il_to=$encName $options";
$res = $db->query( $sql, __METHOD__ );

View file

@ -127,7 +127,7 @@ class SiteStatsUpdate {
# Update schema if required
if ( $row->ss_total_pages == -1 && !$this->mViews ) {
$dbr =& wfGetDB( DB_SLAVE, array( 'SpecialStatistics', 'vslow') );
extract( $dbr->tableNames( 'page', 'user' ) );
list( $page, $user ) = $dbr->tableNamesN( 'page', 'user' );
$sql = "SELECT COUNT(page_namespace) AS total FROM $page";
$res = $dbr->query( $sql, $fname );

View file

@ -1034,7 +1034,7 @@ END;
if ($wgPageShowWatchingUsers && $wgUser->getOption( 'shownumberswatching' )) {
$dbr =& wfGetDB( DB_SLAVE );
extract( $dbr->tableNames( 'watchlist' ) );
$watchlist = $dbr->tableName( 'watchlist' );
$sql = "SELECT COUNT(*) AS n FROM $watchlist
WHERE wl_title='" . $dbr->strencode($wgTitle->getDBKey()) .
"' AND wl_namespace=" . $wgTitle->getNamespace() ;

View file

@ -54,6 +54,7 @@ class MediaWiki_I18N {
$value = wfMsg( $value );
// interpolate variables
$m = array();
while (preg_match('/\$([0-9]*?)/sm', $value, $m)) {
list($src, $var) = $m;
wfSuppressWarnings();
@ -344,7 +345,7 @@ class SkinTemplate extends Skin {
if ($wgPageShowWatchingUsers) {
$dbr =& wfGetDB( DB_SLAVE );
extract( $dbr->tableNames( 'watchlist' ) );
$watchlist = $dbr->tableName( 'watchlist' );
$sql = "SELECT COUNT(*) AS n FROM $watchlist
WHERE wl_title='" . $dbr->strencode($this->mTitle->getDBKey()) .
"' AND wl_namespace=" . $this->mTitle->getNamespace() ;

View file

@ -27,7 +27,7 @@ class BrokenRedirectsPage extends PageQueryPage {
function getSQL() {
$dbr =& wfGetDB( DB_SLAVE );
extract( $dbr->tableNames( 'page', 'pagelinks' ) );
list( $page, $pagelinks ) = $dbr->tableNamesN( 'page', 'pagelinks' );
$sql = "SELECT 'BrokenRedirects' AS type,
p1.page_namespace AS namespace,

View file

@ -31,7 +31,7 @@ class ContribsFinder {
list( $index, $usercond ) = $this->getUserCond();
$nscond = $this->getNamespaceCond();
$use_index = $this->dbr->useIndexClause( $index );
extract( $this->dbr->tableNames( 'revision', 'page' ) );
list( $revision, $page) = $this->dbr->tableNamesN( 'revision', 'page' );
$sql = "SELECT rev_timestamp " .
" FROM $page,$revision $use_index " .
" WHERE rev_page=page_id AND $usercond $nscond" .
@ -82,7 +82,7 @@ class ContribsFinder {
$nscond = $this->getNamespaceCond();
$use_index = $this->dbr->useIndexClause( $index );
extract( $this->dbr->tableNames( 'page', 'revision' ) );
list( $page, $revision ) = $this->dbr->tableNamesN( 'page', 'revision' );
$sql = "SELECT rev_timestamp FROM $page, $revision $use_index " .
"WHERE page_id = rev_page AND rev_timestamp > '" . $this->offset . "' AND " .
@ -106,7 +106,7 @@ class ContribsFinder {
function getFirstOffsetForPaging() {
list( $index, $usercond ) = $this->getUserCond();
$use_index = $this->dbr->useIndexClause( $index );
extract( $this->dbr->tableNames( 'page', 'revision' ) );
list( $page, $revision ) = $this->dbr->tableNamesN( 'page', 'revision' );
$nscond = $this->getNamespaceCond();
$sql = "SELECT rev_timestamp FROM $page, $revision $use_index " .
"WHERE page_id = rev_page AND " .
@ -128,9 +128,9 @@ class ContribsFinder {
}
/* private */ function makeSql() {
$userCond = $condition = $index = $offsetQuery = '';
$offsetQuery = '';
extract( $this->dbr->tableNames( 'page', 'revision' ) );
list( $page, $revision ) = $this->dbr->tableNamesN( 'page', 'revision' );
list( $index, $userCond ) = $this->getUserCond();
if ( $this->offset )

View file

@ -43,7 +43,7 @@ class DeadendPagesPage extends PageQueryPage {
*/
function getSQL() {
$dbr =& wfGetDB( DB_SLAVE );
extract( $dbr->tableNames( 'page', 'pagelinks' ) );
list( $page, $pagelinks ) = $dbr->tableNamesN( 'page', 'pagelinks' );
return "SELECT 'Deadendpages' as type, page_namespace AS namespace, page_title as title, page_title AS value " .
"FROM $page LEFT JOIN $pagelinks ON page_id = pl_from " .
"WHERE pl_from IS NULL " .

View file

@ -26,7 +26,7 @@ class DoubleRedirectsPage extends PageQueryPage {
function getSQLText( &$dbr, $namespace = null, $title = null ) {
extract( $dbr->tableNames( 'page', 'pagelinks' ) );
list( $page, $pagelinks ) = $dbr->tableNamesN( 'page', 'pagelinks' );
$limitToTitle = !( $namespace === null && $title === null );
$sql = $limitToTitle ? "SELECT" : "SELECT 'DoubleRedirects' as type," ;

View file

@ -30,7 +30,7 @@ class LonelyPagesPage extends PageQueryPage {
function getSQL() {
$dbr =& wfGetDB( DB_SLAVE );
extract( $dbr->tableNames( 'page', 'pagelinks' ) );
list( $page, $pagelinks ) = $dbr->tableNamesN( 'page', 'pagelinks' );
return
"SELECT 'Lonelypages' AS type,

View file

@ -20,7 +20,7 @@ class MostcategoriesPage extends QueryPage {
function getSQL() {
$dbr =& wfGetDB( DB_SLAVE );
extract( $dbr->tableNames( 'categorylinks', 'page' ) );
list( $categorylinks, $page) = $dbr->tableNamesN( 'categorylinks', 'page' );
return
"
SELECT

View file

@ -20,7 +20,7 @@ class MostimagesPage extends QueryPage {
function getSQL() {
$dbr =& wfGetDB( DB_SLAVE );
extract( $dbr->tableNames( 'imagelinks' ) );
$imagelinks = $dbr->tableName( 'imagelinks' );
return
"
SELECT

View file

@ -28,7 +28,7 @@ class MostlinkedPage extends QueryPage {
*/
function getSQL() {
$dbr =& wfGetDB( DB_SLAVE );
extract( $dbr->tableNames( 'pagelinks', 'page' ) );
list( $pagelinks, $page ) = $dbr->tableNamesN( 'pagelinks', 'page' );
return
"SELECT 'Mostlinked' AS type,
pl_namespace AS namespace,

View file

@ -22,7 +22,7 @@ class MostlinkedCategoriesPage extends QueryPage {
function getSQL() {
$dbr =& wfGetDB( DB_SLAVE );
extract( $dbr->tableNames( 'categorylinks', 'page' ) );
$categorylinks = $dbr->tableName( 'categorylinks' );
$name = $dbr->addQuotes( $this->getName() );
return
"

View file

@ -22,7 +22,7 @@ class MostrevisionsPage extends QueryPage {
function getSQL() {
$dbr =& wfGetDB( DB_SLAVE );
extract( $dbr->tableNames( 'revision', 'page' ) );
list( $revision, $page ) = $dbr->tableNamesN( 'revision', 'page' );
return
"
SELECT

View file

@ -42,7 +42,7 @@ class NewPagesPage extends QueryPage {
global $wgUser, $wgUseRCPatrol;
$usepatrol = ( $wgUseRCPatrol && $wgUser->isAllowed( 'patrol' ) ) ? 1 : 0;
$dbr =& wfGetDB( DB_SLAVE );
extract( $dbr->tableNames( 'recentchanges', 'page', 'text' ) );
list( $recentchanges, $page ) = $dbr->tableNamesN( 'recentchanges', 'page' );
$uwhere = $this->makeUserWhere( $dbr );
@ -172,6 +172,7 @@ function wfSpecialNewpages($par, $specialPage) {
if ( is_numeric( $bit ) )
$limit = $bit;
$m = array();
if ( preg_match( '/^limit=(\d+)$/', $bit, $m ) )
$limit = intval($m[1]);
if ( preg_match( '/^offset=(\d+)$/', $bit, $m ) )

View file

@ -108,7 +108,7 @@ function wfSpecialRecentchanges( $par, $specialPage ) {
# Database connection and caching
$dbr =& wfGetDB( DB_SLAVE );
extract( $dbr->tableNames( 'recentchanges', 'watchlist' ) );
list( $recentchanges, $watchlist ) = $dbr->tableNamesN( 'recentchanges', 'watchlist' );
$cutoff_unixtime = time() - ( $days * 86400 );

View file

@ -67,7 +67,8 @@ function wfSpecialRecentchangeslinked( $par = NULL ) {
$cmq = 'AND rc_minor=0';
} else { $cmq = ''; }
extract( $dbr->tableNames( 'recentchanges', 'categorylinks', 'pagelinks', 'revision', 'page' , "watchlist" ) );
list($recentchanges, $categorylinks, $pagelinks, $watchlist) =
$dbr->tableNamesN( 'recentchanges', 'categorylinks', 'pagelinks', "watchlist" );
$uid = $wgUser->getID();

View file

@ -15,7 +15,6 @@ function wfSpecialStatistics() {
$action = $wgRequest->getVal( 'action' );
$dbr =& wfGetDB( DB_SLAVE );
extract( $dbr->tableNames( 'site_stats', 'user', 'user_groups' ) );
$views = SiteStats::views();
$edits = SiteStats::edits();
@ -59,6 +58,7 @@ function wfSpecialStatistics() {
global $wgDisableCounters, $wgMiserMode, $wgUser, $wgLang, $wgContLang;
if( !$wgDisableCounters && !$wgMiserMode ) {
$page = $dbr->tableName( 'page' );
$sql = "SELECT page_namespace, page_title, page_counter FROM {$page} WHERE page_is_redirect = 0 AND page_counter > 0 ORDER BY page_counter DESC";
$sql = $dbr->limitResult($sql, 10, 0);
$res = $dbr->query( $sql, $fname );

View file

@ -28,7 +28,7 @@ class UncategorizedImagesPage extends QueryPage {
function getSQL() {
$dbr =& wfGetDB( DB_SLAVE );
extract( $dbr->tableNames( 'page', 'categorylinks' ) );
list( $page, $categorylinks ) = $dbr->tableNamesN( 'page', 'categorylinks' );
$ns = NS_IMAGE;
return "SELECT 'Uncategorizedimages' AS type, page_namespace AS namespace,

View file

@ -28,7 +28,7 @@ class UncategorizedPagesPage extends PageQueryPage {
function getSQL() {
$dbr =& wfGetDB( DB_SLAVE );
extract( $dbr->tableNames( 'page', 'categorylinks' ) );
list( $page, $categorylinks ) = $dbr->tableNamesN( 'page', 'categorylinks' );
$name = $dbr->addQuotes( $this->getName() );
return

View file

@ -278,12 +278,12 @@ class PageArchive {
* @return int number of revisions restored
*/
private function undeleteRevisions( $timestamps ) {
global $wgParser, $wgDBtype;
global $wgDBtype;
$restoreAll = empty( $timestamps );
$dbw =& wfGetDB( DB_MASTER );
extract( $dbw->tableNames( 'page', 'archive' ) );
$page = $dbw->tableName( 'archive' );
# Does this page already exist? We'll have to update it...
$article = new Article( $this->title );
@ -453,6 +453,7 @@ class UndeleteForm {
$timestamps = array();
$this->mFileVersions = array();
foreach( $_REQUEST as $key => $val ) {
$matches = array();
if( preg_match( '/^ts(\d{14})$/', $key, $matches ) ) {
array_push( $timestamps, $matches[1] );
}

View file

@ -23,7 +23,7 @@ class UnusedCategoriesPage extends QueryPage {
function getSQL() {
$NScat = NS_CATEGORY;
$dbr =& wfGetDB( DB_SLAVE );
extract( $dbr->tableNames( 'categorylinks','page' ));
list( $categorylinks, $page ) = $dbr->tableNamesN( 'categorylinks', 'page' );
return "SELECT 'Unusedcategories' as type,
{$NScat} as namespace, page_title as title, 1 as value
FROM $page

View file

@ -25,7 +25,7 @@ class UnusedimagesPage extends QueryPage {
$dbr =& wfGetDB( DB_SLAVE );
if ( $wgCountCategorizedImagesAsUsed ) {
extract( $dbr->tableNames( 'page', 'image', 'imagelinks', 'categorylinks' ) );
list( $page, $image, $imagelinks, $categorylinks ) = $dbr->tableNamesN( 'page', 'image', 'imagelinks', 'categorylinks' );
return 'SELECT img_name as title, img_user, img_user_text, img_timestamp as value, img_description
FROM ((('.$page.' AS I LEFT JOIN '.$categorylinks.' AS L ON I.page_id = L.cl_from)
@ -33,7 +33,7 @@ class UnusedimagesPage extends QueryPage {
INNER JOIN '.$image.' AS G ON I.page_title = G.img_name)
WHERE I.page_namespace = '.NS_IMAGE.' AND L.cl_from IS NULL AND P.il_to IS NULL';
} else {
extract( $dbr->tableNames( 'image','imagelinks' ) );
list( $image, $imagelinks ) = $dbr->tableNamesN( 'image','imagelinks' );
return 'SELECT img_name as title, img_user, img_user_text, img_timestamp as value, img_description' .
' FROM '.$image.' LEFT JOIN '.$imagelinks.' ON img_name=il_to WHERE il_to IS NULL ';

View file

@ -23,7 +23,7 @@ class UnusedtemplatesPage extends QueryPage {
function getSQL() {
$dbr =& wfGetDB( DB_SLAVE );
extract( $dbr->tableNames( 'page', 'templatelinks' ) );
list( $page, $templatelinks) = $dbr->tableNamesN( 'page', 'templatelinks' );
$sql = "SELECT 'Unusedtemplates' AS type, page_title AS title,
page_namespace AS namespace, 0 AS value
FROM $page

View file

@ -22,7 +22,7 @@ class UnwatchedpagesPage extends QueryPage {
function getSQL() {
$dbr =& wfGetDB( DB_SLAVE );
extract( $dbr->tableNames( 'page', 'watchlist' ) );
list( $page, $watchlist ) = $dbr->tableNamesN( 'page', 'watchlist' );
$mwns = NS_MEDIAWIKI;
return
"

View file

@ -22,7 +22,7 @@ class WantedCategoriesPage extends QueryPage {
function getSQL() {
$dbr =& wfGetDB( DB_SLAVE );
extract( $dbr->tableNames( 'categorylinks', 'page' ) );
list( $categorylinks, $page ) = $dbr->tableNamesN( 'categorylinks', 'page' );
$name = $dbr->addQuotes( $this->getName() );
return
"

View file

@ -113,7 +113,7 @@ function wfSpecialWatchlist( $par ) {
}
$dbr =& wfGetDB( DB_SLAVE );
extract( $dbr->tableNames( 'page', 'revision', 'watchlist', 'recentchanges' ) );
list( $page, $watchlist, $recentchanges ) = $dbr->tableNamesN( 'page', 'watchlist', 'recentchanges' );
$sql = "SELECT COUNT(*) AS n FROM $watchlist WHERE wl_user=$uid";
$res = $dbr->query( $sql, $fname );

View file

@ -76,8 +76,6 @@ class WhatLinksHerePage {
$dbr =& wfGetDB( DB_READ );
extract( $dbr->tableNames( 'pagelinks', 'templatelinks', 'page' ) );
// Some extra validation
$from = intval( $from );
if ( !$from && $dir == 'prev' ) {

View file

@ -248,7 +248,6 @@ class EmailNotification {
}
if( $userCondition ) {
$dbr =& wfGetDB( DB_MASTER );
extract( $dbr->tableNames( 'watchlist' ) );
$res = $dbr->select( 'watchlist', array( 'wl_user' ),
array(

View file

@ -41,7 +41,7 @@ class ApiQueryLogEvents extends ApiQueryBase {
$db = & $this->getDB();
extract($db->tableNames('logging', 'page', 'user'), EXTR_PREFIX_ALL, 'tbl');
list($tbl_logging, $tbl_page, $tbl_user) = $db->tableNamesN('logging', 'page', 'user');
$this->addOption('STRAIGHT_JOIN');
$this->addTables("$tbl_logging LEFT OUTER JOIN $tbl_page ON " .

View file

@ -54,8 +54,8 @@ class ApiQueryContributions extends ApiQueryBase {
if (!$userid)
$this->dieUsage("User name $user not found", 'param_user');
//Extract the table names, in case we have a prefix
extract($db->tableNames( 'page', 'revision'), EXTR_PREFIX_ALL, 'tbl');
//Get the table names
list ($tbl_page, $tbl_revision) = $db->tableNamesN('page', 'revision');
//We're after the revision table, and the corresponding page row for
//anything we retrieve.