r23756 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r23755‎ | r23756 | r23757 >
Date:19:42, 5 July 2007
Author:tstarling
Status:old
Tags:
Comment:
Return a ResultWrapper from Database::query() and query builder functions, instead of a raw DB result resource. Backwards compatibility is maintained, except with naughty code that was calling database driver functions directly on result objects.
Modified paths:
  • /trunk/phase3/includes/Database.php (modified) (history)
  • /trunk/phase3/includes/DatabasePostgres.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/DatabasePostgres.php
——@@ -505,12 +505,18 @@
506506 }
507507
508508 function freeResult( $res ) {
 509+ if ( $res instanceof ResultWrapper ) {
 510+ $res = $res->result;
 511+ }
509512 if ( !@pg_free_result( $res ) ) {
510513 throw new DBUnexpectedError($this, "Unable to free Postgres result\n" );
511514 }
512515 }
513516
514517 function fetchObject( $res ) {
 518+ if ( $res instanceof ResultWrapper ) {
 519+ $res = $res->result;
 520+ }
515521 @$row = pg_fetch_object( $res );
516522 # FIXME: HACK HACK HACK HACK debug
517523
——@@ -524,6 +530,9 @@
525531 }
526532
527533 function fetchRow( $res ) {
 534+ if ( $res instanceof ResultWrapper ) {
 535+ $res = $res->result;
 536+ }
528537 @$row = pg_fetch_array( $res );
529538 if( pg_last_error($this->mConn) ) {
530539 throw new DBUnexpectedError($this, 'SQL error: ' . htmlspecialchars( pg_last_error($this->mConn) ) );
——@@ -532,14 +541,27 @@
533542 }
534543
535544 function numRows( $res ) {
 545+ if ( $res instanceof ResultWrapper ) {
 546+ $res = $res->result;
 547+ }
536548 @$n = pg_num_rows( $res );
537549 if( pg_last_error($this->mConn) ) {
538550 throw new DBUnexpectedError($this, 'SQL error: ' . htmlspecialchars( pg_last_error($this->mConn) ) );
539551 }
540552 return $n;
541553 }
542 - function numFields( $res ) { return pg_num_fields( $res ); }
543 - function fieldName( $res, $n ) { return pg_field_name( $res, $n ); }
 554+ function numFields( $res ) {
 555+ if ( $res instanceof ResultWrapper ) {
 556+ $res = $res->result;
 557+ }
 558+ return pg_num_fields( $res );
 559+ }
 560+ function fieldName( $res, $n ) {
 561+ if ( $res instanceof ResultWrapper ) {
 562+ $res = $res->result;
 563+ }
 564+ return pg_field_name( $res, $n );
 565+ }
544566
545567 /**
546568 * This must be called after nextSequenceVal
——@@ -548,7 +570,13 @@
549571 return $this->mInsertId;
550572 }
551573
552 - function dataSeek( $res, $row ) { return pg_result_seek( $res, $row ); }
 574+ function dataSeek( $res, $row ) {
 575+ if ( $res instanceof ResultWrapper ) {
 576+ $res = $res->result;
 577+ }
 578+ return pg_result_seek( $res, $row );
 579+ }
 580+
553581 function lastError() {
554582 if ( $this->mConn ) {
555583 return pg_last_error();
——@@ -917,7 +945,7 @@
918946 . "WHERE c.relnamespace = n.oid AND c.relname = $etable AND n.nspname = $eschema "
919947 . "AND c.relkind IN ('" . implode("','", $types) . "')";
920948 $res = $this->query( $SQL );
921 - $count = $res ? pg_num_rows($res) : 0;
 949+ $count = $res ? $res->numRows() : 0;
922950 if ($res)
923951 $this->freeResult( $res );
924952 return $count ? true : false;
——@@ -950,7 +978,7 @@
951979 $this->addQuotes($trigger)));
952980 if (!$res)
953981 return NULL;
954 - $rows = pg_num_rows($res);
 982+ $rows = $res->numRows();
955983 $this->freeResult($res);
956984 return $rows;
957985 }
——@@ -974,7 +1002,7 @@
9751003 $res = $this->query($SQL);
9761004 if (!$res)
9771005 return NULL;
978 - $rows = pg_num_rows($res);
 1006+ $rows = $res->numRows();
9791007 $this->freeResult($res);
9801008 return $rows;
9811009 }
——@@ -987,7 +1015,12 @@
9881016 $SQL = "SELECT rolname FROM pg_catalog.pg_namespace n, pg_catalog.pg_roles r "
9891017 ."WHERE n.nspowner=r.oid AND n.nspname = '$eschema'";
9901018 $res = $this->query( $SQL );
991 - $owner = $res ? pg_num_rows($res) ? pg_fetch_result($res, 0, 0) : false : false;
 1019+ if ( $res && $res->numRows() ) {
 1020+ $row = $res->fetchRow();
 1021+ $owner = $row->rolname;
 1022+ } else {
 1023+ $owner = false;
 1024+ }
9921025 if ($res)
9931026 $this->freeResult($res);
9941027 return $owner;
——@@ -1005,7 +1038,7 @@
10061039 . "WHERE c.relnamespace = n.oid AND c.relname = '$etable' AND n.nspname = '$eschema' "
10071040 . "AND a.attrelid = c.oid AND a.attname = '$ecol'";
10081041 $res = $this->query( $SQL, $fname );
1009 - $count = $res ? pg_num_rows($res) : 0;
 1042+ $count = $res ? $res->numRows() : 0;
10101043 if ($res)
10111044 $this->freeResult( $res );
10121045 return $count;
——@@ -1071,7 +1104,8 @@
10721105 $tss = $this->addQuotes($wgDBts2schema);
10731106 $pgp = $this->addQuotes($wgDBport);
10741107 $dbn = $this->addQuotes($this->mDBname);
1075 - $ctype = pg_fetch_result($this->doQuery("SHOW lc_ctype"),0,0);
 1108+ $ctypeRow = $this->doQuery("SHOW lc_ctype")->fetchArray();
 1109+ $ctype = $ctypeRow[0];
10761110
10771111 $SQL = "UPDATE mediawiki_version SET mw_version=$mwv, pg_version=$pgv, pg_user=$pgu, ".
10781112 "mw_schema = $mws, ts2_schema = $tss, pg_port=$pgp, pg_dbname=$dbn, ".
Index: trunk/phase3/includes/Database.php
——@@ -678,9 +678,12 @@
679679 * Usually aborts on failure. If errors are explicitly ignored, returns success.
680680 *
681681 * @param $sql String: SQL query
682 - * @param $fname String: Name of the calling function, for profiling/SHOW PROCESSLIST comment (you can use __METHOD__ or add some extra info)
683 - * @param $tempIgnore Bool: Whether to avoid throwing an exception on errors... maybe best to catch the exception instead?
684 - * @return Result object to feed to fetchObject, fetchRow, ...; or false on failure if $tempIgnore set
 682+ * @param $fname String: Name of the calling function, for profiling/SHOW PROCESSLIST
 683+ * comment (you can use __METHOD__ or add some extra info)
 684+ * @param $tempIgnore Bool: Whether to avoid throwing an exception on errors...
 685+ * maybe best to catch the exception instead?
 686+ * @return true for a successful write query, ResultWrapper object for a successful read query,
 687+ * or false on failure if $tempIgnore set
685688 * @throws DBQueryError Thrown when the database returns an error of any kind
686689 */
687690 public function query( $sql, $fname = '', $tempIgnore = false ) {
——@@ -765,7 +768,7 @@
766769 wfProfileOut( $queryProf );
767770 wfProfileOut( $totalProf );
768771 }
769 - return $ret;
 772+ return $this->resultObject( $ret );
770773 }
771774
772775 /**
——@@ -909,6 +912,9 @@
910913 * Free a result object
911914 */
912915 function freeResult( $res ) {
 916+ if ( $res instanceof ResultWrapper ) {
 917+ $res = $res->result;
 918+ }
913919 if ( !@/**/mysql_free_result( $res ) ) {
914920 throw new DBUnexpectedError( $this, "Unable to free MySQL result" );
915921 }
——@@ -924,6 +930,9 @@
925931 * @throws DBUnexpectedError Thrown if the database returns an error
926932 */
927933 function fetchObject( $res ) {
 934+ if ( $res instanceof ResultWrapper ) {
 935+ $res = $res->result;
 936+ }
928937 @/**/$row = mysql_fetch_object( $res );
929938 if( $this->lastErrno() ) {
930939 throw new DBUnexpectedError( $this, 'Error in fetchObject(): ' . htmlspecialchars( $this->lastError() ) );
——@@ -940,6 +949,9 @@
941950 * @throws DBUnexpectedError Thrown if the database returns an error
942951 */
943952 function fetchRow( $res ) {
 953+ if ( $res instanceof ResultWrapper ) {
 954+ $res = $res->result;
 955+ }
944956 @/**/$row = mysql_fetch_array( $res );
945957 if ( $this->lastErrno() ) {
946958 throw new DBUnexpectedError( $this, 'Error in fetchRow(): ' . htmlspecialchars( $this->lastError() ) );
——@@ -951,6 +963,9 @@
952964 * Get the number of rows in a result object
953965 */
954966 function numRows( $res ) {
 967+ if ( $res instanceof ResultWrapper ) {
 968+ $res = $res->result;
 969+ }
955970 @/**/$n = mysql_num_rows( $res );
956971 if( $this->lastErrno() ) {
957972 throw new DBUnexpectedError( $this, 'Error in numRows(): ' . htmlspecialchars( $this->lastError() ) );
——@@ -962,14 +977,24 @@
963978 * Get the number of fields in a result object
964979 * See documentation for mysql_num_fields()
965980 */
966 - function numFields( $res ) { return mysql_num_fields( $res ); }
 981+ function numFields( $res ) {
 982+ if ( $res instanceof ResultWrapper ) {
 983+ $res = $res->result;
 984+ }
 985+ return mysql_num_fields( $res );
 986+ }
967987
968988 /**
969989 * Get a field name in a result object
970990 * See documentation for mysql_field_name():
971991 * http://www.php.net/mysql_field_name
972992 */
973 - function fieldName( $res, $n ) { return mysql_field_name( $res, $n ); }
 993+ function fieldName( $res, $n ) {
 994+ if ( $res instanceof ResultWrapper ) {
 995+ $res = $res->result;
 996+ }
 997+ return mysql_field_name( $res, $n );
 998+ }
974999
9751000 /**
9761001 * Get the inserted value of an auto-increment row
——@@ -987,7 +1012,12 @@
9881013 * Change the position of the cursor in a result object
9891014 * See mysql_data_seek()
9901015 */
991 - function dataSeek( $res, $row ) { return mysql_data_seek( $res, $row ); }
 1016+ function dataSeek( $res, $row ) {
 1017+ if ( $res instanceof ResultWrapper ) {
 1018+ $res = $res->result;
 1019+ }
 1020+ return mysql_data_seek( $res, $row );
 1021+ }
9921022
9931023 /**
9941024 * Get the last error number
——@@ -1352,9 +1382,9 @@
13531383 function fieldInfo( $table, $field ) {
13541384 $table = $this->tableName( $table );
13551385 $res = $this->query( "SELECT * FROM $table LIMIT 1" );
1356 - $n = mysql_num_fields( $res );
 1386+ $n = mysql_num_fields( $res->result );
13571387 for( $i = 0; $i < $n; $i++ ) {
1358 - $meta = mysql_fetch_field( $res, $i );
 1388+ $meta = mysql_fetch_field( $res->result, $i );
13591389 if( $field == $meta->name ) {
13601390 return new MySQLField($meta);
13611391 }
——@@ -1366,6 +1396,9 @@
13671397 * mysql_field_type() wrapper
13681398 */
13691399 function fieldType( $res, $index ) {
 1400+ if ( $res instanceof ResultWrapper ) {
 1401+ $res = $res->result;
 1402+ }
13701403 return mysql_field_type( $res, $index );
13711404 }
13721405
——@@ -2001,7 +2034,12 @@
20022035 */
20032036 function resultObject( $result ) {
20042037 if( empty( $result ) ) {
2005 - return NULL;
 2038+ return false;
 2039+ } elseif ( $result instanceof ResultWrapper ) {
 2040+ return $result;
 2041+ } elseif ( $result === true ) {
 2042+ // Successful write query
 2043+ return $result;
20062044 } else {
20072045 return new ResultWrapper( $this, $result );
20082046 }
——@@ -2176,7 +2214,7 @@
21772215 $cmd = $this->replaceVars( $cmd );
21782216 $res = $this->query( $cmd, __METHOD__, true );
21792217 if ( $resultCallback ) {
2180 - call_user_func( $resultCallback, $this->resultObject( $res ) );
 2218+ call_user_func( $resultCallback, $res );
21812219 }
21822220
21832221 if ( false === $res ) {
——@@ -2248,36 +2286,51 @@
22492287 var $db, $result;
22502288
22512289 /**
2252 - * @todo document
 2290+ * Create a new result object from a result resource and a Database object
22532291 */
2254 - function ResultWrapper( &$database, $result ) {
2255 - $this->db =& $database;
2256 - $this->result =& $result;
 2292+ function ResultWrapper( $database, $result ) {
 2293+ $this->db = $database;
 2294+ if ( $result instanceof ResultWrapper ) {
 2295+ $this->result = $result->result;
 2296+ } else {
 2297+ $this->result = $result;
 2298+ }
22572299 }
22582300
22592301 /**
2260 - * @todo document
 2302+ * Get the number of rows in a result object
22612303 */
22622304 function numRows() {
22632305 return $this->db->numRows( $this->result );
22642306 }
22652307
22662308 /**
2267 - * @todo document
 2309+ * Fetch the next row from the given result object, in object form.
 2310+ * Fields can be retrieved with $row->fieldname, with fields acting like
 2311+ * member variables.
 2312+ *
 2313+ * @param $res SQL result object as returned from Database::query(), etc.
 2314+ * @return MySQL row object
 2315+ * @throws DBUnexpectedError Thrown if the database returns an error
22682316 */
22692317 function fetchObject() {
22702318 return $this->db->fetchObject( $this->result );
22712319 }
22722320
22732321 /**
2274 - * @todo document
 2322+ * Fetch the next row from the given result object, in associative array
 2323+ * form. Fields are retrieved with $row['fieldname'].
 2324+ *
 2325+ * @param $res SQL result object as returned from Database::query(), etc.
 2326+ * @return MySQL row object
 2327+ * @throws DBUnexpectedError Thrown if the database returns an error
22752328 */
22762329 function fetchRow() {
22772330 return $this->db->fetchRow( $this->result );
22782331 }
22792332
22802333 /**
2281 - * @todo document
 2334+ * Free a result object
22822335 */
22832336 function free() {
22842337 $this->db->freeResult( $this->result );
——@@ -2285,10 +2338,17 @@
22862339 unset( $this->db );
22872340 }
22882341
 2342+ /**
 2343+ * Change the position of the cursor in a result object
 2344+ * See mysql_data_seek()
 2345+ */
22892346 function seek( $row ) {
22902347 $this->db->dataSeek( $this->result, $row );
22912348 }
22922349
 2350+ /**
 2351+ * Reset the cursor to the start of the result set
 2352+ */
22932353 function rewind() {
22942354 if ($this->numRows()) {
22952355 $this->db->dataSeek($this->result, 0);

Follow-up revisions

RevisionCommit summaryAuthorDate
r23912Merged revisions 23662-23909 via svnmerge from...david18:11, 9 July 2007

Status & tagging log