User "MZMcBride" posted a comment on MediaWiki.r95496.
Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/95496#c21649
Commit summary:
Revert r8811
Reverting followups r88117, 88252
Comment:
So there were undisclosed changes in some of the previous revisions that were
accidentally reverted here. Basically what needs to happen is...
<source lang="php" enclose="div">
$fields = array(
'page_namespace', 'page_title', 'page_is_new',
'page_latest', 'page_is_redirect',
'page_len','rev_id', 'rev_page', 'rev_text_id',
'rev_timestamp', 'rev_comment',
'rev_minor_edit', 'rev_user', 'rev_user_text',
'rev_parent_id', 'rev_deleted'
);
$queryInfo = array(
'tables' => $tables,
'fields' => array(
'page_namespace', 'page_title', 'page_is_new',
'page_latest', 'page_is_redirect',
'page_len','rev_id', 'rev_page', 'rev_text_id',
'rev_timestamp', 'rev_comment',
'rev_minor_edit', 'rev_user', 'rev_user_text',
'rev_parent_id', 'rev_deleted',
'rc_old_len', 'rc_new_len'
),
'conds' => $conds,
'options' => array( 'USE INDEX' => array('revision' =>
$index) ),
'join_conds' => $join_cond
);
</source>
This section needs to be made sane. You're defining $fields twice, once with
'rc_old_len' and 'rc_new_len' and once without. One possible solution:
<source lang="php" enclose="div">
$fields = array(
'page_namespace', 'page_title', 'page_is_new',
'page_latest', 'page_is_redirect',
'page_len','rev_id', 'rev_page', 'rev_text_id',
'rev_timestamp', 'rev_comment',
'rev_minor_edit', 'rev_user', 'rev_user_text',
'rev_parent_id', 'rev_deleted'
);
$queryInfo = array(
'tables' => $tables,
'fields' => $fields,
'conds' => $conds,
'options' => array( 'USE INDEX' => array('revision' =>
$index) ),
'join_conds' => $join_cond
);
</source>
Later in the file...
<source lang="php" enclose="div">
if( $this->target == 'newbies' ) {
$tables = array( 'recentchanges', 'user_groups',
'page', 'revision' );
$max = $this->mDb->selectField( 'user', 'max(user_id)',
false, __METHOD__ );
$condition[] = 'rev_user >' . (int)($max - $max / 100);
$condition[] = 'ug_group IS NULL';
$index = 'user_timestamp';
# @todo FIXME: Other groups may have 'bot' rights
$join_conds['user_groups'] = array( 'LEFT JOIN',
"ug_user = rev_user AND ug_group = 'bot'" );
$join_conds['recentchanges'] = array( 'INNER JOIN',
"rev_id = rc_this_oldid" );
} else {
$tables = array( 'recentchanges', 'page', 'revision' );
$condition['rev_user_text'] = $this->target;
$join_conds['recentchanges'] = array( 'INNER JOIN',
"rev_id = rc_this_oldid" );
$index = 'usertext_timestamp';
}
</source>
This is still doing an inner join on rev_id = rc_this_oldid. Don't do this.
This could be switched to something like...
<source lang="php" enclose="div">
if( $this->target == 'newbies' ) {
$tables = array( 'recentchanges', 'user_groups',
'page', 'revision' );
$max = $this->mDb->selectField( 'user', 'max(user_id)',
false, __METHOD__ );
$condition[] = 'rev_user >' . (int)($max - $max / 100);
$condition[] = 'ug_group IS NULL';
$index = 'user_timestamp';
# @todo FIXME: Other groups may have 'bot' rights
$join_conds['user_groups'] = array( 'LEFT JOIN',
"ug_user = rev_user AND ug_group = 'bot'" );
$join_conds['recentchanges'] = array( 'INNER JOIN',
"rev_id = rc_this_oldid" );
} else {
$tables = array( 'page', 'revision' );
$condition['rev_user_text'] = $this->target;
$index = 'usertext_timestamp';
}
</source>
And finally, this part needs a healthy evaluation:
<source lang="php" enclose="div">
$diffOut = ' . . ' . $wgLang->getDirMark() .
ChangesList::showCharacterDifference( $row->rc_old_len,
$row->rc_new_len );
</source>
Thanks to [[User:Fran Rogers|Fran Rogers]] for helping me debug this mess.
_______________________________________________
MediaWiki-CodeReview mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
