Fix SQLite exception when $wgDBuser is set
authorTim Starling <tstarling@wikimedia.org>
Thu, 17 Oct 2013 04:16:46 +0000 (15:16 +1100)
committerAaron Schulz <aschulz@wikimedia.org>
Thu, 17 Oct 2013 22:36:47 +0000 (22:36 +0000)
Fixed from several different directions:
* The error reporting was screwed up, so I wrapped the PDOException
  in a DBUnexpectedError.
* Make $db->open() trigger a close/open sequence if called a second
  time, like MySQL, instead of breaking mTrxLevel.
* Don't call $this->open() twice from the constructor, even if $user is
  set.

The bug dates to r84485. The nasty hack allowing you to construct a
do-nothing database object with "new Database()" dates back to 2004 and
has probably outlived its usefulness -- noted this. It was formerly used
by LoadBalancer::reportConnectionError().

Bug: 49254
Change-Id: I571f9209bec8e8ed5058b6327e1738eb4315d5a0

includes/db/Database.php
includes/db/DatabaseSqlite.php

index 1f7a830..c3850b9 100644 (file)
@@ -662,6 +662,18 @@ abstract class DatabaseBase implements IDatabase, DatabaseType {
 
        /**
         * Constructor.
+        *
+        * FIXME: It is possible to construct a Database object with no associated
+        * connection object, by specifying no parameters to __construct(). This
+        * feature is deprecated and should be removed.
+        *
+        * FIXME: The long list of formal parameters here is not really appropriate
+        * for MySQL, and not at all appropriate for any other DBMS. It should be
+        * replaced by named parameters as in DatabaseBase::factory().
+        *
+        * DatabaseBase subclasses should not be constructed directly in external
+        * code. DatabaseBase::factory() should be used instead.
+        *
         * @param string $server database server host
         * @param string $user database user name
         * @param string $password database user password
index a8270bf..4a51226 100644 (file)
@@ -52,7 +52,7 @@ class DatabaseSqlite extends DatabaseBase {
                $this->mName = $dbName;
                parent::__construct( $server, $user, $password, $dbName, $flags );
                // parent doesn't open when $user is false, but we can work with $dbName
-               if ( $dbName ) {
+               if ( $dbName && !$this->isOpen() ) {
                        global $wgSharedDB;
                        if ( $this->open( $server, $user, $password, $dbName ) && $wgSharedDB ) {
                                $this->attachDatabase( $wgSharedDB );
@@ -90,6 +90,7 @@ class DatabaseSqlite extends DatabaseBase {
        function open( $server, $user, $pass, $dbName ) {
                global $wgSQLiteDataDir;
 
+               $this->close();
                $fileName = self::generateFileName( $wgSQLiteDataDir, $dbName );
                if ( !is_readable( $fileName ) ) {
                        $this->mConn = false;
@@ -655,7 +656,11 @@ class DatabaseSqlite extends DatabaseBase {
                if ( $this->mTrxLevel == 1 ) {
                        $this->commit( __METHOD__ );
                }
-               $this->mConn->beginTransaction();
+               try {
+                       $this->mConn->beginTransaction();
+               } catch ( PDOException $e ) {
+                       throw new DBUnexpectedError( $this, 'Error in BEGIN query: ' . $e->getMessage() );
+               }
                $this->mTrxLevel = 1;
        }
 
@@ -663,7 +668,11 @@ class DatabaseSqlite extends DatabaseBase {
                if ( $this->mTrxLevel == 0 ) {
                        return;
                }
-               $this->mConn->commit();
+               try {
+                       $this->mConn->commit();
+               } catch ( PDOException $e ) {
+                       throw new DBUnexpectedError( $this, 'Error in COMMIT query: ' . $e->getMessage() );
+               }
                $this->mTrxLevel = 0;
        }