From eb16f5898d1132a86fe2beeed4cfa1fba02bd054 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Fri, 15 Jun 2012 22:24:59 -0700 Subject: [PATCH] [Database] Various DB cleanups. * Removed unused and obsolete set() and safeQuery() functions. * Removed unused deprecated constructor functions. * Removed unused limitResultForUpdate() cruft function. * Removed unused standardSelectDistinct(), it's better to just follow the standard for all queries. * Removed other cruft functions unused by core/extensions. * Made some internal functions protected. Change-Id: I90be88ea740834a417a17d7751f1be7bac4eae4e --- RELEASE-NOTES-1.20 | 2 + includes/db/Database.php | 218 +++---------------- includes/db/DatabaseIbm_db2.php | 62 ++---- includes/db/DatabaseMssql.php | 8 - includes/db/DatabaseMysql.php | 7 - includes/db/DatabaseOracle.php | 5 - includes/db/DatabasePostgres.php | 5 - includes/db/DatabaseSqlite.php | 29 +-- tests/phpunit/includes/db/TestORMRowTest.php | 2 +- 9 files changed, 63 insertions(+), 275 deletions(-) diff --git a/RELEASE-NOTES-1.20 b/RELEASE-NOTES-1.20 index 05dd980304..dfd747f702 100644 --- a/RELEASE-NOTES-1.20 +++ b/RELEASE-NOTES-1.20 @@ -266,6 +266,8 @@ changes to languages because of Bugzilla reports. so the WikiPage code wasn't useful there either. * Deprecated mw.user.name in favour of mw.user.getName. * Deprecated mw.user.anonymous in favour of mw.user.isAnon. +* Deprecated DatabaseBase functions newFromParams(), newFromType(), set(), + quote_ident(), and escapeLike() were removed. == Compatibility == diff --git a/includes/db/Database.php b/includes/db/Database.php index cc4ffafe0a..4771659d45 100644 --- a/includes/db/Database.php +++ b/includes/db/Database.php @@ -462,16 +462,6 @@ abstract class DatabaseBase implements DatabaseType { return true; } - /** - * Returns true if this database requires that SELECT DISTINCT queries require that all - ORDER BY expressions occur in the SELECT list per the SQL92 standard - * - * @return bool - */ - function standardSelectDistinct() { - return true; - } - /** * Returns true if this database can do a native search on IP columns * e.g. this works as expected: .. WHERE rc_ip = '127.42.12.102/32'; @@ -651,36 +641,6 @@ abstract class DatabaseBase implements DatabaseType { throw new MWException( 'Database serialization may cause problems, since the connection is not restored on wakeup.' ); } - /** - * Same as new DatabaseMysql( ... ), kept for backward compatibility - * @deprecated since 1.17 - * - * @param $server - * @param $user - * @param $password - * @param $dbName - * @param $flags int - * @return DatabaseMysql - */ - static function newFromParams( $server, $user, $password, $dbName, $flags = 0 ) { - wfDeprecated( __METHOD__, '1.17' ); - return new DatabaseMysql( $server, $user, $password, $dbName, $flags ); - } - - /** - * Same as new factory( ... ), kept for backward compatibility - * @deprecated since 1.18 - * @see Database::factory() - * @return DatabaseBase - */ - public final static function newFromType( $dbType, $p = array() ) { - wfDeprecated( __METHOD__, '1.18' ); - if ( isset( $p['tableprefix'] ) ) { - $p['tablePrefix'] = $p['tableprefix']; - } - return self::factory( $dbType, $p ); - } - /** * Given a DB type, construct the name of the appropriate child class of * DatabaseBase. This is designed to replace all of the manual stuff like: @@ -982,16 +942,12 @@ abstract class DatabaseBase implements DatabaseType { * & = filename; reads the file and inserts as a blob * (we don't use this though...) * - * This function should not be used directly by new code outside of the - * database classes. The query wrapper functions (select() etc.) should be - * used instead. - * * @param $sql string * @param $func string * * @return array */ - function prepare( $sql, $func = 'DatabaseBase::prepare' ) { + protected function prepare( $sql, $func = 'DatabaseBase::prepare' ) { /* MySQL doesn't support prepared statements (yet), so just pack up the query for reference. We'll manually replace the bits later. */ @@ -1002,7 +958,7 @@ abstract class DatabaseBase implements DatabaseType { * Free a prepared query, generated by prepare(). * @param $prepared */ - function freePrepared( $prepared ) { + protected function freePrepared( $prepared ) { /* No-op by default */ } @@ -1026,36 +982,8 @@ abstract class DatabaseBase implements DatabaseType { } /** - * Prepare & execute an SQL statement, quoting and inserting arguments - * in the appropriate places. + * For faking prepared SQL statements on DBs that don't support it directly. * - * This function should not be used directly by new code outside of the - * database classes. The query wrapper functions (select() etc.) should be - * used instead. - * - * @param $query String - * @param $args ... - * - * @return ResultWrapper - */ - function safeQuery( $query, $args = null ) { - $prepared = $this->prepare( $query, 'DatabaseBase::safeQuery' ); - - if ( !is_array( $args ) ) { - # Pull the var args - $args = func_get_args(); - array_shift( $args ); - } - - $retval = $this->execute( $prepared, $args ); - $this->freePrepared( $prepared ); - - return $retval; - } - - /** - * For faking prepared SQL statements on DBs that don't support - * it directly. * @param $preparedQuery String: a 'preparable' SQL statement * @param $args Array of arguments to fill it with * @return string executable SQL @@ -1076,7 +1004,7 @@ abstract class DatabaseBase implements DatabaseType { * @param $matches Array * @return String */ - function fillPreparedArg( $matches ) { + protected function fillPreparedArg( $matches ) { switch( $matches[1] ) { case '\\?': return '?'; case '\\!': return '!'; @@ -1103,32 +1031,7 @@ abstract class DatabaseBase implements DatabaseType { * * @param $res Mixed: A SQL result */ - function freeResult( $res ) { - } - - /** - * Simple UPDATE wrapper. - * Usually throws a DBQueryError on failure. - * If errors are explicitly ignored, returns success - * - * This function exists for historical reasons, DatabaseBase::update() has a more standard - * calling convention and feature set - * - * @param $table string - * @param $var - * @param $value - * @param $cond - * @param $fname string - * - * @return bool - */ - function set( $table, $var, $value, $cond, $fname = 'DatabaseBase::set' ) { - $table = $this->tableName( $table ); - $sql = "UPDATE $table SET $var = '" . - $this->strencode( $value ) . "' WHERE ($cond)"; - - return (bool)$this->query( $sql, $fname ); - } + public function freeResult( $res ) {} /** * A SELECT wrapper which returns a single field from a single result row. @@ -1414,7 +1317,9 @@ abstract class DatabaseBase implements DatabaseType { /** * The equivalent of DatabaseBase::select() except that the constructed SQL - * is returned, instead of being immediately executed. + * is returned, instead of being immediately executed. This can be useful for + * doing UNION queries, where the SQL text of each query is needed. In general, + * however, callers outside of Database classes should just use select(). * * @param $table string|array Table name * @param $vars string|array Field names @@ -1537,7 +1442,7 @@ abstract class DatabaseBase implements DatabaseType { $fname = 'DatabaseBase::estimateRowCount', $options = array() ) { $rows = 0; - $res = $this->select ( $table, 'COUNT(*) AS rowcount', $conds, $fname, $options ); + $res = $this->select( $table, 'COUNT(*) AS rowcount', $conds, $fname, $options ); if ( $res ) { $row = $this->fetchRow( $res ); @@ -1664,7 +1569,7 @@ abstract class DatabaseBase implements DatabaseType { * @param $options array * @return string */ - function makeInsertOptions( $options ) { + protected function makeInsertOptions( $options ) { return implode( ' ', $options ); } @@ -1749,7 +1654,7 @@ abstract class DatabaseBase implements DatabaseType { * @param $options Array: The options passed to DatabaseBase::update * @return string */ - function makeUpdateOptions( $options ) { + protected function makeUpdateOptions( $options ) { if ( !is_array( $options ) ) { $options = array( $options ); } @@ -1900,8 +1805,16 @@ abstract class DatabaseBase implements DatabaseType { } /** - * Bitwise operations + * Return aggregated value alias + * + * @param $valuedata + * @param $valuename string + * + * @return string */ + public function aggregateValue( $valuedata, $valuename = 'value' ) { + return $valuename; + } /** * @param $field @@ -1929,6 +1842,15 @@ abstract class DatabaseBase implements DatabaseType { return "($fieldLeft | $fieldRight)"; } + /** + * Build a concatenation list to feed into a SQL query + * @param $stringList Array: list of raw SQL expressions; caller is responsible for any quoting + * @return String + */ + public function buildConcat( $stringList ) { + return 'CONCAT(' . implode( ',', $stringList ) . ')'; + } + /** * Change the current database * @@ -2188,7 +2110,7 @@ abstract class DatabaseBase implements DatabaseType { * * @return string */ - function indexName( $index ) { + protected function indexName( $index ) { // Backwards-compatibility hack $renamed = array( 'ar_usertext_timestamp' => 'usertext_timestamp', @@ -2249,36 +2171,6 @@ abstract class DatabaseBase implements DatabaseType { return $name[0] == '"' && substr( $name, -1, 1 ) == '"'; } - /** - * Backwards compatibility, identifier quoting originated in DatabasePostgres - * which used quote_ident which does not follow our naming conventions - * was renamed to addIdentifierQuotes. - * @deprecated since 1.18 use addIdentifierQuotes - * - * @param $s string - * - * @return string - */ - function quote_ident( $s ) { - wfDeprecated( __METHOD__, '1.18' ); - return $this->addIdentifierQuotes( $s ); - } - - /** - * Escape string for safe LIKE usage. - * WARNING: you should almost never use this function directly, - * instead use buildLike() that escapes everything automatically - * @deprecated since 1.17, warnings in 1.17, removed in ??? - * - * @param $s string - * - * @return string - */ - public function escapeLike( $s ) { - wfDeprecated( __METHOD__, '1.17' ); - return $this->escapeLikeInternal( $s ); - } - /** * @param $s string * @return string @@ -2646,8 +2538,7 @@ abstract class DatabaseBase implements DatabaseType { * 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. + * $sql is expected to be a SELECT, if that makes a difference. * * The version provided by default works in MySQL and SQLite. It will very * likely need to be overridden for most other DBMSes. @@ -2662,19 +2553,9 @@ abstract class DatabaseBase implements DatabaseType { 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} "; - } - - /** - * @param $sql - * @param $num - * @return string - */ - function limitResultForUpdate( $sql, $num ) { - return $this->limitResult( $sql, $num, 0 ); + . ( ( is_numeric( $offset ) && $offset != 0 ) ? "{$offset}," : "" ) + . "{$limit} "; } /** @@ -3034,18 +2915,6 @@ abstract class DatabaseBase implements DatabaseType { } } - /** - * Return aggregated value alias - * - * @param $valuedata - * @param $valuename string - * - * @return string - */ - function aggregateValue ( $valuedata, $valuename = 'value' ) { - return $valuename; - } - /** * Ping the server and try to reconnect if it there is no connection * @@ -3101,18 +2970,6 @@ abstract class DatabaseBase implements DatabaseType { return $b; } - /** - * Override database's default connection timeout - * - * @param $timeout Integer in seconds - * @return void - * @deprecated since 1.19; use setSessionOptions() - */ - public function setTimeout( $timeout ) { - wfDeprecated( __METHOD__, '1.19' ); - $this->setSessionOptions( array( 'connTimeout' => $timeout ) ); - } - /** * Override database's default behavior. $options include: * 'connTimeout' : Set the connection timeout value in seconds. @@ -3377,15 +3234,6 @@ abstract class DatabaseBase implements DatabaseType { return $this->indexName( $matches[1] ); } - /** - * Build a concatenation list to feed into a SQL query - * @param $stringList Array: list of raw SQL expressions; caller is responsible for any quoting - * @return String - */ - function buildConcat( $stringList ) { - return 'CONCAT(' . implode( ',', $stringList ) . ')'; - } - /** * Check to see if a named lock is available. This is non-blocking. * diff --git a/includes/db/DatabaseIbm_db2.php b/includes/db/DatabaseIbm_db2.php index 1fdcd5cee1..80220af0ec 100644 --- a/includes/db/DatabaseIbm_db2.php +++ b/includes/db/DatabaseIbm_db2.php @@ -145,21 +145,21 @@ class IBM_DB2Result{ */ public function __construct( $db, $result, $num_rows, $sql, $columns ){ $this->db = $db; - + if( $result instanceof ResultWrapper ){ $this->result = $result->result; } else{ $this->result = $result; } - + $this->num_rows = $num_rows; $this->current_pos = 0; if ( $this->num_rows > 0 ) { // Make a lower-case list of the column names // By default, DB2 column names are capitalized // while MySQL column names are lowercase - + // Is there a reasonable maximum value for $i? // Setting to 2048 to prevent an infinite loop for( $i = 0; $i < 2048; $i++ ) { @@ -170,11 +170,11 @@ class IBM_DB2Result{ else { return false; } - + $this->columns[$i] = strtolower( $name ); } } - + $this->sql = $sql; } @@ -202,14 +202,14 @@ class IBM_DB2Result{ * @return mixed Object on success, false on failure. */ public function fetchObject() { - if ( $this->result - && $this->num_rows > 0 - && $this->current_pos >= 0 - && $this->current_pos < $this->num_rows ) + if ( $this->result + && $this->num_rows > 0 + && $this->current_pos >= 0 + && $this->current_pos < $this->num_rows ) { $row = $this->fetchRow(); $ret = new stdClass(); - + foreach ( $row as $k => $v ) { $lc = $this->columns[$k]; $ret->$lc = $v; @@ -225,9 +225,9 @@ class IBM_DB2Result{ * @throws DBUnexpectedError */ public function fetchRow(){ - if ( $this->result - && $this->num_rows > 0 - && $this->current_pos >= 0 + if ( $this->result + && $this->num_rows > 0 + && $this->current_pos >= 0 && $this->current_pos < $this->num_rows ) { if ( $this->loadedLines <= $this->current_pos ) { @@ -242,7 +242,7 @@ class IBM_DB2Result{ if ( $this->loadedLines > $this->current_pos ){ return $this->resultSet[$this->current_pos++]; } - + } return false; } @@ -416,7 +416,7 @@ class DatabaseIbm_db2 extends DatabaseBase { return 'ibm_db2'; } - /** + /** * Returns the database connection object * @return Object */ @@ -1341,10 +1341,10 @@ class DatabaseIbm_db2 extends DatabaseBase { $res2 = parent::select( $table, $vars2, $conds, $fname, $options2, $join_conds ); - + $obj = $this->fetchObject( $res2 ); $this->mNumRows = $obj->num_rows; - + return new ResultWrapper( $this, new IBM_DB2Result( $this, $res, $obj->num_rows, $vars, $sql ) ); } @@ -1441,14 +1441,6 @@ class DatabaseIbm_db2 extends DatabaseBase { ###################################### # Unimplemented and not applicable ###################################### - /** - * Not implemented - * @return string $sql - */ - public function limitResultForUpdate( $sql, $num ) { - $this->installPrint( 'Not implemented for DB2: limitResultForUpdate()' ); - return $sql; - } /** * Only useful with fake prepare like in base Database class @@ -1656,26 +1648,6 @@ SQL; return $res; } - /** - * Prepare & execute an SQL statement, quoting and inserting arguments - * in the appropriate places. - * @param $query String - * @param $args ... - * @return Resource - */ - public function safeQuery( $query, $args = null ) { - // copied verbatim from Database.php - $prepared = $this->prepare( $query, 'DB2::safeQuery' ); - if( !is_array( $args ) ) { - # Pull the var args - $args = func_get_args(); - array_shift( $args ); - } - $retval = $this->execute( $prepared, $args ); - $this->freePrepared( $prepared ); - return $retval; - } - /** * For faking prepared SQL statements on DBs that don't support * it directly. diff --git a/includes/db/DatabaseMssql.php b/includes/db/DatabaseMssql.php index 7a75e1eb42..3846e96101 100644 --- a/includes/db/DatabaseMssql.php +++ b/includes/db/DatabaseMssql.php @@ -620,14 +620,6 @@ class DatabaseMssql extends DatabaseBase { return $sql; } - // MSSQL does support this, but documentation is too thin to make a generalized - // function for this. Apparently UPDATE TOP (N) works, but the sort order - // may not be what we're expecting so the top n results may be a random selection. - // TODO: Implement properly. - function limitResultForUpdate( $sql, $num ) { - return $sql; - } - function timestamp( $ts = 0 ) { return wfTimestamp( TS_ISO_8601, $ts ); } diff --git a/includes/db/DatabaseMysql.php b/includes/db/DatabaseMysql.php index 1d03073b15..e27d3db448 100644 --- a/includes/db/DatabaseMysql.php +++ b/includes/db/DatabaseMysql.php @@ -659,13 +659,6 @@ class DatabaseMysql extends DatabaseBase { return '[http://www.mysql.com/ MySQL]'; } - /** - * @return bool - */ - function standardSelectDistinct() { - return false; - } - /** * @param $options array */ diff --git a/includes/db/DatabaseOracle.php b/includes/db/DatabaseOracle.php index e1b3a9a839..cf3e45dc2e 100644 --- a/includes/db/DatabaseOracle.php +++ b/includes/db/DatabaseOracle.php @@ -979,11 +979,6 @@ class DatabaseOracle extends DatabaseBase { } } - /* Not even sure why this is used in the main codebase... */ - function limitResultForUpdate( $sql, $num ) { - return $sql; - } - /* defines must comply with ^define\s*([^\s=]*)\s*=\s?'\{\$([^\}]*)\}'; */ function sourceStream( $fp, $lineCallback = false, $resultCallback = false, $fname = 'DatabaseOracle::sourceStream', $inputCallback = false ) { diff --git a/includes/db/DatabasePostgres.php b/includes/db/DatabasePostgres.php index 2b7521f21f..3504892e0a 100644 --- a/includes/db/DatabasePostgres.php +++ b/includes/db/DatabasePostgres.php @@ -1296,11 +1296,6 @@ SQL; return pg_field_type( $res, $index ); } - /* Not even sure why this is used in the main codebase... */ - function limitResultForUpdate( $sql, $num ) { - return $sql; - } - /** * @param $b * @return Blob diff --git a/includes/db/DatabaseSqlite.php b/includes/db/DatabaseSqlite.php index 15d1ad024b..952b4974c4 100644 --- a/includes/db/DatabaseSqlite.php +++ b/includes/db/DatabaseSqlite.php @@ -176,7 +176,7 @@ class DatabaseSqlite extends DatabaseBase { } $cachedResult = false; $table = 'dummy_search_test'; - + $db = new DatabaseSqliteStandalone( ':memory:' ); if ( $db->query( "CREATE VIRTUAL TABLE $table USING FTS3(dummy_field)", __METHOD__, true ) ) { @@ -313,7 +313,7 @@ class DatabaseSqlite extends DatabaseBase { /** * @param $res ResultWrapper - * @param $n + * @param $n * @return bool */ function fieldName( $res, $n ) { @@ -669,15 +669,6 @@ class DatabaseSqlite extends DatabaseBase { $this->mTrxLevel = 0; } - /** - * @param $sql - * @param $num - * @return string - */ - function limitResultForUpdate( $sql, $num ) { - return $this->limitResult( $sql, $num ); - } - /** * @param $s string * @return string @@ -827,8 +818,8 @@ class DatabaseSqlite extends DatabaseBase { } return $this->query( $sql, $fname ); } - - + + /** * List all tables on the database * @@ -843,21 +834,21 @@ class DatabaseSqlite extends DatabaseBase { 'name', "type='table'" ); - + $endArray = array(); - - foreach( $result as $table ) { + + foreach( $result as $table ) { $vars = get_object_vars($table); $table = array_pop( $vars ); - + if( !$prefix || strpos( $table, $prefix ) === 0 ) { if ( strpos( $table, 'sqlite_' ) !== 0 ) { $endArray[] = $table; } - + } } - + return $endArray; } diff --git a/tests/phpunit/includes/db/TestORMRowTest.php b/tests/phpunit/includes/db/TestORMRowTest.php index ca3e9e6f9a..fe5867af09 100644 --- a/tests/phpunit/includes/db/TestORMRowTest.php +++ b/tests/phpunit/includes/db/TestORMRowTest.php @@ -68,7 +68,7 @@ class TestORMRowTest extends ORMRowTest { $idField = $isSqlite ? 'INTEGER' : 'INT unsigned'; $primaryKey = $isSqlite ? 'PRIMARY KEY AUTOINCREMENT' : 'auto_increment PRIMARY KEY'; - $dbw->safeQuery( + $dbw->query( 'CREATE TABLE IF NOT EXISTS ' . $dbw->tableName( 'orm_test' ) . '( test_id ' . $idField . ' NOT NULL ' . $primaryKey . ', test_name VARCHAR(255) NOT NULL, -- 2.20.1