From: Aryeh Gregor Date: Tue, 16 Jun 2009 21:00:38 +0000 (+0000) Subject: Abstract more methods in DatabaseBase X-Git-Tag: 1.31.0-rc.0~41340 X-Git-Url: https://git.cyclocoop.org/%242?a=commitdiff_plain;h=9244ed0c57bda8869b38c54fc036f1741f00f146;p=lhc%2Fweb%2Fwiklou.git Abstract more methods in DatabaseBase Notably, this will switch conditional() in MySQL from using IF() to using CASE, like all other DBMSes. Documentation suggests this works back to 4.0. If it's a problem, it's a matter of a few lines to override it in DatabaseMysql.php. Also, some extra explanatory comments have been added to a number of methods in DatabaseBase. --- diff --git a/includes/db/Database.php b/includes/db/Database.php index 622f5df205..7d07816446 100644 --- a/includes/db/Database.php +++ b/includes/db/Database.php @@ -1481,11 +1481,15 @@ abstract class DatabaseBase { } /** - * USE INDEX clause - * PostgreSQL doesn't have them and returns "" + * USE INDEX clause. Unlikely to be useful for anything but MySQL. This + * is only needed because a) MySQL must be as efficient as possible due to + * its use on Wikipedia, and b) MySQL 4.0 is kind of dumb sometimes about + * which index to pick. Anyway, other databases might have different + * indexes on a given table. So don't bother overriding this unless you're + * MySQL. */ function useIndexClause( $index ) { - return "FORCE INDEX (" . $this->indexName( $index ) . ")"; + return ''; } /** @@ -1573,10 +1577,14 @@ abstract class DatabaseBase { } /** + * A string to insert into queries to show that they're low-priority, like + * MySQL's LOW_PRIORITY. If no such feature exists, return an empty + * string and nothing bad should happen. + * * @return string Returns the text of the low priority option if it is supported, or a blank string otherwise */ function lowPriorityOption() { - return 'LOW_PRIORITY'; + return ''; } /** @@ -1630,22 +1638,33 @@ abstract class DatabaseBase { } /** - * Construct a LIMIT query with optional offset - * This is used for query pages + * Construct a LIMIT query with optional offset. This is used for query + * pages. The SQL should be adjusted so that only the first $limit rows + * are returned. If $offset is provided as well, then the first $offset + * rows should be discarded, and the next $limit rows should be returned. + * If the result of the query is not ordered, then the rows to be returned + * are theoretically arbitrary. + * + * $sql is expected to be a SELECT, if that makes a difference. For + * UPDATE, limitResultForUpdate should be used. + * + * The version provided by default works in MySQL and SQLite. It will very + * likely need to be overridden for most other DBMSes. + * * @param $sql String: SQL query we will append the limit too * @param $limit Integer: the SQL limit * @param $offset Integer the SQL offset (default false) */ - function limitResult($sql, $limit, $offset=false) { - if( !is_numeric($limit) ) { + function limitResult( $sql, $limit, $offset=false ) { + if( !is_numeric( $limit ) ) { throw new DBUnexpectedError( $this, "Invalid non-numeric limit passed to limitResult()\n" ); } return "$sql LIMIT " . ( (is_numeric($offset) && $offset != 0) ? "{$offset}," : "" ) . "{$limit} "; } - function limitResultForUpdate($sql, $num) { - return $this->limitResult($sql, $num, 0); + function limitResultForUpdate( $sql, $num ) { + return $this->limitResult( $sql, $num, 0 ); } /** @@ -1662,8 +1681,8 @@ abstract class DatabaseBase { } /** - * Returns an SQL expression for a simple conditional. - * Uses IF on MySQL. + * Returns an SQL expression for a simple conditional. This doesn't need + * to be overridden unless CASE isn't supported in your DBMS. * * @param $cond String: SQL expression which will result in a boolean value * @param $trueVal String: SQL expression to return if true @@ -1671,7 +1690,7 @@ abstract class DatabaseBase { * @return String: SQL fragment */ function conditional( $cond, $trueVal, $falseVal ) { - return " IF($cond, $trueVal, $falseVal) "; + return " (CASE WHEN $cond THEN $trueVal ELSE $falseVal END) "; } /** @@ -1922,13 +1941,21 @@ abstract class DatabaseBase { } /** + * Returns a wikitext link to the DB's website, e.g., + * return "[http://www.mysql.com/ MySQL]"; + * Should probably be overridden to at least contain plain text, if for + * some reason your database has no website. + * * @return String: wikitext of a link to the server software's web site */ function getSoftwareLink() { - return "[http://www.mysql.com/ MySQL]"; + return '(no software link given)'; } /** + * A string describing the current software version, like from + * mysql_get_server_info(). Will be listed on Special:Version, etc. + * * @return String: Version information from the database */ abstract function getServerVersion(); @@ -2007,16 +2034,14 @@ abstract class DatabaseBase { } /** - * Override database's default connection timeout. - * May be useful for very long batch queries such as - * full-wiki dumps, where a single query reads out - * over hours or days. + * Override database's default connection timeout. May be useful for very + * long batch queries such as full-wiki dumps, where a single query reads + * out over hours or days. May or may not be necessary for non-MySQL + * databases. For most purposes, leaving it as a no-op should be fine. + * * @param $timeout Integer in seconds */ - public function setTimeout( $timeout ) { - $this->query( "SET net_read_timeout=$timeout" ); - $this->query( "SET net_write_timeout=$timeout" ); - } + public function setTimeout( $timeout ) {} /** * Read and execute SQL commands from a file. diff --git a/includes/db/DatabaseIbm_db2.php b/includes/db/DatabaseIbm_db2.php index 382b5e2c4d..0716eae2a5 100644 --- a/includes/db/DatabaseIbm_db2.php +++ b/includes/db/DatabaseIbm_db2.php @@ -1237,15 +1237,6 @@ EOF; return db2_num_rows( $this->mLastResult ); } - /** - * USE INDEX clause - * DB2 doesn't have them and returns "" - * @param sting $index - */ - public function useIndexClause( $index ) { - return ""; - } - /** * Simulates REPLACE with a DELETE followed by INSERT * @param $table Object @@ -1462,19 +1453,6 @@ EOF; return "[http://www.ibm.com/software/data/db2/express/?s_cmp=ECDDWW01&s_tact=MediaWiki IBM DB2]"; } - /** - * Returns an SQL expression for a simple conditional. - * Uses CASE on DB2 - * - * @param string $cond SQL expression which will result in a boolean value - * @param string $trueVal SQL expression to return if true - * @param string $falseVal SQL expression to return if false - * @return string SQL fragment - */ - public function conditional( $cond, $trueVal, $falseVal ) { - return " (CASE WHEN $cond THEN $trueVal ELSE $falseVal END) "; - } - ### # Fix search crash ### @@ -1537,11 +1515,6 @@ EOF; * @deprecated */ public function getStatus( $which ) { wfDebug('Not implemented for DB2: getStatus()'); return ''; } - /** - * Not implemented - * @deprecated - */ - public function setTimeout( $timeout ) { wfDebug('Not implemented for DB2: setTimeout()'); } /** * Not implemented * TODO @@ -1570,12 +1543,6 @@ EOF; * @deprecated */ public function limitResultForUpdate($sql, $num) { return $sql; } - /** - * No such option - * @return string '' - * @deprecated - */ - public function lowPriorityOption() { return ''; } ###################################### # Reflection diff --git a/includes/db/DatabaseMssql.php b/includes/db/DatabaseMssql.php index b470690d6e..a26fa45481 100644 --- a/includes/db/DatabaseMssql.php +++ b/includes/db/DatabaseMssql.php @@ -707,13 +707,6 @@ class DatabaseMssql extends DatabaseBase { return str_replace("'","''",$s); } - /** - * USE INDEX clause - */ - function useIndexClause( $index ) { - return ""; - } - /** * REPLACE query wrapper * PostgreSQL simulates this with a DELETE followed by INSERT @@ -857,18 +850,6 @@ class DatabaseMssql extends DatabaseBase { return $sql; } - /** - * Returns an SQL expression for a simple conditional. - * - * @param $cond String: SQL expression which will result in a boolean value - * @param $trueVal String: SQL expression to return if true - * @param $falseVal String: SQL expression to return if false - * @return string SQL fragment - */ - function conditional( $cond, $trueVal, $falseVal ) { - return " (CASE WHEN $cond THEN $trueVal ELSE $falseVal END) "; - } - /** * Should determine if the last failure was due to a deadlock * @return bool @@ -930,11 +911,6 @@ class DatabaseMssql extends DatabaseBase { return $sql; } - /** - * not done - */ - public function setTimeout($timeout) { return; } - /** * How lagged is this slave? */ diff --git a/includes/db/DatabaseMysql.php b/includes/db/DatabaseMysql.php index 27371f8da2..6508eb234e 100644 --- a/includes/db/DatabaseMysql.php +++ b/includes/db/DatabaseMysql.php @@ -276,6 +276,23 @@ class DatabaseMysql extends DatabaseBase { function getServerVersion() { return mysql_get_server_info( $this->mConn ); } + + function useIndexClause( $index ) { + return "FORCE INDEX (" . $this->indexName( $index ) . ")"; + } + + function lowPriorityOption() { + return 'LOW_PRIORITY'; + } + + function getSoftwareLink() { + return '[http://www.mysql.com/ MySQL]'; + } + + public function setTimeout( $timeout ) { + $this->query( "SET net_read_timeout=$timeout" ); + $this->query( "SET net_write_timeout=$timeout" ); + } } /** diff --git a/includes/db/DatabaseOracle.php b/includes/db/DatabaseOracle.php index 66ffe3de97..1110f7bc44 100644 --- a/includes/db/DatabaseOracle.php +++ b/includes/db/DatabaseOracle.php @@ -589,13 +589,6 @@ class DatabaseOracle extends DatabaseBase { return $this->mInsertId; } - /** - * Oracle does not have a "USE INDEX" clause, so return an empty string - */ - function useIndexClause($index) { - return ''; - } - # REPLACE query wrapper # Oracle simulates this with a DELETE followed by INSERT # $row is the row to insert, an associative array @@ -687,10 +680,6 @@ class DatabaseOracle extends DatabaseBase { return $size; } - function lowPriorityOption() { - return ''; - } - function limitResult($sql, $limit, $offset) { if ($offset === false) $offset = 0; @@ -703,19 +692,6 @@ class DatabaseOracle extends DatabaseBase { return 'SELECT * '.($all?'':'/* UNION_UNIQUE */ ').'FROM ('.implode( $glue, $sqls ).')' ; } - /** - * Returns an SQL expression for a simple conditional. - * Uses CASE on Oracle - * - * @param $cond String: SQL expression which will result in a boolean value - * @param $trueVal String: SQL expression to return if true - * @param $falseVal String: SQL expression to return if false - * @return String: SQL fragment - */ - function conditional( $cond, $trueVal, $falseVal ) { - return " (CASE WHEN $cond THEN $trueVal ELSE $falseVal END) "; - } - function wasDeadlock() { return $this->lastErrno() == 'OCI-00060'; } @@ -1043,10 +1019,6 @@ class DatabaseOracle extends DatabaseBase { return 'BITOR('.$fieldLeft.', '.$fieldRight.')'; } - public function setTimeout( $timeout ) { - // @todo fixme no-op - } - /** * How lagged is this slave? * diff --git a/includes/db/DatabasePostgres.php b/includes/db/DatabasePostgres.php index 6a99fd5775..98ef1ac8ca 100644 --- a/includes/db/DatabasePostgres.php +++ b/includes/db/DatabasePostgres.php @@ -909,13 +909,6 @@ class DatabasePostgres extends DatabaseBase { return $currval; } - /** - * Postgres does not have a "USE INDEX" clause, so return an empty string - */ - function useIndexClause( $index ) { - return ''; - } - # REPLACE query wrapper # Postgres simulates this with a DELETE followed by INSERT # $row is the row to insert, an associative array @@ -1009,27 +1002,10 @@ class DatabasePostgres extends DatabaseBase { return $size; } - function lowPriorityOption() { - return ''; - } - function limitResult($sql, $limit, $offset=false) { return "$sql LIMIT $limit ".(is_numeric($offset)?" OFFSET {$offset} ":""); } - /** - * Returns an SQL expression for a simple conditional. - * Uses CASE on Postgres - * - * @param $cond String: SQL expression which will result in a boolean value - * @param $trueVal String: SQL expression to return if true - * @param $falseVal String: SQL expression to return if false - * @return String: SQL fragment - */ - function conditional( $cond, $trueVal, $falseVal ) { - return " (CASE WHEN $cond THEN $trueVal ELSE $falseVal END) "; - } - function wasDeadlock() { return $this->lastErrno() == '40P01'; } @@ -1387,10 +1363,6 @@ END; return array( $startOpts, $useIndex, $preLimitTail, $postLimitTail ); } - public function setTimeout( $timeout ) { - // @todo fixme no-op - } - /** * How lagged is this slave? * diff --git a/includes/db/DatabaseSqlite.php b/includes/db/DatabaseSqlite.php index 1678633daf..056b77f98b 100644 --- a/includes/db/DatabaseSqlite.php +++ b/includes/db/DatabaseSqlite.php @@ -264,13 +264,6 @@ class DatabaseSqlite extends DatabaseBase { return $ret; } - /** - * SQLite does not have a "USE INDEX" clause, so return an empty string - */ - function useIndexClause($index) { - return ''; - } - /** * Returns the size of a text field, or -1 for "unlimited" * In SQLite this is SQLITE_MAX_LENGTH, by default 1GB. No way to query it though. @@ -279,21 +272,6 @@ class DatabaseSqlite extends DatabaseBase { return -1; } - /** - * No low priority option in SQLite - */ - function lowPriorityOption() { - return ''; - } - - /** - * Returns an SQL expression for a simple conditional. - * - uses CASE on SQLite - */ - function conditional($cond, $trueVal, $falseVal) { - return " (CASE WHEN $cond THEN $trueVal ELSE $falseVal END) "; - } - function wasDeadlock() { return $this->lastErrno() == SQLITE_BUSY; } @@ -389,11 +367,6 @@ class DatabaseSqlite extends DatabaseBase { function quote_ident($s) { return $s; } - /** - * not done - */ - public function setTimeout($timeout) { return; } - /** * How lagged is this slave? */