rdbms: remove $opened field from Database for simplicity
authorAaron Schulz <aschulz@wikimedia.org>
Thu, 13 Jun 2019 12:46:03 +0000 (13:46 +0100)
committerTimo Tijhof <krinklemail@gmail.com>
Thu, 27 Jun 2019 17:29:12 +0000 (18:29 +0100)
This avoids having two similar fields that have to stay
in sync. Clean up the related error handling for connections.
If a connection handle is unusable, like when essential SET
queries fail, then destroy it.

Also:
* Avoid use of transactions in DatabasePostgres::determineCoreSchema.
* Make sure all subclasses log on connection failure.
* Add schema sanity checks to mysql/sqlite classes.
* Add IDatabase::QUERY_NO_RETRY flag to simplify reasoning about
  queries that already run on open() to begin with.
* Remove unused return value of Database::open.
* Remove deprecated Database::reportConnectionError method.

Change-Id: I97beba7ead1523085bda8784234d00c69ef1accc

RELEASE-NOTES-1.34
includes/db/DatabaseOracle.php
includes/installer/PostgresInstaller.php
includes/libs/rdbms/database/Database.php
includes/libs/rdbms/database/DatabaseMssql.php
includes/libs/rdbms/database/DatabaseMysqlBase.php
includes/libs/rdbms/database/DatabaseMysqli.php
includes/libs/rdbms/database/DatabasePostgres.php
includes/libs/rdbms/database/DatabaseSqlite.php
includes/libs/rdbms/database/IDatabase.php
tests/phpunit/includes/db/DatabaseTestHelper.php

index d34b459..a79f57a 100644 (file)
@@ -251,6 +251,7 @@ because of Phabricator reports.
   calls that use an invalid path (i.e., something other than an absolute path,
   protocol-relative URL, or full scheme URL), and will instead pass them to the
   client where they will likely 404. This usage was deprecated in 1.24.
+* Database::reportConnectionError, deprecated in 1.32, has been removed.
 * …
 
 === Deprecations in 1.34 ===
index 5df7aef..4af62a0 100644 (file)
@@ -21,6 +21,7 @@
  * @ingroup Database
  */
 
+use Wikimedia\AtEase\AtEase;
 use Wikimedia\Timestamp\ConvertibleTimestamp;
 use Wikimedia\Rdbms\Database;
 use Wikimedia\Rdbms\DatabaseDomain;
@@ -68,10 +69,10 @@ class DatabaseOracle extends Database {
        }
 
        function __destruct() {
-               if ( $this->opened ) {
-                       Wikimedia\suppressWarnings();
+               if ( $this->conn ) {
+                       AtEase::suppressWarnings();
                        $this->close();
-                       Wikimedia\restoreWarnings();
+                       AtEase::restoreWarnings();
                }
        }
 
@@ -159,8 +160,6 @@ class DatabaseOracle extends Database {
                        throw new DBConnectionError( $this, $this->lastError() );
                }
 
-               $this->opened = true;
-
                # removed putenv calls because they interfere with the system globaly
                $this->doQuery( 'ALTER SESSION SET NLS_TIMESTAMP_FORMAT=\'DD-MM-YYYY HH24:MI:SS.FF6\'' );
                $this->doQuery( 'ALTER SESSION SET NLS_TIMESTAMP_TZ_FORMAT=\'DD-MM-YYYY HH24:MI:SS.FF6\'' );
index a1a8061..6592c51 100644 (file)
@@ -505,6 +505,7 @@ class PostgresInstaller extends DatabaseInstaller {
                if ( !$status->isOK() ) {
                        return $status;
                }
+               /** @var DatabasePostgres $conn */
                $conn = $status->value;
 
                // Create the schema if necessary
index bc8883c..971257f 100644 (file)
@@ -74,7 +74,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        protected $delimiter = ';';
        /** @var string|bool|null Stashed value of html_errors INI setting */
        protected $htmlErrors;
-       /** @var int */
+       /** @var int Row batch size to use for emulated INSERT SELECT queries */
        protected $nonNativeInsertSelectBatchSize = 10000;
 
        /** @var BagOStuff APC cache */
@@ -93,14 +93,12 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        protected $trxProfiler;
        /** @var DatabaseDomain */
        protected $currentDomain;
+       /** @var object|resource|null Database connection */
+       protected $conn;
+
        /** @var IDatabase|null Lazy handle to the master DB this server replicates from */
        private $lazyMasterHandle;
 
-       /** @var object|resource|null Database connection */
-       protected $conn = null;
-       /** @var bool Whether a connection handle is open (connection itself might be dead) */
-       protected $opened = false;
-
        /** @var array Map of (name => 1) for locks obtained via lock() */
        protected $sessionNamedLocks = [];
        /** @var array Map of (table name => 1) for TEMPORARY tables */
@@ -308,7 +306,6 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         * @param string $dbName Database name
         * @param string|null $schema Database schema name
         * @param string $tablePrefix Table prefix
-        * @return bool
         * @throws DBConnectionError
         */
        abstract protected function open( $server, $user, $password, $dbName, $schema, $tablePrefix );
@@ -721,7 +718,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        public function isOpen() {
-               return $this->opened;
+               return (bool)$this->conn;
        }
 
        public function setFlag( $flag, $remember = self::REMEMBER_NOTHING ) {
@@ -865,7 +862,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        final public function close() {
                $exception = null; // error to throw after disconnecting
 
-               $wasOpen = $this->opened;
+               $wasOpen = (bool)$this->conn;
                // This should mostly do nothing if the connection is already closed
                if ( $this->conn ) {
                        // Roll back any dangling transaction first
@@ -914,7 +911,6 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                }
 
                $this->conn = false;
-               $this->opened = false;
 
                // Throw any unexpected errors after having disconnected
                if ( $exception instanceof Exception ) {
@@ -978,16 +974,6 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         */
        abstract protected function closeConnection();
 
-       /**
-        * @deprecated since 1.32
-        * @param string $error Fallback message, if none is given by DB
-        * @throws DBConnectionError
-        */
-       public function reportConnectionError( $error = 'Unknown error' ) {
-               call_user_func( $this->deprecationLogger, 'Use of ' . __METHOD__ . ' is deprecated.' );
-               throw new DBConnectionError( $this, $this->lastError() ?: $error );
-       }
-
        /**
         * Run a query and return a DBMS-dependent wrapper or boolean
         *
@@ -1194,8 +1180,10 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                // Send the query to the server and fetch any corresponding errors
                list( $ret, $err, $errno, $recoverableSR, $recoverableCL, $reconnected ) =
                        $this->executeQueryAttempt( $sql, $commentedSql, $isPermWrite, $fname, $flags );
+
                // Check if the query failed due to a recoverable connection loss
-               if ( $ret === false && $recoverableCL && $reconnected ) {
+               $allowRetry = !$this->hasFlags( $flags, self::QUERY_NO_RETRY );
+               if ( $ret === false && $recoverableCL && $reconnected && $allowRetry ) {
                        // Silently resend the query to the server since it is safe and possible
                        list( $ret, $err, $errno, $recoverableSR, $recoverableCL ) =
                                $this->executeQueryAttempt( $sql, $commentedSql, $isPermWrite, $fname, $flags );
@@ -4134,7 +4122,6 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         */
        protected function replaceLostConnection( $fname ) {
                $this->closeConnection();
-               $this->opened = false;
                $this->conn = false;
 
                $this->handleSessionLossPreconnect();
@@ -4708,7 +4695,6 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 
                if ( $this->isOpen() ) {
                        // Open a new connection resource without messing with the old one
-                       $this->opened = false;
                        $this->conn = false;
                        $this->trxEndCallbacks = []; // don't copy
                        $this->handleSessionLossPreconnect(); // no trx or locks anymore
@@ -4755,7 +4741,6 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                        $this->closeConnection();
                        Wikimedia\restoreWarnings();
                        $this->conn = false;
-                       $this->opened = false;
                }
        }
 }
index 5632027..6c003dd 100644 (file)
@@ -27,9 +27,9 @@
 
 namespace Wikimedia\Rdbms;
 
-use Wikimedia;
 use Exception;
 use stdClass;
+use Wikimedia\AtEase\AtEase;
 
 /**
  * @ingroup Database
@@ -78,7 +78,7 @@ class DatabaseMssql extends Database {
        }
 
        protected function open( $server, $user, $password, $dbName, $schema, $tablePrefix ) {
-               # Test for driver support, to avoid suppressed fatal error
+               // Test for driver support, to avoid suppressed fatal error
                if ( !function_exists( 'sqlsrv_connect' ) ) {
                        throw new DBConnectionError(
                                $this,
@@ -87,11 +87,6 @@ class DatabaseMssql extends Database {
                        );
                }
 
-               # e.g. the class is being loaded
-               if ( !strlen( $user ) ) {
-                       return null;
-               }
-
                $this->close();
                $this->server = $server;
                $this->user = $user;
@@ -110,15 +105,19 @@ class DatabaseMssql extends Database {
                        $connectionInfo['PWD'] = $password;
                }
 
-               Wikimedia\suppressWarnings();
+               AtEase::suppressWarnings();
                $this->conn = sqlsrv_connect( $server, $connectionInfo );
-               Wikimedia\restoreWarnings();
+               AtEase::restoreWarnings();
 
                if ( $this->conn === false ) {
-                       throw new DBConnectionError( $this, $this->lastError() );
+                       $error = $this->lastError();
+                       $this->connLogger->error(
+                               "Error connecting to {db_server}: {error}",
+                               $this->getLogContext( [ 'method' => __METHOD__, 'error' => $error ] )
+                       );
+                       throw new DBConnectionError( $this, $error );
                }
 
-               $this->opened = true;
                $this->currentDomain = new DatabaseDomain(
                        ( $dbName != '' ) ? $dbName : null,
                        null,
index ef28f33..e3c2268 100644 (file)
@@ -122,9 +122,12 @@ abstract class DatabaseMysqlBase extends Database {
        }
 
        protected function open( $server, $user, $password, $dbName, $schema, $tablePrefix ) {
-               # Close/unset connection handle
                $this->close();
 
+               if ( $schema !== null ) {
+                       throw new DBExpectedError( $this, __CLASS__ . ": domain schemas are not supported." );
+               }
+
                $this->server = $server;
                $this->user = $user;
                $this->password = $password;
@@ -143,88 +146,52 @@ abstract class DatabaseMysqlBase extends Database {
                        $error = $error ?: $this->lastError();
                        $this->connLogger->error(
                                "Error connecting to {db_server}: {error}",
-                               $this->getLogContext( [
-                                       'method' => __METHOD__,
-                                       'error' => $error,
-                               ] )
+                               $this->getLogContext( [ 'method' => __METHOD__, 'error' => $error ] )
                        );
                        $this->connLogger->debug( "DB connection error\n" .
                                "Server: $server, User: $user, Password: " .
                                substr( $password, 0, 3 ) . "..., error: " . $error . "\n" );
-
                        throw new DBConnectionError( $this, $error );
                }
 
-               if ( strlen( $dbName ) ) {
-                       $this->selectDomain( new DatabaseDomain( $dbName, null, $tablePrefix ) );
-               } else {
-                       $this->currentDomain = new DatabaseDomain( null, null, $tablePrefix );
-               }
-
-               // Tell the server what we're communicating with
-               if ( !$this->connectInitCharset() ) {
-                       $error = $this->lastError();
-                       $this->queryLogger->error(
-                               "Error setting character set: {error}",
-                               $this->getLogContext( [
-                                       'method' => __METHOD__,
-                                       'error' => $this->lastError(),
-                               ] )
+               try {
+                       $this->currentDomain = new DatabaseDomain(
+                               strlen( $dbName ) ? $dbName : null,
+                               null,
+                               $tablePrefix
                        );
-                       throw new DBConnectionError( $this, "Error setting character set: $error" );
-               }
 
-               // Abstract over any insane MySQL defaults
-               $set = [ 'group_concat_max_len = 262144' ];
-               // Set SQL mode, default is turning them all off, can be overridden or skipped with null
-               if ( is_string( $this->sqlMode ) ) {
-                       $set[] = 'sql_mode = ' . $this->addQuotes( $this->sqlMode );
-               }
-               // Set any custom settings defined by site config
-               // (e.g. https://dev.mysql.com/doc/refman/4.1/en/innodb-parameters.html)
-               foreach ( $this->connectionVariables as $var => $val ) {
-                       // Escape strings but not numbers to avoid MySQL complaining
-                       if ( !is_int( $val ) && !is_float( $val ) ) {
-                               $val = $this->addQuotes( $val );
+                       // Abstract over any insane MySQL defaults
+                       $set = [ 'group_concat_max_len = 262144' ];
+                       // Set SQL mode, default is turning them all off, can be overridden or skipped with null
+                       if ( is_string( $this->sqlMode ) ) {
+                               $set[] = 'sql_mode = ' . $this->addQuotes( $this->sqlMode );
+                       }
+                       // Set any custom settings defined by site config
+                       // (e.g. https://dev.mysql.com/doc/refman/4.1/en/innodb-parameters.html)
+                       foreach ( $this->connectionVariables as $var => $val ) {
+                               // Escape strings but not numbers to avoid MySQL complaining
+                               if ( !is_int( $val ) && !is_float( $val ) ) {
+                                       $val = $this->addQuotes( $val );
+                               }
+                               $set[] = $this->addIdentifierQuotes( $var ) . ' = ' . $val;
                        }
-                       $set[] = $this->addIdentifierQuotes( $var ) . ' = ' . $val;
-               }
 
-               if ( $set ) {
-                       // Use doQuery() to avoid opening implicit transactions (DBO_TRX)
-                       $success = $this->doQuery( 'SET ' . implode( ', ', $set ) );
-                       if ( !$success ) {
-                               $error = $this->lastError();
-                               $this->queryLogger->error(
-                                       'Error setting MySQL variables on server {db_server}: {error}',
-                                       $this->getLogContext( [
-                                               'method' => __METHOD__,
-                                               'error' => $error,
-                                       ] )
+                       if ( $set ) {
+                               $this->query(
+                                       'SET ' . implode( ', ', $set ),
+                                       __METHOD__,
+                                       self::QUERY_IGNORE_DBO_TRX | self::QUERY_NO_RETRY
                                );
-                               throw new DBConnectionError( $this, "Error setting MySQL variables: $error" );
                        }
+               } catch ( Exception $e ) {
+                       // Connection was not fully initialized and is not safe for use
+                       $this->conn = false;
                }
 
-               $this->opened = true;
-
                return true;
        }
 
-       /**
-        * Set the character set information right after connection
-        * @return bool
-        */
-       protected function connectInitCharset() {
-               if ( $this->utf8Mode ) {
-                       // Tell the server we're communicating with it in UTF-8.
-                       // This may engage various charset conversions.
-                       return $this->mysqlSetCharset( 'utf8' );
-               } else {
-                       return $this->mysqlSetCharset( 'binary' );
-               }
-       }
-
        protected function doSelectDomain( DatabaseDomain $domain ) {
                if ( $domain->getSchema() !== null ) {
                        throw new DBExpectedError( $this, __CLASS__ . ": domain schemas are not supported." );
@@ -269,14 +236,6 @@ abstract class DatabaseMysqlBase extends Database {
         */
        abstract protected function mysqlConnect( $realServer, $dbName );
 
-       /**
-        * Set the character set of the MySQL link
-        *
-        * @param string $charset
-        * @return bool
-        */
-       abstract protected function mysqlSetCharset( $charset );
-
        /**
         * @param IResultWrapper|resource $res
         * @throws DBUnexpectedError
index 703c64d..0f444cd 100644 (file)
@@ -125,21 +125,6 @@ class DatabaseMysqli extends DatabaseMysqlBase {
                return false;
        }
 
-       protected function connectInitCharset() {
-               // already done in mysqlConnect()
-               return true;
-       }
-
-       /**
-        * @param string $charset
-        * @return bool
-        */
-       protected function mysqlSetCharset( $charset ) {
-               $conn = $this->getBindingHandle();
-
-               return $conn->set_charset( $charset );
-       }
-
        /**
         * @return bool
         */
index d8be62f..a19a1a4 100644 (file)
@@ -87,7 +87,7 @@ class DatabasePostgres extends Database {
        }
 
        protected function open( $server, $user, $password, $dbName, $schema, $tablePrefix ) {
-               # Test for Postgres support, to avoid suppressed fatal error
+               // Test for Postgres support, to avoid suppressed fatal error
                if ( !function_exists( 'pg_connect' ) ) {
                        throw new DBConnectionError(
                                $this,
@@ -143,23 +143,36 @@ class DatabasePostgres extends Database {
                        throw new DBConnectionError( $this, str_replace( "\n", ' ', $phpError ) );
                }
 
-               $this->opened = true;
+               try {
+                       // If called from the command-line (e.g. importDump), only show errors.
+                       // No transaction should be open at this point, so the problem of the SET
+                       // effects being rolled back should not be an issue.
+                       // See https://www.postgresql.org/docs/8.3/sql-set.html
+                       $variables = [];
+                       if ( $this->cliMode ) {
+                               $variables['client_min_messages'] = 'ERROR';
+                       }
+                       $variables += [
+                               'client_encoding' => 'UTF8',
+                               'datestyle' => 'ISO, YMD',
+                               'timezone' => 'GMT',
+                               'standard_conforming_strings' => 'on',
+                               'bytea_output' => 'escape'
+                       ];
+                       foreach ( $variables as $var => $val ) {
+                               $this->query(
+                                       'SET ' . $this->addIdentifierQuotes( $var ) . ' = ' . $this->addQuotes( $val ),
+                                       __METHOD__,
+                                       self::QUERY_IGNORE_DBO_TRX | self::QUERY_NO_RETRY
+                               );
+                       }
 
-               # If called from the command-line (e.g. importDump), only show errors
-               if ( $this->cliMode ) {
-                       $this->doQuery( "SET client_min_messages = 'ERROR'" );
+                       $this->determineCoreSchema( $schema );
+                       $this->currentDomain = new DatabaseDomain( $dbName, $schema, $tablePrefix );
+               } catch ( Exception $e ) {
+                       // Connection was not fully initialized and is not safe for use
+                       $this->conn = false;
                }
-
-               $this->query( "SET client_encoding='UTF8'", __METHOD__ );
-               $this->query( "SET datestyle = 'ISO, YMD'", __METHOD__ );
-               $this->query( "SET timezone = 'GMT'", __METHOD__ );
-               $this->query( "SET standard_conforming_strings = on", __METHOD__ );
-               $this->query( "SET bytea_output = 'escape'", __METHOD__ ); // PHP bug 53127
-
-               $this->determineCoreSchema( $schema );
-               $this->currentDomain = new DatabaseDomain( $dbName, $schema, $tablePrefix );
-
-               return (bool)$this->conn;
        }
 
        protected function relationSchemaQualifier() {
@@ -981,7 +994,7 @@ __INDEXATTR__;
         * @return string Default schema for the current session
         */
        public function getCurrentSchema() {
-               $res = $this->query( "SELECT current_schema()", __METHOD__ );
+               $res = $this->query( "SELECT current_schema()", __METHOD__, self::QUERY_IGNORE_DBO_TRX );
                $row = $this->fetchRow( $res );
 
                return $row[0];
@@ -998,7 +1011,11 @@ __INDEXATTR__;
         * @return array List of actual schemas for the current sesson
         */
        public function getSchemas() {
-               $res = $this->query( "SELECT current_schemas(false)", __METHOD__ );
+               $res = $this->query(
+                       "SELECT current_schemas(false)",
+                       __METHOD__,
+                       self::QUERY_IGNORE_DBO_TRX
+               );
                $row = $this->fetchRow( $res );
                $schemas = [];
 
@@ -1017,7 +1034,7 @@ __INDEXATTR__;
         * @return array How to search for table names schemas for the current user
         */
        public function getSearchPath() {
-               $res = $this->query( "SHOW search_path", __METHOD__ );
+               $res = $this->query( "SHOW search_path", __METHOD__, self::QUERY_IGNORE_DBO_TRX );
                $row = $this->fetchRow( $res );
 
                /* PostgreSQL returns SHOW values as strings */
@@ -1033,7 +1050,11 @@ __INDEXATTR__;
         * @param array $search_path List of schemas to be searched by default
         */
        private function setSearchPath( $search_path ) {
-               $this->query( "SET search_path = " . implode( ", ", $search_path ) );
+               $this->query(
+                       "SET search_path = " . implode( ", ", $search_path ),
+                       __METHOD__,
+                       self::QUERY_IGNORE_DBO_TRX
+               );
        }
 
        /**
@@ -1051,7 +1072,15 @@ __INDEXATTR__;
         * @param string $desiredSchema
         */
        public function determineCoreSchema( $desiredSchema ) {
-               $this->begin( __METHOD__, self::TRANSACTION_INTERNAL );
+               if ( $this->trxLevel ) {
+                       // We do not want the schema selection to change on ROLLBACK or INSERT SELECT.
+                       // See https://www.postgresql.org/docs/8.3/sql-set.html
+                       throw new DBUnexpectedError(
+                               $this,
+                               __METHOD__ . ": a transaction is currently active."
+                       );
+               }
+
                if ( $this->schemaExists( $desiredSchema ) ) {
                        if ( in_array( $desiredSchema, $this->getSchemas() ) ) {
                                $this->coreSchema = $desiredSchema;
@@ -1064,8 +1093,7 @@ __INDEXATTR__;
                                 * Fixes T17816
                                 */
                                $search_path = $this->getSearchPath();
-                               array_unshift( $search_path,
-                                       $this->addIdentifierQuotes( $desiredSchema ) );
+                               array_unshift( $search_path, $this->addIdentifierQuotes( $desiredSchema ) );
                                $this->setSearchPath( $search_path );
                                $this->coreSchema = $desiredSchema;
                                $this->queryLogger->debug(
@@ -1077,8 +1105,6 @@ __INDEXATTR__;
                                "Schema \"" . $desiredSchema . "\" not found, using current \"" .
                                $this->coreSchema . "\"\n" );
                }
-               /* Commit SET otherwise it will be rollbacked on error or IGNORE SELECT */
-               $this->commit( __METHOD__, self::FLUSHING_INTERNAL );
        }
 
        /**
@@ -1243,10 +1269,14 @@ SQL;
                        return false; // short-circuit
                }
 
-               $exists = $this->selectField(
-                       '"pg_catalog"."pg_namespace"', 1, [ 'nspname' => $schema ], __METHOD__ );
+               $res = $this->query(
+                       "SELECT 1 FROM pg_catalog.pg_namespace " .
+                       "WHERE nspname = " . $this->addQuotes( $schema ) . " LIMIT 1",
+                       __METHOD__,
+                       self::QUERY_IGNORE_DBO_TRX
+               );
 
-               return (bool)$exists;
+               return ( $this->numRows( $res ) > 0 );
        }
 
        /**
index aff3774..46c34b4 100644 (file)
@@ -168,15 +168,23 @@ class DatabaseSqlite extends Database {
 
        protected function open( $server, $user, $pass, $dbName, $schema, $tablePrefix ) {
                $this->close();
+
+               if ( $schema !== null ) {
+                       throw new DBExpectedError( $this, __CLASS__ . ": domain schemas are not supported." );
+               }
+
                $fileName = self::generateFileName( $this->dbDir, $dbName );
                if ( !is_readable( $fileName ) ) {
-                       $this->conn = false;
-                       throw new DBConnectionError( $this, "SQLite database not accessible" );
+                       $error = "SQLite database file not readable";
+                       $this->connLogger->error(
+                               "Error connecting to {db_server}: {error}",
+                               $this->getLogContext( [ 'method' => __METHOD__, 'error' => $error ] )
+                       );
+                       throw new DBConnectionError( $this, $error );
                }
+
                // Only $dbName is used, the other parameters are irrelevant for SQLite databases
                $this->openFile( $fileName, $dbName, $tablePrefix );
-
-               return (bool)$this->conn;
        }
 
        /**
@@ -186,45 +194,48 @@ class DatabaseSqlite extends Database {
         * @param string $dbName
         * @param string $tablePrefix
         * @throws DBConnectionError
-        * @return PDO|bool SQL connection or false if failed
         */
        protected function openFile( $fileName, $dbName, $tablePrefix ) {
-               $err = false;
-
                $this->dbPath = $fileName;
                try {
-                       if ( $this->flags & self::DBO_PERSISTENT ) {
-                               $this->conn = new PDO( "sqlite:$fileName", '', '',
-                                       [ PDO::ATTR_PERSISTENT => true ] );
-                       } else {
-                               $this->conn = new PDO( "sqlite:$fileName", '', '' );
-                       }
+                       $this->conn = new PDO(
+                               "sqlite:$fileName",
+                               '',
+                               '',
+                               [ PDO::ATTR_PERSISTENT => (bool)( $this->flags & self::DBO_PERSISTENT ) ]
+                       );
+                       $error = 'unknown error';
                } catch ( PDOException $e ) {
-                       $err = $e->getMessage();
+                       $error = $e->getMessage();
                }
 
                if ( !$this->conn ) {
-                       $this->queryLogger->debug( "DB connection error: $err\n" );
-                       throw new DBConnectionError( $this, $err );
+                       $this->connLogger->error(
+                               "Error connecting to {db_server}: {error}",
+                               $this->getLogContext( [ 'method' => __METHOD__, 'error' => $error ] )
+                       );
+                       throw new DBConnectionError( $this, $error );
                }
 
-               $this->opened = is_object( $this->conn );
-               if ( $this->opened ) {
-                       $this->currentDomain = new DatabaseDomain( $dbName, null, $tablePrefix );
-                       # Set error codes only, don't raise exceptions
+               try {
+                       // Set error codes only, don't raise exceptions
                        $this->conn->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_SILENT );
-                       # Enforce LIKE to be case sensitive, just like MySQL
-                       $this->query( 'PRAGMA case_sensitive_like = 1' );
 
+                       $this->currentDomain = new DatabaseDomain( $dbName, null, $tablePrefix );
+
+                       $flags = self::QUERY_IGNORE_DBO_TRX | self::QUERY_NO_RETRY;
+                       // Enforce LIKE to be case sensitive, just like MySQL
+                       $this->query( 'PRAGMA case_sensitive_like = 1', __METHOD__, $flags );
+                       // Apply an optimizations or requirements regarding fsync() usage
                        $sync = $this->connectionVariables['synchronous'] ?? null;
                        if ( in_array( $sync, [ 'EXTRA', 'FULL', 'NORMAL', 'OFF' ], true ) ) {
-                               $this->query( "PRAGMA synchronous = $sync" );
+                               $this->query( "PRAGMA synchronous = $sync", __METHOD__ );
                        }
-
-                       return $this->conn;
+               } catch ( Exception $e ) {
+                       // Connection was not fully initialized and is not safe for use
+                       $this->conn = false;
+                       throw $e;
                }
-
-               return false;
        }
 
        /**
index 037ae99..a462916 100644 (file)
@@ -106,6 +106,8 @@ interface IDatabase {
        /** @var int Enable compression in connection protocol */
        const DBO_COMPRESS = 512;
 
+       /** @var int Idiom for "no special flags" */
+       const QUERY_NORMAL = 0;
        /** @var int Ignore query errors and return false when they happen */
        const QUERY_SILENCE_ERRORS = 1; // b/c for 1.32 query() argument; note that (int)true = 1
        /**
@@ -117,6 +119,8 @@ interface IDatabase {
        const QUERY_REPLICA_ROLE = 4;
        /** @var int Ignore the current presence of any DBO_TRX flag */
        const QUERY_IGNORE_DBO_TRX = 8;
+       /** @var int Do not try to retry the query if the connection was lost */
+       const QUERY_NO_RETRY = 16;
 
        /** @var bool Parameter to unionQueries() for UNION ALL */
        const UNION_ALL = true;
index fb4041d..43e7075 100644 (file)
@@ -229,10 +229,6 @@ class DatabaseTestHelper extends Database {
                return 'test';
        }
 
-       function isOpen() {
-               return $this->conn ? true : false;
-       }
-
        function ping( &$rtt = null ) {
                $rtt = 0.0;
                return true;