From 7911da9c6f3a027925139f2831dd674e3bcf35aa Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Thu, 13 Jun 2019 13:46:03 +0100 Subject: [PATCH] rdbms: remove $opened field from Database for simplicity 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 | 1 + includes/db/DatabaseOracle.php | 9 +- includes/installer/PostgresInstaller.php | 1 + includes/libs/rdbms/database/Database.php | 33 ++---- .../libs/rdbms/database/DatabaseMssql.php | 21 ++-- .../libs/rdbms/database/DatabaseMysqlBase.php | 105 ++++++------------ .../libs/rdbms/database/DatabaseMysqli.php | 15 --- .../libs/rdbms/database/DatabasePostgres.php | 86 +++++++++----- .../libs/rdbms/database/DatabaseSqlite.php | 65 ++++++----- includes/libs/rdbms/database/IDatabase.php | 4 + .../includes/db/DatabaseTestHelper.php | 4 - 11 files changed, 157 insertions(+), 187 deletions(-) diff --git a/RELEASE-NOTES-1.34 b/RELEASE-NOTES-1.34 index d34b45959b..a79f57ac56 100644 --- a/RELEASE-NOTES-1.34 +++ b/RELEASE-NOTES-1.34 @@ -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 === diff --git a/includes/db/DatabaseOracle.php b/includes/db/DatabaseOracle.php index 5df7aef620..4af62a0b05 100644 --- a/includes/db/DatabaseOracle.php +++ b/includes/db/DatabaseOracle.php @@ -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\'' ); diff --git a/includes/installer/PostgresInstaller.php b/includes/installer/PostgresInstaller.php index a1a80612ab..6592c51420 100644 --- a/includes/installer/PostgresInstaller.php +++ b/includes/installer/PostgresInstaller.php @@ -505,6 +505,7 @@ class PostgresInstaller extends DatabaseInstaller { if ( !$status->isOK() ) { return $status; } + /** @var DatabasePostgres $conn */ $conn = $status->value; // Create the schema if necessary diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php index bc8883c3ac..971257f500 100644 --- a/includes/libs/rdbms/database/Database.php +++ b/includes/libs/rdbms/database/Database.php @@ -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; } } } diff --git a/includes/libs/rdbms/database/DatabaseMssql.php b/includes/libs/rdbms/database/DatabaseMssql.php index 56320273f6..6c003dd5a7 100644 --- a/includes/libs/rdbms/database/DatabaseMssql.php +++ b/includes/libs/rdbms/database/DatabaseMssql.php @@ -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, diff --git a/includes/libs/rdbms/database/DatabaseMysqlBase.php b/includes/libs/rdbms/database/DatabaseMysqlBase.php index ef28f33ac6..e3c2268f91 100644 --- a/includes/libs/rdbms/database/DatabaseMysqlBase.php +++ b/includes/libs/rdbms/database/DatabaseMysqlBase.php @@ -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 diff --git a/includes/libs/rdbms/database/DatabaseMysqli.php b/includes/libs/rdbms/database/DatabaseMysqli.php index 703c64d7c0..0f444cd210 100644 --- a/includes/libs/rdbms/database/DatabaseMysqli.php +++ b/includes/libs/rdbms/database/DatabaseMysqli.php @@ -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 */ diff --git a/includes/libs/rdbms/database/DatabasePostgres.php b/includes/libs/rdbms/database/DatabasePostgres.php index d8be62f1f4..a19a1a469b 100644 --- a/includes/libs/rdbms/database/DatabasePostgres.php +++ b/includes/libs/rdbms/database/DatabasePostgres.php @@ -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 ); } /** diff --git a/includes/libs/rdbms/database/DatabaseSqlite.php b/includes/libs/rdbms/database/DatabaseSqlite.php index aff3774d7e..46c34b4ad5 100644 --- a/includes/libs/rdbms/database/DatabaseSqlite.php +++ b/includes/libs/rdbms/database/DatabaseSqlite.php @@ -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; } /** diff --git a/includes/libs/rdbms/database/IDatabase.php b/includes/libs/rdbms/database/IDatabase.php index 037ae999f8..a4629164a6 100644 --- a/includes/libs/rdbms/database/IDatabase.php +++ b/includes/libs/rdbms/database/IDatabase.php @@ -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; diff --git a/tests/phpunit/includes/db/DatabaseTestHelper.php b/tests/phpunit/includes/db/DatabaseTestHelper.php index fb4041dd92..43e7075ee3 100644 --- a/tests/phpunit/includes/db/DatabaseTestHelper.php +++ b/tests/phpunit/includes/db/DatabaseTestHelper.php @@ -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; -- 2.20.1