Abstract more methods in DatabaseBase
authorAryeh Gregor <simetrical@users.mediawiki.org>
Tue, 16 Jun 2009 21:00:38 +0000 (21:00 +0000)
committerAryeh Gregor <simetrical@users.mediawiki.org>
Tue, 16 Jun 2009 21:00:38 +0000 (21:00 +0000)
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.

includes/db/Database.php
includes/db/DatabaseIbm_db2.php
includes/db/DatabaseMssql.php
includes/db/DatabaseMysql.php
includes/db/DatabaseOracle.php
includes/db/DatabasePostgres.php
includes/db/DatabaseSqlite.php

index 622f5df..7d07816 100644 (file)
@@ -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.
index 382b5e2..0716eae 100644 (file)
@@ -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
index b470690..a26fa45 100644 (file)
@@ -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?
         */
index 27371f8..6508eb2 100644 (file)
@@ -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" );
+       }
 }
 
 /**
index 66ffe3d..1110f7b 100644 (file)
@@ -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?
         *
index 6a99fd5..98ef1ac 100644 (file)
@@ -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?
         *
index 1678633..056b77f 100644 (file)
@@ -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?
         */