r89512 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r89511‎ | r89512 | r89513 >
Date:17:49, 5 June 2011
Author:reedy
Status:ok
Tags:
Comment:
* (bug 25734) API: possible issue with revids validation


It seems, on databases with loads of revision rows (ie enwiki, not testwiki), although the EXPLAIN is sane, it's doing something stupid (table scan? I've nfi). So, as any revision id's less than 0 aren't valid, just prefilter them from the database SQL query

mysql> DESCRIBE SELECT /* ApiPageSet::initFromRevIDs */ rev_id,rev_page FROM `revision`,`page` WHERE rev_id IN ('10','20','30','40','-50','60') AND (rev_page = page_id);
+----+-------------+----------+-------+-------------------------------+---------+---------+-----------------------+-------+--------------------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra |
+----+-------------+----------+-------+-------------------------------+---------+---------+-----------------------+-------+--------------------------+
| 1 | SIMPLE | page | index | PRIMARY | PRIMARY | 4 | NULL | 22982 | Using index |
| 1 | SIMPLE | revision | ref | PRIMARY,rev_id,page_timestamp | PRIMARY | 4 | testwiki.page.page_id | 1 | Using where; Using index |
+----+-------------+----------+-------+-------------------------------+---------+---------+-----------------------+-------+--------------------------+
Modified paths:
  • /trunk/phase3/RELEASE-NOTES-1.19 (modified) (history)
  • /trunk/phase3/includes/api/ApiPageSet.php (modified) (history)

Diff [purge]

Index: trunk/phase3/RELEASE-NOTES-1.19
@@ -96,6 +96,7 @@
9797 * (bug 29221) Expose oldrevid in watchlist output
9898 * (bug 29267) always give the servername for meta=siteinfo&siprop=dbrepllag
9999 * (bug 28897) rvparse doesn’t seem to work with rvsection
 100+* (bug 25734) API: possible issue with revids validation
100101
101102 === Languages updated in 1.19 ===
102103
Index: trunk/phase3/includes/api/ApiPageSet.php
@@ -535,6 +535,15 @@
536536 $pageids = array();
537537 $remaining = array_flip( $revids );
538538
 539+ // bug 25734 API: possible issue with revids validation
 540+ // It seems with a load of revision rows, MySQL gets upset
 541+ // Remove any < 0 revids, as they can't be valid
 542+ foreach( $revids as $i => $revid ) {
 543+ if ( $revid < 0 ) {
 544+ unset( $revids[$i] );
 545+ }
 546+ }
 547+
539548 $tables = array( 'revision', 'page' );
540549 $fields = array( 'rev_id', 'rev_page' );
541550 $where = array( 'rev_id' => $revids, 'rev_page = page_id' );

Follow-up revisions

RevisionCommit summaryAuthorDate
r89513Followup r89512...reedy18:01, 5 June 2011
r89514MFT r89512, r89513reedy18:05, 5 June 2011
r92339REL1_18 MFT r89401, r89451, r89512, r89513, r89523, r89529, r89532, r89549, r...reedy23:08, 15 July 2011

Status & tagging log