r65311 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r65310‎ | r65311 | r65312 >
Date:13:49, 20 April 2010
Author:reedy
Status:deferred
Tags:
Comment:
querypage-work2: Merge r49084 skip r49527 (already in trunk)
Modified paths:
  • /branches/querypage-work2 (modified) (history)
  • /branches/querypage-work2/phase3 (modified) (history)
  • /branches/querypage-work2/phase3/includes (modified) (history)
  • /branches/querypage-work2/phase3/includes/ChangesList.php (modified) (history)
  • /branches/querypage-work2/phase3/includes/ConfEditor.php (modified) (history)
  • /branches/querypage-work2/phase3/includes/OutputPage.php (modified) (history)
  • /branches/querypage-work2/phase3/includes/QueryPage.php (modified) (history)
  • /branches/querypage-work2/phase3/includes/api (modified) (history)
  • /branches/querypage-work2/phase3/includes/specials (modified) (history)
  • /branches/querypage-work2/phase3/includes/specials/SpecialAncientpages.php (modified) (history)
  • /branches/querypage-work2/phase3/includes/specials/SpecialBrokenRedirects.php (modified) (history)
  • /branches/querypage-work2/phase3/includes/specials/SpecialDeadendpages.php (modified) (history)
  • /branches/querypage-work2/phase3/maintenance/cleanupTable.inc (modified) (history)
  • /branches/querypage-work2/phase3/skins/common/jquery.js (modified) (history)
  • /branches/querypage-work2/phase3/skins/common/jquery.min.js (modified) (history)

Diff [purge]

Property changes on: branches/querypage-work2/phase3/maintenance/cleanupTable.inc
___________________________________________________________________
Name: svn:mergeinfo
11 - /branches/REL1_15/phase3/maintenance/cleanupTable.inc:51646
/branches/wmf-deployment/maintenance/cleanupTable.inc:56715
22 + /branches/REL1_15/phase3/maintenance/cleanupTable.inc:51646
/branches/querypage-work/phase3/maintenance/cleanupTable.inc:49084
/branches/wmf-deployment/maintenance/cleanupTable.inc:56715
Property changes on: branches/querypage-work2/phase3/skins/common/jquery.js
___________________________________________________________________
Name: svn:mergeinfo
33 -
44 + /branches/querypage-work/phase3/skins/common/jquery.js:49084
Property changes on: branches/querypage-work2/phase3/skins/common/jquery.min.js
___________________________________________________________________
Name: svn:mergeinfo
55 -
66 + /branches/querypage-work/phase3/skins/common/jquery.min.js:49084
Property changes on: branches/querypage-work2/phase3/includes/OutputPage.php
___________________________________________________________________
Name: svn:mergeinfo
77 - /branches/REL1_15/phase3/includes/OutputPage.php:51646
/branches/wmf-deployment/includes/OutputPage.php:53381,57468
88 + /branches/REL1_15/phase3/includes/OutputPage.php:51646
/branches/querypage-work/phase3/includes/OutputPage.php:49084
/branches/wmf-deployment/includes/OutputPage.php:53381,57468
Property changes on: branches/querypage-work2/phase3/includes/api
___________________________________________________________________
Name: svn:mergeinfo
99 - /branches/REL1_15/phase3/includes/api:51646
/branches/REL1_16/phase3/includes/api:63621-63636
/branches/sqlite/includes/api:58211-58321
/branches/wmf-deployment/includes/api:53381,59952
1010 + /branches/REL1_15/phase3/includes/api:51646
/branches/REL1_16/phase3/includes/api:63621-63636
/branches/querypage-work/phase3/includes/api:49084
/branches/sqlite/includes/api:58211-58321
/branches/wmf-deployment/includes/api:53381,59952
Property changes on: branches/querypage-work2/phase3/includes/ChangesList.php
___________________________________________________________________
Name: svn:mergeinfo
1111 - /branches/REL1_15/phase3/includes/ChangesList.php:51646
/branches/wmf-deployment/includes/ChangesList.php:53381,57589
1212 + /branches/REL1_15/phase3/includes/ChangesList.php:51646
/branches/querypage-work/phase3/includes/ChangesList.php:49084
/branches/wmf-deployment/includes/ChangesList.php:53381,57589
Property changes on: branches/querypage-work2/phase3/includes/ConfEditor.php
___________________________________________________________________
Name: svn:mergeinfo
1313 -
1414 + /branches/querypage-work/phase3/includes/ConfEditor.php:49084
Index: branches/querypage-work2/phase3/includes/specials/SpecialBrokenRedirects.php
——@@ -39,9 +39,33 @@
4040 AND p2.page_namespace IS NULL";
4141 return $sql;
4242 }
 43+
 44+ function getQueryInfo() {
 45+ return array(
 46+ 'tables' => array( 'redirect', 'page AS p1', 'page AS p2' ),
 47+ 'fields' => array( "'{$this->getName()}' AS type",
 48+ 'p1.page_namespace AS namespace',
 49+ 'p1.page_title AS title',
 50+ 'rd_namespace',
 51+ 'rd_title'
 52+ ),
 53+ 'conds' => array( 'rd_namespace >= 0',
 54+ 'p2.page_namespace IS NULL'
 55+ ),
 56+ // TODO test this join
 57+ 'join_conds' => array( 'page AS p1' => array( 'LEFT JOIN', array(
 58+ 'rd_from=p1.page_id',
 59+ ) ),
 60+ 'page AS p2' => array( 'LEFT JOIN', array(
 61+ 'rd_namespace=p2.page_namespace',
 62+ 'rd_title=p2.page_title'
 63+ ) )
 64+ )
 65+ );
 66+ }
4367
4468 function getOrder() {
45 - return '';
 69+ return array ();
4670 }
4771
4872 function formatResult( $skin, $result ) {
Index: branches/querypage-work2/phase3/includes/specials/SpecialAncientpages.php
——@@ -20,38 +20,23 @@
2121
2222 function isSyndicated() { return false; }
2323
24 - function getSQL() {
 24+ function getQueryInfo() {
 25+ // FIXME convert timestamps elsewhere
 26+ // Possibly add bool returnsTimestamps()
 27+ // FIXME standardize 'name' AS type ?
2528 global $wgDBtype;
26 - $db = wfGetDB( DB_SLAVE );
27 - $page = $db->tableName( 'page' );
28 - $revision = $db->tableName( 'revision' );
29 -
30 - switch ($wgDBtype) {
31 - case 'mysql':
32 - $epoch = 'UNIX_TIMESTAMP(rev_timestamp)';
33 - break;
34 - case 'ibm_db2':
35 - // TODO implement proper conversion to a Unix epoch
36 - $epoch = 'rev_timestamp';
37 - break;
38 - case 'oracle':
39 - $epoch = '((trunc(rev_timestamp) - to_date(\'19700101\',\'YYYYMMDD\')) * 86400)';
40 - break;
41 - case 'sqlite':
42 - $epoch = 'rev_timestamp';
43 - break;
44 - default:
45 - $epoch = 'EXTRACT(epoch FROM rev_timestamp)';
46 - }
47 -
48 - return
49 - "SELECT 'Ancientpages' as type,
50 - page_namespace as namespace,
51 - page_title as title,
52 - $epoch as value
53 - FROM $page, $revision
54 - WHERE page_namespace=".NS_MAIN." AND page_is_redirect=0
55 - AND page_latest=rev_id";
 29+ $epoch = $wgDBtype == 'mysql' ? 'UNIX_TIMESTAMP(rev_timestamp)' :
 30+ 'EXTRACT(epoch FROM rev_timestamp)';
 31+ return array(
 32+ 'tables' => array( 'page', 'revision' ),
 33+ 'fields' => array( "'{$this->getName()}' AS type",
 34+ 'page_namespace AS namespace',
 35+ 'page_title AS title',
 36+ "$epoch AS value" ),
 37+ 'conds' => array( 'page_namespace' => NS_MAIN,
 38+ 'page_is_redirect' => 0,
 39+ 'page_latest=rev_id' )
 40+ );
5641 }
5742
5843 function sortDescending() {
Index: branches/querypage-work2/phase3/includes/specials/SpecialDeadendpages.php
——@@ -47,6 +47,24 @@
4848 "AND page_namespace = 0 " .
4949 "AND page_is_redirect = 0";
5050 }
 51+
 52+ function getQueryInfo() {
 53+ return array(
 54+ 'tables' => array( 'page', 'pagelinks' ),
 55+ 'fields' => array( "'{$this->getName()} AS type",
 56+ 'page_namespace AS namespace',
 57+ 'page_title AS title',
 58+ 'page_title AS value'
 59+ ),
 60+ 'conds' => array( 'pl_from IS NULL',
 61+ 'page_namespace' => NS_MAIN,
 62+ 'page_is_redirect' => 0
 63+ ),
 64+ 'join_conds' => array( 'pagelinks' => array( 'LEFT JOIN', array(
 65+ 'page_id=pl_from'
 66+ ) ) )
 67+ );
 68+ }
5169 }
5270
5371 /**
Property changes on: branches/querypage-work2/phase3/includes/specials
___________________________________________________________________
Name: svn:mergeinfo
5472 - /branches/REL1_15/phase3/includes/specials:51646
/branches/sqlite/includes/specials:58211-58321
/branches/wmf-deployment/includes/specials:53381,56967
5573 + /branches/REL1_15/phase3/includes/specials:51646
/branches/querypage-work/phase3/includes/specials:49084
/branches/sqlite/includes/specials:58211-58321
/branches/wmf-deployment/includes/specials:53381,56967
Index: branches/querypage-work2/phase3/includes/QueryPage.php
——@@ -60,7 +60,7 @@
6161 * subclasses derive from it.
6262 * @ingroup SpecialPage
6363 */
64 -class QueryPage {
 64+abstract class QueryPage {
6565 /**
6666 * Whether or not we want plain listoutput rather than an ordered list
6767 *
——@@ -106,8 +106,14 @@
107107 }
108108
109109 /**
110 - * Subclasses return an SQL query here.
111 - *
 110+ * Subclasses return an SQL query here, formatted as an array with the
 111+ * following keys:
 112+ * tables => Table(s) for passing to Database::select()
 113+ * fields => Field(s) for passing to Database::select(), may be *
 114+ * conds => WHERE conditions
 115+ * options => options
 116+ * join_conds => JOIN conditions
 117+ *
112118 * Note that the query itself should return the following four columns:
113119 * 'type' (your special page's name), 'namespace', 'title', and 'value'
114120 * *in that order*. 'value' is used for sorting.
——@@ -118,10 +124,19 @@
119125 * an integer; non-numeric values are useful only for sorting the initial
120126 * query.
121127 *
122 - * Don't include an ORDER or LIMIT clause, this will be added.
 128+ * Don't include an ORDER or LIMIT clause, they will be added
 129+ * @return array
123130 */
124 - function getSQL() {
125 - return "SELECT 'sample' as type, 0 as namespace, 'Sample result' as title, 42 as value";
 131+ abstract function getQueryInfo();
 132+
 133+ /**
 134+ * Subclasses return an array of fields to order by here. Don't append
 135+ * DESC to the field names, that'll be done automatically if
 136+ * sortDescending() returns true
 137+ * @return array
 138+ */
 139+ function getOrderFields() {
 140+ return array('value');
126141 }
127142
128143 /**
——@@ -133,11 +148,6 @@
134149 return true;
135150 }
136151
137 - function getOrder() {
138 - return ' ORDER BY value ' .
139 - ($this->sortDescending() ? 'DESC' : '');
140 - }
141 -
142152 /**
143153 * Is this query expensive (for some definition of expensive)? Then we
144154 * don't let it run in miser mode. $wgDisableQueryPages causes all query
——@@ -151,7 +161,7 @@
152162 }
153163
154164 /**
155 - * Whether or not the output of the page in question is retrived from
 165+ * Whether or not the output of the page in question is retrieved from
156166 * the database cache.
157167 *
158168 * @return Boolean
——@@ -175,14 +185,15 @@
176186 * Formats the results of the query for display. The skin is the current
177187 * skin; you can use it for making links. The result is a single row of
178188 * result data. You should be able to grab SQL results off of it.
179 - * If the function return "false", the line output will be skipped.
 189+ * If the function returns false, the line output will be skipped.
 190+ * @param $skin Skin
 191+ * @param $result object Result row
 192+ * @return mixed String or false to skip
180193 *
181194 * @param $skin Skin object
182195 * @param $result Object: database row
183196 */
184 - function formatResult( $skin, $result ) {
185 - return '';
186 - }
 197+ abstract function formatResult( $skin, $result );
187198
188199 /**
189200 * The content returned by this function will be output before any result
——@@ -207,8 +218,9 @@
208219 /**
209220 * Some special pages (for example SpecialListusers) might not return the
210221 * current object formatted, but return the previous one instead.
211 - * Setting this to return true, will call one more time wfFormatResult to
212 - * be sure that the very last result is formatted and shown.
 222+ * Setting this to return true will ensure formatResult() is called
 223+ * one more time to make sure that the very last result is formatted
 224+ * as well.
213225 */
214226 function tryLastResult() {
215227 return false;
——@@ -228,8 +240,6 @@
229241 return false;
230242 }
231243
232 - $querycache = $dbr->tableName( 'querycache' );
233 -
234244 if ( $ignoreErrors ) {
235245 $ignoreW = $dbw->ignoreErrors( true );
236246 $ignoreR = $dbr->ignoreErrors( true );
——@@ -238,10 +248,7 @@
239249 # Clear out any old cached data
240250 $dbw->delete( 'querycache', array( 'qc_type' => $this->getName() ), $fname );
241251 # Do query
242 - $sql = $this->getSQL() . $this->getOrder();
243 - if ( $limit !== false )
244 - $sql = $dbr->limitResult( $sql, $limit, 0 );
245 - $res = $dbr->query( $sql, $fname );
 252+ $res = $this->reallyDoQuery( $limit, false );
246253 $num = false;
247254 if ( $res ) {
248255 $num = $dbr->numRows( $res );
——@@ -265,7 +272,7 @@
266273 if ( !$dbw->insert( 'querycache', $vals, __METHOD__ ) ) {
267274 // Set result to false to indicate error
268275 $dbr->freeResult( $res );
269 - $res = false;
 276+ $num = false;
270277 }
271278 }
272279 if ( $res ) {
——@@ -283,6 +290,68 @@
284291 }
285292 return $num;
286293 }
 294+
 295+ /**
 296+ * Run the query and return the result
 297+ * @param $limit mixed Numerical limit or false for no limit
 298+ * @param $offset mixed Numerical offset or false for no offset
 299+ * @return ResultWrapper
 300+ */
 301+ function reallyDoQuery( $limit, $offset = false ) {
 302+ $fname = get_class( $this ) . "::reallyDoQuery";
 303+ $query = $this->getQueryInfo();
 304+ $order = $this->getOrderFields();
 305+ if( $this->sortDescending() ) {
 306+ foreach( $order as &$field ) {
 307+ $field .= ' DESC';
 308+ }
 309+ }
 310+ if( !is_array( $query['options'] ) ) {
 311+ $options = array ();
 312+ }
 313+ if( count( $order ) ) {
 314+ $query['options']['ORDER BY'] = implode( ', ', $order );
 315+ }
 316+ if( $limit !== false) {
 317+ $query['options']['LIMIT'] = intval( $limit );
 318+ }
 319+ if( $offset !== false) {
 320+ $query['options']['OFFSET'] = intval( $offset );
 321+ }
 322+
 323+ $dbr = wfGetDB( DB_SLAVE );
 324+ $res = $dbr->select( (array)$query['tables'],
 325+ (array)$query['fields'],
 326+ (array)$query['conds'], $fname,
 327+ $query['options'], (array)$query['join_conds']
 328+ );
 329+ return $dbr->resultObject( $res );
 330+ }
 331+
 332+ /**
 333+ * Fetch the query results from the query cache
 334+ * @param $limit mixed Numerical limit or false for no limit
 335+ * @param $offset mixed Numerical offset or false for no offset
 336+ * @return ResultWrapper
 337+ */
 338+ function fetchFromCache( $limit, $offset = false ) {
 339+ $dbr = wfGetDB( DB_SLAVE );
 340+ $options = array ();
 341+ if( $limit !== false ) {
 342+ $options['LIMIT'] = intval( $limit );
 343+ }
 344+ if( $offset !== false) {
 345+ $options['OFFSET'] = intval( $offset );
 346+ }
 347+ $res = $dbr->select( 'querycache', array( 'qc_type',
 348+ 'qc_namespace AS namespace',
 349+ 'qc_title AS title',
 350+ 'qc_value AS value' ),
 351+ array( 'qc_type' => $this->getName() ),
 352+ __METHOD__, $options
 353+ );
 354+ return $dbr->resultObject( $res );
 355+ }
287356
288357 /**
289358 * This is the actual workhorse. It does everything needed to make a
——@@ -304,16 +373,12 @@
305374
306375 $wgOut->setSyndicated( $this->isSyndicated() );
307376
 377+ //$res = null;
308378 if ( !$this->isCached() ) {
309 - $sql = $this->getSQL();
 379+ $res = $this->reallyDoQuery( $limit, $offset );
310380 } else {
311381 # Get the cached result
312 - $querycache = $dbr->tableName( 'querycache' );
313 - $type = $dbr->strencode( $sname );
314 - $sql =
315 - "SELECT qc_type as type, qc_namespace as namespace,qc_title as title, qc_value as value
316 - FROM $querycache WHERE qc_type='$type'";
317 -
 382+ $res = $this->fetchFromCache( $limit, $offset );
318383 if( !$this->listoutput ) {
319384
320385 # Fetch the timestamp of this update
——@@ -342,9 +407,6 @@
343408
344409 }
345410
346 - $sql .= $this->getOrder();
347 - $sql = $dbr->limitResult($sql, $limit, $offset);
348 - $res = $dbr->query( $sql );
349411 $num = $dbr->numRows($res);
350412
351413 $this->preprocessResults( $dbr, $res );
——@@ -485,9 +547,7 @@
486548 $feed->outHeader();
487549
488550 $dbr = wfGetDB( DB_SLAVE );
489 - $sql = $this->getSQL() . $this->getOrder();
490 - $sql = $dbr->limitResult( $sql, $limit, 0 );
491 - $res = $dbr->query( $sql, 'QueryPage::doFeed' );
 551+ $res = $this->reallyDoQuery( $limit, 0 );
492552 while( $obj = $dbr->fetchObject( $res ) ) {
493553 $item = $this->feedResult( $obj );
494554 if( $item ) $feed->outItem( $item );
Property changes on: branches/querypage-work2/phase3/includes
___________________________________________________________________
Name: svn:mergeinfo
495555 - /branches/REL1_15/phase3/includes:51646
/branches/sqlite/includes:58211-58321
/branches/wmf-deployment/includes:53381
496556 + /branches/REL1_15/phase3/includes:51646
/branches/querypage-work/phase3/includes:49084
/branches/sqlite/includes:58211-58321
/branches/wmf-deployment/includes:53381
Property changes on: branches/querypage-work2/phase3
___________________________________________________________________
Name: svn:mergeinfo
497557 - /branches/REL1_15/phase3:51646
/branches/sqlite:58211-58321
498558 + /branches/REL1_15/phase3:51646
/branches/querypage-work/phase3:49084
/branches/sqlite:58211-58321
Property changes on: branches/querypage-work2
___________________________________________________________________
Name: svn:mergeinfo
499559 + /branches/querypage-work:49084

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r49084querypage-work: Commit initial workcatrope15:54, 31 March 2009
r49527querypage-work: Merge r49489 from trunkcatrope21:04, 15 April 2009

Status & tagging log