From: Aaron Schulz <aschulz@wikimedia.org>
Date: Thu, 11 Jul 2019 02:35:46 +0000 (-0700)
Subject: rdbms: normalize Database open() code and error handling
X-Git-Tag: 1.34.0-rc.0~869
X-Git-Url: http://git.cyclocoop.org/%7B%24www_url%7Dadmin/%22%24ccApp/ecrire?a=commitdiff_plain;h=023c73f6128ab2c761f37358f125b6108fd102f5;p=lhc%2Fweb%2Fwiklou.git

rdbms: normalize Database open() code and error handling

Mainly:
* Use oci_new_connect() for Oracle to avoid broken connection reuse
  similar to the PGSQL_CONNECT_FORCE_NEW flag in DatabasePostgres
* Set 'client_min_messages' unconditionally for PostgreSQL
* Factor out Database::getConnectExceptionAndLog() helper method
* Use the same style of query() calls in DatabaseOracle::open() as
  the other subclasses
* Make sure the Database driver handle field is null on failure
  instead of false for sanity

Also:
* Disallow changing of Database handle DBO_* flags after construction
  where it does not make sense to change them
* Do not mention DBO_* flags meant for non-config use in $wgDBservers
* Ignore DBO_PERSISTENT for SQLite if DBO_TRX is also set for sanity
* Remove $wgDBOracleDRCP variable to discourage careless automatic
  setting of DBO_PERSISTENT that breaks LoadBalancer assumptions

Change-Id: Iea948f7f872294ea8fc5d897fc10c9d29b7141d5
---

diff --git a/RELEASE-NOTES-1.34 b/RELEASE-NOTES-1.34
index 368d2a4731..4bd655907c 100644
--- a/RELEASE-NOTES-1.34
+++ b/RELEASE-NOTES-1.34
@@ -72,6 +72,8 @@ For notes on 1.33.x and older releases, see HISTORY.
   configurable via $wgDebugLogFile.
 * $wgPasswordSalt – This setting, used for migrating exceptionally old, insecure
   password setups and deprecated since 1.24, is now removed.
+* $wgDBOracleDRCP - If you must use persistent connections, set DBO_PERSISTENT
+  in the 'flags' field for servers in $wgDBServers (or $wgLBFactoryConf).
 
 === New user-facing features in 1.34 ===
 * Special:Mute has been added as a quick way for users to block unwanted emails
diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index cf8491a3bb..608ef6ae55 100644
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -2052,23 +2052,21 @@ $wgSharedSchema = false;
  *                  sent to it. It will be excluded from lag checks in maintenance scripts.
  *                  The only way it can receive traffic is if groupLoads is used.
  *
- *   - groupLoads:  array of load ratios, the key is the query group name. A query may belong
- *                  to several groups, the most specific group defined here is used.
- *
- *   - flags:       bit field
- *                  - DBO_DEFAULT -- turns on DBO_TRX only if "cliMode" is off (recommended)
- *                  - DBO_DEBUG -- equivalent of $wgDebugDumpSql
- *                  - DBO_TRX -- wrap entire request in a transaction
- *                  - DBO_NOBUFFER -- turn off buffering (not useful in LocalSettings.php)
- *                  - DBO_PERSISTENT -- enables persistent database connections
- *                  - DBO_SSL -- uses SSL/TLS encryption in database connections, if available
- *                  - DBO_COMPRESS -- uses internal compression in database connections,
- *                                    if available
+ *   - groupLoads:  (optional) Array of load ratios, the key is the query group name. A query
+ *                  may belong to several groups, the most specific group defined here is used.
+ *
+ *   - flags:       (optional) Bit field of properties:
+ *                  - DBO_DEFAULT:    Transactionalize web requests and use autocommit otherwise
+ *                  - DBO_DEBUG:      Equivalent of $wgDebugDumpSql
+ *                  - DBO_SSL:        Use TLS connection encryption if available
+ *                  - DBO_COMPRESS:   Use protocol compression with database connections
+ *                  - DBO_PERSISTENT: Enables persistent database connections
  *
  *   - max lag:     (optional) Maximum replication lag before a replica DB goes out of rotation
  *   - is static:   (optional) Set to true if the dataset is static and no replication is used.
  *   - cliMode:     (optional) Connection handles will not assume that requests are short-lived
  *                  nor that INSERT..SELECT can be rewritten into a buffered SELECT and INSERT.
+ *                  This is what DBO_DEFAULT uses to determine when a web request is present.
  *                  [Default: uses value of $wgCommandLineMode]
  *
  *   These and any other user-defined properties will be assigned to the mLBInfo member
@@ -2138,34 +2136,6 @@ $wgDBerrorLog = false;
  */
 $wgDBerrorLogTZ = false;
 
-/**
- * Set true to enable Oracle DCRP (supported from 11gR1 onward)
- *
- * To use this feature set to true and use a datasource defined as
- * POOLED (i.e. in tnsnames definition set server=pooled in connect_data
- * block).
- *
- * Starting from 11gR1 you can use DCRP (Database Resident Connection
- * Pool) that maintains established sessions and reuses them on new
- * connections.
- *
- * Not completely tested, but it should fall back on normal connection
- * in case the pool is full or the datasource is not configured as
- * pooled.
- * And the other way around; using oci_pconnect on a non pooled
- * datasource should produce a normal connection.
- *
- * When it comes to frequent shortlived DB connections like with MW
- * Oracle tends to s***. The problem is the driver connects to the
- * database reasonably fast, but establishing a session takes time and
- * resources. MW does not rely on session state (as it does not use
- * features such as package variables) so establishing a valid session
- * is in this case an unwanted overhead that just slows things down.
- *
- * @warning EXPERIMENTAL!
- */
-$wgDBOracleDRCP = false;
-
 /**
  * Other wikis on this site, can be administered from a single developer account.
  *
diff --git a/includes/db/DatabaseOracle.php b/includes/db/DatabaseOracle.php
index 501f01a3e9..3c868dbf26 100644
--- a/includes/db/DatabaseOracle.php
+++ b/includes/db/DatabaseOracle.php
@@ -28,7 +28,6 @@ use Wikimedia\Rdbms\DatabaseDomain;
 use Wikimedia\Rdbms\Blob;
 use Wikimedia\Rdbms\ResultWrapper;
 use Wikimedia\Rdbms\IResultWrapper;
-use Wikimedia\Rdbms\DBConnectionError;
 use Wikimedia\Rdbms\DBUnexpectedError;
 use Wikimedia\Rdbms\DBExpectedError;
 
@@ -90,91 +89,90 @@ class DatabaseOracle extends Database {
 
 	protected function open( $server, $user, $password, $dbName, $schema, $tablePrefix ) {
 		if ( !function_exists( 'oci_connect' ) ) {
-			throw new DBConnectionError(
-				$this,
+			throw $this->newExceptionAfterConnectError(
 				"Oracle functions missing, have you compiled PHP with the --with-oci8 option?\n " .
-					"(Note: if you recently installed PHP, you may need to restart your webserver\n " .
-					"and database)\n" );
+				"(Note: if you recently installed PHP, you may need to restart your webserver\n " .
+				"and database)"
+			);
 		}
 
+		$this->close();
+
 		if ( $schema !== null ) {
-			// We use the *database* aspect of $domain for schema, not the domain schema
-			throw new DBExpectedError(
-				$this,
-				__CLASS__ . ": cannot use schema '$schema'; " .
-				"the database component '$dbName' is actually interpreted as the Oracle schema."
+			// This uses the *database* aspect of $domain for schema, not the domain schema
+			throw $this->newExceptionAfterConnectError(
+				"Got schema '$schema'; not supported. " .
+				"The database component '$dbName' is actually interpreted as the Oracle schema."
 			);
 		}
 
-		$this->close();
 		$this->user = $user;
 		$this->password = $password;
-		if ( !$server ) {
-			// Backward compatibility (server used to be null and TNS was supplied in dbname)
+		if ( strlen( $server ) ) {
+			// Transparent Network Substrate (TNS) endpoint
+			$this->server = $server;
+			// Database name, defaulting to the user name
+			$realDatabase = strlen( $dbName ) ? $dbName : $user;
+		} else {
+			// Backward compatibility; $server used to be null and $dbName was the TNS
 			$this->server = $dbName;
 			$realDatabase = $user;
-		} else {
-			// $server now holds the TNS endpoint
-			$this->server = $server;
-			// $dbName is schema name if different from username
-			$realDatabase = $dbName ?: $user;
 		}
-
-		if ( !strlen( $user ) ) { # e.g. the class is being loaded
-			return null;
-		}
-
 		$session_mode = ( $this->flags & DBO_SYSDBA ) ? OCI_SYSDBA : OCI_DEFAULT;
 
-		Wikimedia\suppressWarnings();
-		if ( $this->flags & DBO_PERSISTENT ) {
-			$this->conn = oci_pconnect(
-				$this->user,
-				$this->password,
-				$this->server,
-				$this->defaultCharset,
-				$session_mode
-			);
-		} elseif ( $this->flags & DBO_DEFAULT ) {
-			$this->conn = oci_new_connect(
-				$this->user,
-				$this->password,
-				$this->server,
-				$this->defaultCharset,
-				$session_mode
-			);
-		} else {
-			$this->conn = oci_connect(
-				$this->user,
-				$this->password,
-				$this->server,
-				$this->defaultCharset,
-				$session_mode
-			);
-		}
-		Wikimedia\restoreWarnings();
-
-		if ( $this->user != $realDatabase ) {
-			// change current schema in session
-			$this->selectDB( $realDatabase );
-		} else {
-			$this->currentDomain = new DatabaseDomain(
-				$realDatabase,
-				null,
-				$tablePrefix
-			);
-		}
+		$this->installErrorHandler();
+		try {
+			$this->conn = $this->getFlag( DBO_PERSISTENT )
+				? oci_pconnect(
+					$this->user,
+					$this->password,
+					$this->server,
+					$this->defaultCharset,
+					$session_mode
+				)
+				: oci_new_connect(
+					$this->user,
+					$this->password,
+					$this->server,
+					$this->defaultCharset,
+					$session_mode
+				);
+		} catch ( Exception $e ) {
+			$this->restoreErrorHandler();
+			throw $this->newExceptionAfterConnectError( $e->getMessage() );
+		}
+		$error = $this->restoreErrorHandler();
 
 		if ( !$this->conn ) {
-			throw new DBConnectionError( $this, $this->lastError() );
+			throw $this->newExceptionAfterConnectError( $error ?: $this->lastError() );
 		}
 
-		# 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\'' );
-		$this->doQuery( 'ALTER SESSION SET NLS_NUMERIC_CHARACTERS=\'.,\'' );
-
-		return (bool)$this->conn;
+		try {
+			if ( $this->user != $realDatabase ) {
+				// Change current schema for the entire session
+				$this->selectDomain( new DatabaseDomain(
+					$realDatabase,
+					$this->currentDomain->getSchema(),
+					$this->currentDomain->getTablePrefix()
+				) );
+			} else {
+				$this->currentDomain = new DatabaseDomain( $realDatabase, null, $tablePrefix );
+			}
+			$set = [
+				'NLS_TIMESTAMP_FORMAT' => 'DD-MM-YYYY HH24:MI:SS.FF6',
+				'NLS_TIMESTAMP_TZ_FORMAT' => 'DD-MM-YYYY HH24:MI:SS.FF6',
+				'NLS_NUMERIC_CHARACTERS' => '.,'
+			];
+			foreach ( $set as $var => $val ) {
+				$this->query(
+					"ALTER SESSION SET {$var}=" . $this->addQuotes( $val ),
+					__METHOD__,
+					self::QUERY_IGNORE_DBO_TRX | self::QUERY_NO_RETRY
+				);
+			}
+		} catch ( Exception $e ) {
+			throw $this->newExceptionAfterConnectError( $e->getMessage() );
+		}
 	}
 
 	/**
diff --git a/includes/db/MWLBFactory.php b/includes/db/MWLBFactory.php
index 3d404d3c8e..0c17840e4a 100644
--- a/includes/db/MWLBFactory.php
+++ b/includes/db/MWLBFactory.php
@@ -212,9 +212,6 @@ abstract class MWLBFactory {
 		$flags = DBO_DEFAULT;
 		$flags |= $options->get( 'DebugDumpSql' ) ? DBO_DEBUG : 0;
 		$flags |= $options->get( 'DebugLogFile' ) ? DBO_DEBUG : 0;
-		if ( $server['type'] === 'oracle' ) {
-			$flags |= $options->get( 'DBOracleDRCP' ) ? DBO_PERSISTENT : 0;
-		}
 
 		$server += [
 			'tablePrefix' => $options->get( 'DBprefix' ),
diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index 60062fbc13..384168ea3a 100644
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -61,7 +61,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 	protected $cliMode;
 	/** @var string Agent name for query profiling */
 	protected $agent;
-	/** @var int Bitfield of class DBO_* constants */
+	/** @var int Bit field of class DBO_* constants */
 	protected $flags;
 	/** @var array LoadBalancer tracking information */
 	protected $lbInfo = [];
@@ -217,6 +217,18 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 	/** @var float Assume an insert of this many rows or less should be fast to replicate */
 	private static $SMALL_WRITE_ROWS = 100;
 
+	/** @var string[] List of DBO_* flags that can be changed after connection */
+	protected static $MUTABLE_FLAGS = [
+		'DBO_DEBUG',
+		'DBO_NOBUFFER',
+		'DBO_TRX',
+		'DBO_DDLMODE',
+	];
+	/** @var int Bit field of all DBO_* flags that can be changed after connection */
+	protected static $DBO_MUTABLE = (
+		self::DBO_DEBUG | self::DBO_NOBUFFER | self::DBO_TRX | self::DBO_DDLMODE
+	);
+
 	/**
 	 * @note exceptions for missing libraries/drivers should be thrown in initConnection()
 	 * @param array $params Parameters passed from Database::factory()
@@ -283,23 +295,18 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 	/**
 	 * Actually connect to the database over the wire (or to local files)
 	 *
-	 * @throws InvalidArgumentException
 	 * @throws DBConnectionError
 	 * @since 1.31
 	 */
 	protected function doInitConnection() {
-		if ( strlen( $this->connectionParams['user'] ) ) {
-			$this->open(
-				$this->connectionParams['host'],
-				$this->connectionParams['user'],
-				$this->connectionParams['password'],
-				$this->connectionParams['dbname'],
-				$this->connectionParams['schema'],
-				$this->connectionParams['tablePrefix']
-			);
-		} else {
-			throw new InvalidArgumentException( "No database user provided" );
-		}
+		$this->open(
+			$this->connectionParams['host'],
+			$this->connectionParams['user'],
+			$this->connectionParams['password'],
+			$this->connectionParams['dbname'],
+			$this->connectionParams['schema'],
+			$this->connectionParams['tablePrefix']
+		);
 	}
 
 	/**
@@ -335,7 +342,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 	 *      equivalent to a "database" in MySQL. Note that MySQL and SQLite do not use schemas.
 	 *   - tablePrefix : Optional table prefix that is implicitly added on to all table names
 	 *      recognized in queries. This can be used in place of schemas for handle site farms.
-	 *   - flags : Optional bitfield of DBO_* constants that define connection, protocol,
+	 *   - flags : Optional bit field of DBO_* constants that define connection, protocol,
 	 *      buffering, and transaction behavior. It is STRONGLY adviced to leave the DBO_DEFAULT
 	 *      flag in place UNLESS this this database simply acts as a key/value store.
 	 *   - driver: Optional name of a specific DB client driver. For MySQL, there is only the
@@ -741,24 +748,32 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 	}
 
 	public function setFlag( $flag, $remember = self::REMEMBER_NOTHING ) {
-		if ( ( $flag & self::DBO_IGNORE ) ) {
-			throw new UnexpectedValueException( "Modifying DBO_IGNORE is not allowed" );
+		if ( $flag & ~static::$DBO_MUTABLE ) {
+			throw new DBUnexpectedError(
+				$this,
+				"Got $flag (allowed: " . implode( ', ', static::$MUTABLE_FLAGS ) . ')'
+			);
 		}
 
 		if ( $remember === self::REMEMBER_PRIOR ) {
 			array_push( $this->priorFlags, $this->flags );
 		}
+
 		$this->flags |= $flag;
 	}
 
 	public function clearFlag( $flag, $remember = self::REMEMBER_NOTHING ) {
-		if ( ( $flag & self::DBO_IGNORE ) ) {
-			throw new UnexpectedValueException( "Modifying DBO_IGNORE is not allowed" );
+		if ( $flag & ~static::$DBO_MUTABLE ) {
+			throw new DBUnexpectedError(
+				$this,
+				"Got $flag (allowed: " . implode( ', ', static::$MUTABLE_FLAGS ) . ')'
+			);
 		}
 
 		if ( $remember === self::REMEMBER_PRIOR ) {
 			array_push( $this->priorFlags, $this->flags );
 		}
+
 		$this->flags &= ~$flag;
 	}
 
@@ -776,7 +791,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 	}
 
 	public function getFlag( $flag ) {
-		return (bool)( $this->flags & $flag );
+		return ( ( $this->flags & $flag ) === $flag );
 	}
 
 	/**
@@ -929,7 +944,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 			$closed = true; // already closed; nothing to do
 		}
 
-		$this->conn = false;
+		$this->conn = null;
 
 		// Throw any unexpected errors after having disconnected
 		if ( $exception instanceof Exception ) {
@@ -1177,7 +1192,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 	 *
 	 * @param string $sql Original SQL query
 	 * @param string $fname Name of the calling function
-	 * @param int $flags Bitfield of class QUERY_* constants
+	 * @param int $flags Bit field of class QUERY_* constants
 	 * @return array An n-tuple of:
 	 *   - mixed|bool: An object, resource, or true on success; false on failure
 	 *   - string: The result of calling lastError()
@@ -1265,7 +1280,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 	 * @param string $commentedSql SQL query with debugging/trace comment
 	 * @param bool $isPermWrite Whether the query is a (non-temporary table) write
 	 * @param string $fname Name of the calling function
-	 * @param int $flags Bitfield of class QUERY_* constants
+	 * @param int $flags Bit field of class QUERY_* constants
 	 * @return array An n-tuple of:
 	 *   - mixed|bool: An object, resource, or true on success; false on failure
 	 *   - string: The result of calling lastError()
@@ -1570,9 +1585,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 		if ( $ignore ) {
 			$this->queryLogger->debug( "SQL ERROR (ignored): $error" );
 		} else {
-			$exception = $this->getQueryExceptionAndLog( $error, $errno, $sql, $fname );
-
-			throw $exception;
+			throw $this->getQueryExceptionAndLog( $error, $errno, $sql, $fname );
 		}
 	}
 
@@ -1584,19 +1597,18 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 	 * @return DBError
 	 */
 	private function getQueryExceptionAndLog( $error, $errno, $sql, $fname ) {
-		$sql1line = mb_substr( str_replace( "\n", "\\n", $sql ), 0, 5 * 1024 );
 		$this->queryLogger->error(
 			"{fname}\t{db_server}\t{errno}\t{error}\t{sql1line}",
 			$this->getLogContext( [
 				'method' => __METHOD__,
 				'errno' => $errno,
 				'error' => $error,
-				'sql1line' => $sql1line,
+				'sql1line' => mb_substr( str_replace( "\n", "\\n", $sql ), 0, 5 * 1024 ),
 				'fname' => $fname,
 				'trace' => ( new RuntimeException() )->getTraceAsString()
 			] )
 		);
-		$this->queryLogger->debug( "SQL ERROR: " . $error . "" );
+
 		if ( $this->wasQueryTimeout( $error, $errno ) ) {
 			$e = new DBQueryTimeoutError( $this, $error, $errno, $sql, $fname );
 		} elseif ( $this->wasConnectionError( $errno ) ) {
@@ -1608,6 +1620,25 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 		return $e;
 	}
 
+	/**
+	 * @param string $error
+	 * @return DBConnectionError
+	 */
+	final protected function newExceptionAfterConnectError( $error ) {
+		// Connection was not fully initialized and is not safe for use
+		$this->conn = null;
+
+		$this->connLogger->error(
+			"Error connecting to {db_server} as user {db_user}: {error}",
+			$this->getLogContext( [
+				'error' => $error,
+				'trace' => ( new RuntimeException() )->getTraceAsString()
+			] )
+		);
+
+		return new DBConnectionError( $this, $error );
+	}
+
 	public function freeResult( $res ) {
 	}
 
@@ -4297,7 +4328,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 	 */
 	protected function replaceLostConnection( $fname ) {
 		$this->closeConnection();
-		$this->conn = false;
+		$this->conn = null;
 
 		$this->handleSessionLossPreconnect();
 
@@ -4876,7 +4907,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 
 		if ( $this->isOpen() ) {
 			// Open a new connection resource without messing with the old one
-			$this->conn = false;
+			$this->conn = null;
 			$this->trxEndCallbacks = []; // don't copy
 			$this->trxSectionCancelCallbacks = []; // don't copy
 			$this->handleSessionLossPreconnect(); // no trx or locks anymore
diff --git a/includes/libs/rdbms/database/DatabaseMssql.php b/includes/libs/rdbms/database/DatabaseMssql.php
index d06bcb9274..49df57b3b9 100644
--- a/includes/libs/rdbms/database/DatabaseMssql.php
+++ b/includes/libs/rdbms/database/DatabaseMssql.php
@@ -79,53 +79,50 @@ class DatabaseMssql extends Database {
 	}
 
 	protected function open( $server, $user, $password, $dbName, $schema, $tablePrefix ) {
-		// Test for driver support, to avoid suppressed fatal error
 		if ( !function_exists( 'sqlsrv_connect' ) ) {
 			throw new DBConnectionError(
 				$this,
-				"Microsoft SQL Server Native (sqlsrv) functions missing.
-				You can download the driver from: http://go.microsoft.com/fwlink/?LinkId=123470\n"
+				"Microsoft SQL Server Native (sqlsrv) functions missing.\n
+				You can download the driver from: http://go.microsoft.com/fwlink/?LinkId=123470"
 			);
 		}
 
 		$this->close();
+
+		if ( $schema !== null ) {
+			throw $this->newExceptionAfterConnectError( "Got schema '$schema'; not supported." );
+		}
+
 		$this->server = $server;
 		$this->user = $user;
 		$this->password = $password;
 
 		$connectionInfo = [];
-
-		if ( $dbName != '' ) {
+		if ( strlen( $dbName ) ) {
 			$connectionInfo['Database'] = $dbName;
 		}
-
-		// Decide which auth scenerio to use
-		// if we are using Windows auth, then don't add credentials to $connectionInfo
 		if ( !$this->useWindowsAuth ) {
 			$connectionInfo['UID'] = $user;
 			$connectionInfo['PWD'] = $password;
 		}
 
 		AtEase::suppressWarnings();
-		$this->conn = sqlsrv_connect( $server, $connectionInfo );
+		$this->conn = sqlsrv_connect( $server, $connectionInfo ) ?: null;
 		AtEase::restoreWarnings();
 
-		if ( $this->conn === false ) {
-			$error = $this->lastError();
-			$this->connLogger->error(
-				"Error connecting to {db_server}: {error}",
-				$this->getLogContext( [ 'method' => __METHOD__, 'error' => $error ] )
-			);
-			throw new DBConnectionError( $this, $error );
+		if ( !$this->conn ) {
+			throw $this->newExceptionAfterConnectError( $this->lastError() );
 		}
 
-		$this->currentDomain = new DatabaseDomain(
-			( $dbName != '' ) ? $dbName : null,
-			null,
-			$tablePrefix
-		);
-
-		return (bool)$this->conn;
+		try {
+			$this->currentDomain = new DatabaseDomain(
+				strlen( $dbName ) ? $dbName : null,
+				null,
+				$tablePrefix
+			);
+		} catch ( Exception $e ) {
+			throw $this->newExceptionAfterConnectError( $e->getMessage() );
+		}
 	}
 
 	/**
diff --git a/includes/libs/rdbms/database/DatabaseMysqlBase.php b/includes/libs/rdbms/database/DatabaseMysqlBase.php
index 1e3fa845a3..b1a88ed7b9 100644
--- a/includes/libs/rdbms/database/DatabaseMysqlBase.php
+++ b/includes/libs/rdbms/database/DatabaseMysqlBase.php
@@ -125,7 +125,7 @@ abstract class DatabaseMysqlBase extends Database {
 		$this->close();
 
 		if ( $schema !== null ) {
-			throw new DBExpectedError( $this, __CLASS__ . ": cannot use schemas ('$schema')" );
+			throw $this->newExceptionAfterConnectError( "Got schema '$schema'; not supported." );
 		}
 
 		$this->server = $server;
@@ -135,23 +135,14 @@ abstract class DatabaseMysqlBase extends Database {
 		$this->installErrorHandler();
 		try {
 			$this->conn = $this->mysqlConnect( $this->server, $dbName );
-		} catch ( Exception $ex ) {
+		} catch ( Exception $e ) {
 			$this->restoreErrorHandler();
-			throw $ex;
+			throw $this->newExceptionAfterConnectError( $e->getMessage() );
 		}
 		$error = $this->restoreErrorHandler();
 
-		# Always log connection errors
 		if ( !$this->conn ) {
-			$error = $error ?: $this->lastError();
-			$this->connLogger->error(
-				"Error connecting to {db_server}: {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 );
+			throw $this->newExceptionAfterConnectError( $error ?: $this->lastError() );
 		}
 
 		try {
@@ -160,7 +151,6 @@ abstract class DatabaseMysqlBase extends Database {
 				null,
 				$tablePrefix
 			);
-
 			// 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
@@ -185,11 +175,8 @@ abstract class DatabaseMysqlBase extends Database {
 				);
 			}
 		} catch ( Exception $e ) {
-			// Connection was not fully initialized and is not safe for use
-			$this->conn = false;
+			throw $this->newExceptionAfterConnectError( $e->getMessage() );
 		}
-
-		return true;
 	}
 
 	protected function doSelectDomain( DatabaseDomain $domain ) {
@@ -234,7 +221,7 @@ abstract class DatabaseMysqlBase extends Database {
 	 *
 	 * @param string $realServer
 	 * @param string|null $dbName
-	 * @return mixed Raw connection
+	 * @return mixed|null Driver connection handle
 	 * @throws DBConnectionError
 	 */
 	abstract protected function mysqlConnect( $realServer, $dbName );
diff --git a/includes/libs/rdbms/database/DatabaseMysqli.php b/includes/libs/rdbms/database/DatabaseMysqli.php
index 0f444cd210..ddb39446b4 100644
--- a/includes/libs/rdbms/database/DatabaseMysqli.php
+++ b/includes/libs/rdbms/database/DatabaseMysqli.php
@@ -54,14 +54,14 @@ class DatabaseMysqli extends DatabaseMysqlBase {
 	/**
 	 * @param string $realServer
 	 * @param string|null $dbName
-	 * @return bool|mysqli
+	 * @return mysqli|null
 	 * @throws DBConnectionError
 	 */
 	protected function mysqlConnect( $realServer, $dbName ) {
-		# Avoid suppressed fatal error, which is very hard to track down
 		if ( !function_exists( 'mysqli_init' ) ) {
-			throw new DBConnectionError( $this, "MySQLi functions missing,"
-				. " have you compiled PHP with the --with-mysqli option?\n" );
+			throw $this->newExceptionAfterConnectError(
+				"MySQLi functions missing, have you compiled PHP with the --with-mysqli option?"
+			);
 		}
 
 		// Other than mysql_connect, mysqli_real_connect expects an explicit port
@@ -84,7 +84,7 @@ class DatabaseMysqli extends DatabaseMysqlBase {
 		$mysqli = mysqli_init();
 
 		$connFlags = 0;
-		if ( $this->flags & self::DBO_SSL ) {
+		if ( $this->getFlag( self::DBO_SSL ) ) {
 			$connFlags |= MYSQLI_CLIENT_SSL;
 			$mysqli->ssl_set(
 				$this->sslKeyPath,
@@ -94,10 +94,10 @@ class DatabaseMysqli extends DatabaseMysqlBase {
 				$this->sslCiphers
 			);
 		}
-		if ( $this->flags & self::DBO_COMPRESS ) {
+		if ( $this->getFlag( self::DBO_COMPRESS ) ) {
 			$connFlags |= MYSQLI_CLIENT_COMPRESS;
 		}
-		if ( $this->flags & self::DBO_PERSISTENT ) {
+		if ( $this->getFlag( self::DBO_PERSISTENT ) ) {
 			$realServer = 'p:' . $realServer;
 		}
 
@@ -122,7 +122,7 @@ class DatabaseMysqli extends DatabaseMysqlBase {
 			return $mysqli;
 		}
 
-		return false;
+		return null;
 	}
 
 	/**
diff --git a/includes/libs/rdbms/database/DatabasePostgres.php b/includes/libs/rdbms/database/DatabasePostgres.php
index 840b4280b6..1c087578e9 100644
--- a/includes/libs/rdbms/database/DatabasePostgres.php
+++ b/includes/libs/rdbms/database/DatabasePostgres.php
@@ -31,22 +31,19 @@ use Exception;
  * @ingroup Database
  */
 class DatabasePostgres extends Database {
-	/** @var int|bool */
-	protected $port;
-
-	/** @var resource */
-	protected $lastResultHandle = null;
-
-	/** @var float|string */
-	private $numericVersion = null;
-	/** @var string Connect string to open a PostgreSQL connection */
-	private $connectString;
+	/** @var int|null */
+	private $port;
 	/** @var string */
 	private $coreSchema;
 	/** @var string */
 	private $tempSchema;
 	/** @var string[] Map of (reserved table name => alternate table name) */
 	private $keywordTableMap = [];
+	/** @var float|string */
+	private $numericVersion;
+
+	/** @var resource|null */
+	private $lastResultHandle;
 
 	/**
 	 * @see Database::__construct()
@@ -54,7 +51,7 @@ class DatabasePostgres extends Database {
 	 *   - keywordTableMap : Map of reserved table names to alternative table names to use
 	 */
 	public function __construct( array $params ) {
-		$this->port = $params['port'] ?? false;
+		$this->port = intval( $params['port'] ?? null );
 		$this->keywordTableMap = $params['keywordTableMap'] ?? [];
 
 		parent::__construct( $params );
@@ -87,13 +84,11 @@ class DatabasePostgres extends Database {
 	}
 
 	protected function open( $server, $user, $password, $dbName, $schema, $tablePrefix ) {
-		// Test for Postgres support, to avoid suppressed fatal error
 		if ( !function_exists( 'pg_connect' ) ) {
-			throw new DBConnectionError(
-				$this,
+			throw $this->newExceptionAfterConnectError(
 				"Postgres functions missing, have you compiled PHP with the --with-pgsql\n" .
 				"option? (Note: if you recently installed PHP, you may need to restart your\n" .
-				"webserver and database)\n"
+				"webserver and database)"
 			);
 		}
 
@@ -104,60 +99,47 @@ class DatabasePostgres extends Database {
 		$this->password = $password;
 
 		$connectVars = [
-			// pg_connect() user $user as the default database. Since a database is *required*,
-			// at least pick a "don't care" database that is more likely to exist. This case
-			// arrises when LoadBalancer::getConnection( $i, [], '' ) is used.
+			// pg_connect() user $user as the default database. Since a database is required,
+			// then pick a "don't care" database that is more likely to exist than that one.
 			'dbname' => strlen( $dbName ) ? $dbName : 'postgres',
 			'user' => $user,
 			'password' => $password
 		];
-		if ( $server != false && $server != '' ) {
+		if ( strlen( $server ) ) {
 			$connectVars['host'] = $server;
 		}
-		if ( (int)$this->port > 0 ) {
-			$connectVars['port'] = (int)$this->port;
+		if ( $this->port > 0 ) {
+			$connectVars['port'] = $this->port;
 		}
-		if ( $this->flags & self::DBO_SSL ) {
+		if ( $this->getFlag( self::DBO_SSL ) ) {
 			$connectVars['sslmode'] = 'require';
 		}
-
-		$this->connectString = $this->makeConnectionString( $connectVars );
+		$connectString = $this->makeConnectionString( $connectVars );
 
 		$this->installErrorHandler();
 		try {
-			// Use new connections to let LoadBalancer/LBFactory handle reuse
-			$this->conn = pg_connect( $this->connectString, PGSQL_CONNECT_FORCE_NEW );
-		} catch ( Exception $ex ) {
+			$this->conn = pg_connect( $connectString, PGSQL_CONNECT_FORCE_NEW ) ?: null;
+		} catch ( Exception $e ) {
 			$this->restoreErrorHandler();
-			throw $ex;
+			throw $this->newExceptionAfterConnectError( $e->getMessage() );
 		}
-		$phpError = $this->restoreErrorHandler();
+		$error = $this->restoreErrorHandler();
 
 		if ( !$this->conn ) {
-			$this->queryLogger->debug(
-				"DB connection error\n" .
-				"Server: $server, Database: $dbName, User: $user, Password: " .
-				substr( $password, 0, 3 ) . "...\n"
-			);
-			$this->queryLogger->debug( $this->lastError() . "\n" );
-			throw new DBConnectionError( $this, str_replace( "\n", ' ', $phpError ) );
+			throw $this->newExceptionAfterConnectError( $error ?: $this->lastError() );
 		}
 
 		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.
+			// Since no transaction is active at this point, any SET commands should apply
+			// for the entire session (e.g. will not be reverted on transaction rollback).
 			// See https://www.postgresql.org/docs/8.3/sql-set.html
-			$variables = [];
-			if ( $this->cliMode ) {
-				$variables['client_min_messages'] = 'ERROR';
-			}
-			$variables += [
+			$variables = [
 				'client_encoding' => 'UTF8',
 				'datestyle' => 'ISO, YMD',
 				'timezone' => 'GMT',
 				'standard_conforming_strings' => 'on',
-				'bytea_output' => 'escape'
+				'bytea_output' => 'escape',
+				'client_min_messages' => 'ERROR'
 			];
 			foreach ( $variables as $var => $val ) {
 				$this->query(
@@ -166,12 +148,10 @@ class DatabasePostgres extends Database {
 					self::QUERY_IGNORE_DBO_TRX | self::QUERY_NO_RETRY
 				);
 			}
-
 			$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;
+			throw $this->newExceptionAfterConnectError( $e->getMessage() );
 		}
 	}
 
@@ -1026,7 +1006,7 @@ __INDEXATTR__;
 	 * Values may contain magic keywords like "$user"
 	 * @since 1.19
 	 *
-	 * @param array $search_path List of schemas to be searched by default
+	 * @param string[] $search_path List of schemas to be searched by default
 	 */
 	private function setSearchPath( $search_path ) {
 		$this->query(
@@ -1066,11 +1046,7 @@ __INDEXATTR__;
 				$this->queryLogger->debug(
 					"Schema \"" . $desiredSchema . "\" already in the search path\n" );
 			} else {
-				/**
-				 * Prepend our schema (e.g. 'mediawiki') in front
-				 * of the search path
-				 * Fixes T17816
-				 */
+				// Prepend the desired schema to the search path (T17816)
 				$search_path = $this->getSearchPath();
 				array_unshift( $search_path, $this->addIdentifierQuotes( $desiredSchema ) );
 				$this->setSearchPath( $search_path );
diff --git a/includes/libs/rdbms/database/DatabaseSqlite.php b/includes/libs/rdbms/database/DatabaseSqlite.php
index 11dda2fb39..7c040c60a7 100644
--- a/includes/libs/rdbms/database/DatabaseSqlite.php
+++ b/includes/libs/rdbms/database/DatabaseSqlite.php
@@ -60,6 +60,9 @@ class DatabaseSqlite extends Database {
 	/** @var bool Whether full text is enabled */
 	private static $fulltextEnabled = null;
 
+	/** @var string[] See https://www.sqlite.org/lang_transaction.html */
+	private static $VALID_TRX_MODES = [ '', 'DEFERRED', 'IMMEDIATE', 'EXCLUSIVE' ];
+
 	/**
 	 * Additional params include:
 	 *   - dbDirectory : directory containing the DB and the lock file directory
@@ -77,8 +80,7 @@ class DatabaseSqlite extends Database {
 			$this->dbDir = $p['dbDirectory'];
 		}
 
-		// Set a dummy user to make initConnection() trigger open()
-		parent::__construct( [ 'user' => '@' ] + $p );
+		parent::__construct( $p );
 
 		$this->trxMode = strtoupper( $p['trxMode'] ?? '' );
 
@@ -138,7 +140,7 @@ class DatabaseSqlite extends Database {
 		// Note that for SQLite, $server, $user, and $pass are ignored
 
 		if ( $schema !== null ) {
-			throw new DBExpectedError( $this, __CLASS__ . ": cannot use schemas ('$schema')" );
+			throw $this->newExceptionAfterConnectError( "Got schema '$schema'; not supported." );
 		}
 
 		if ( $this->dbPath !== null ) {
@@ -146,59 +148,45 @@ class DatabaseSqlite extends Database {
 		} elseif ( $this->dbDir !== null ) {
 			$path = self::generateFileName( $this->dbDir, $dbName );
 		} else {
-			throw new DBExpectedError( $this, __CLASS__ . ": DB path or directory required" );
+			throw $this->newExceptionAfterConnectError( "DB path or directory required" );
 		}
 
-		if ( !in_array( $this->trxMode, [ '', 'DEFERRED', 'IMMEDIATE', 'EXCLUSIVE' ], true ) ) {
-			throw new DBExpectedError(
-				$this,
-				__CLASS__ . ": invalid transaction mode '{$this->trxMode}'"
-			);
+		if ( !self::isProcessMemoryPath( $path ) && !is_readable( $path ) ) {
+			throw $this->newExceptionAfterConnectError( 'SQLite database file is not readable' );
+		} elseif ( !in_array( $this->trxMode, self::$VALID_TRX_MODES, true ) ) {
+			throw $this->newExceptionAfterConnectError( "Got mode '{$this->trxMode}' for BEGIN" );
 		}
 
-		if ( !self::isProcessMemoryPath( $path ) && !is_readable( $path ) ) {
-			$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 );
+		$attributes = [];
+		if ( $this->getFlag( self::DBO_PERSISTENT ) ) {
+			// Persistent connections can avoid some schema index reading overhead.
+			// On the other hand, they can cause horrible contention with DBO_TRX.
+			if ( $this->getFlag( self::DBO_TRX ) ) {
+				$this->connLogger->warning( __METHOD__ . ": DBO_PERSISTENT mixed with DBO_TRX" );
+			} else {
+				$attributes[PDO::ATTR_PERSISTENT] = true;
+			}
 		}
 
 		try {
-			$conn = new PDO(
-				"sqlite:$path",
-				'',
-				'',
-				[ PDO::ATTR_PERSISTENT => (bool)( $this->flags & self::DBO_PERSISTENT ) ]
-			);
-			// Set error codes only, don't raise exceptions
-			$conn->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_SILENT );
+			$this->conn = new PDO( "sqlite:$path", null, null, $attributes );
 		} catch ( PDOException $e ) {
-			$error = $e->getMessage();
-			$this->connLogger->error(
-				"Error connecting to {db_server}: {error}",
-				$this->getLogContext( [ 'method' => __METHOD__, 'error' => $error ] )
-			);
-			throw new DBConnectionError( $this, $error );
+			throw $this->newExceptionAfterConnectError( $e->getMessage() );
 		}
 
-		$this->conn = $conn;
 		$this->currentDomain = new DatabaseDomain( $dbName, null, $tablePrefix );
 
 		try {
 			$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
+			// Apply 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", __METHOD__, $flags );
 			}
 		} catch ( Exception $e ) {
-			// Connection was not fully initialized and is not safe for use
-			$this->conn = false;
-			throw $e;
+			throw $this->newExceptionAfterConnectError( $e->getMessage() );
 		}
 	}
 
diff --git a/includes/libs/rdbms/database/IDatabase.php b/includes/libs/rdbms/database/IDatabase.php
index ef7f1e24f6..026a70506c 100644
--- a/includes/libs/rdbms/database/IDatabase.php
+++ b/includes/libs/rdbms/database/IDatabase.php
@@ -87,7 +87,7 @@ interface IDatabase {
 	/** @var int Combine list with OR clauses */
 	const LIST_OR = 4;
 
-	/** @var int Enable debug logging */
+	/** @var int Enable debug logging of all SQL queries */
 	const DBO_DEBUG = 1;
 	/** @var int Disable query buffering (only one result set can be iterated at a time) */
 	const DBO_NOBUFFER = 2;
@@ -336,13 +336,7 @@ interface IDatabase {
 	/**
 	 * Set a flag for this connection
 	 *
-	 * @param int $flag DBO_* constants from Defines.php:
-	 *   - DBO_DEBUG: output some debug info (same as debug())
-	 *   - DBO_NOBUFFER: don't buffer results (inverse of bufferResults())
-	 *   - DBO_TRX: automatically start transactions
-	 *   - DBO_DEFAULT: automatically sets DBO_TRX if not in command line mode
-	 *       and removes it in command line mode
-	 *   - DBO_PERSISTENT: use persistant database connection
+	 * @param int $flag IDatabase::DBO_DEBUG, IDatabase::DBO_NOBUFFER, or IDatabase::DBO_TRX
 	 * @param string $remember IDatabase::REMEMBER_* constant [default: REMEMBER_NOTHING]
 	 */
 	public function setFlag( $flag, $remember = self::REMEMBER_NOTHING );
@@ -350,13 +344,7 @@ interface IDatabase {
 	/**
 	 * Clear a flag for this connection
 	 *
-	 * @param int $flag DBO_* constants from Defines.php:
-	 *   - DBO_DEBUG: output some debug info (same as debug())
-	 *   - DBO_NOBUFFER: don't buffer results (inverse of bufferResults())
-	 *   - DBO_TRX: automatically start transactions
-	 *   - DBO_DEFAULT: automatically sets DBO_TRX if not in command line mode
-	 *       and removes it in command line mode
-	 *   - DBO_PERSISTENT: use persistant database connection
+	 * @param int $flag IDatabase::DBO_DEBUG, IDatabase::DBO_NOBUFFER, or IDatabase::DBO_TRX
 	 * @param string $remember IDatabase::REMEMBER_* constant [default: REMEMBER_NOTHING]
 	 */
 	public function clearFlag( $flag, $remember = self::REMEMBER_NOTHING );
@@ -372,11 +360,7 @@ interface IDatabase {
 	/**
 	 * Returns a boolean whether the flag $flag is set for this connection
 	 *
-	 * @param int $flag DBO_* constants from Defines.php:
-	 *   - DBO_DEBUG: output some debug info (same as debug())
-	 *   - DBO_NOBUFFER: don't buffer results (inverse of bufferResults())
-	 *   - DBO_TRX: automatically start transactions
-	 *   - DBO_PERSISTENT: use persistant database connection
+	 * @param int $flag One of the class IDatabase::DBO_* constants
 	 * @return bool
 	 */
 	public function getFlag( $flag );
diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php
index 482ab4b5f5..a775dd70e3 100644
--- a/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php
+++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php
@@ -568,62 +568,74 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
 	public function testFlagSetting() {
 		$db = $this->db;
 		$origTrx = $db->getFlag( DBO_TRX );
-		$origSsl = $db->getFlag( DBO_SSL );
+		$origNoBuffer = $db->getFlag( DBO_NOBUFFER );
 
 		$origTrx
 			? $db->clearFlag( DBO_TRX, $db::REMEMBER_PRIOR )
 			: $db->setFlag( DBO_TRX, $db::REMEMBER_PRIOR );
 		$this->assertEquals( !$origTrx, $db->getFlag( DBO_TRX ) );
 
-		$origSsl
-			? $db->clearFlag( DBO_SSL, $db::REMEMBER_PRIOR )
-			: $db->setFlag( DBO_SSL, $db::REMEMBER_PRIOR );
-		$this->assertEquals( !$origSsl, $db->getFlag( DBO_SSL ) );
+		$origNoBuffer
+			? $db->clearFlag( DBO_NOBUFFER, $db::REMEMBER_PRIOR )
+			: $db->setFlag( DBO_NOBUFFER, $db::REMEMBER_PRIOR );
+		$this->assertEquals( !$origNoBuffer, $db->getFlag( DBO_NOBUFFER ) );
 
 		$db->restoreFlags( $db::RESTORE_INITIAL );
 		$this->assertEquals( $origTrx, $db->getFlag( DBO_TRX ) );
-		$this->assertEquals( $origSsl, $db->getFlag( DBO_SSL ) );
+		$this->assertEquals( $origNoBuffer, $db->getFlag( DBO_NOBUFFER ) );
 
 		$origTrx
 			? $db->clearFlag( DBO_TRX, $db::REMEMBER_PRIOR )
 			: $db->setFlag( DBO_TRX, $db::REMEMBER_PRIOR );
-		$origSsl
-			? $db->clearFlag( DBO_SSL, $db::REMEMBER_PRIOR )
-			: $db->setFlag( DBO_SSL, $db::REMEMBER_PRIOR );
+		$origNoBuffer
+			? $db->clearFlag( DBO_NOBUFFER, $db::REMEMBER_PRIOR )
+			: $db->setFlag( DBO_NOBUFFER, $db::REMEMBER_PRIOR );
 
 		$db->restoreFlags();
-		$this->assertEquals( $origSsl, $db->getFlag( DBO_SSL ) );
+		$this->assertEquals( $origNoBuffer, $db->getFlag( DBO_NOBUFFER ) );
 		$this->assertEquals( !$origTrx, $db->getFlag( DBO_TRX ) );
 
 		$db->restoreFlags();
-		$this->assertEquals( $origSsl, $db->getFlag( DBO_SSL ) );
+		$this->assertEquals( $origNoBuffer, $db->getFlag( DBO_NOBUFFER ) );
 		$this->assertEquals( $origTrx, $db->getFlag( DBO_TRX ) );
 	}
 
+	public function provideImmutableDBOFlags() {
+		return [
+			[ Database::DBO_IGNORE ],
+			[ Database::DBO_DEFAULT ],
+			[ Database::DBO_PERSISTENT ]
+		];
+	}
+
 	/**
-	 * @expectedException UnexpectedValueException
+	 * @expectedException DBUnexpectedError
 	 * @covers Wikimedia\Rdbms\Database::setFlag
+	 * @dataProvider provideImmutableDBOFlags
+	 * @param int $flag
 	 */
-	public function testDBOIgnoreSet() {
+	public function testDBOCannotSet( $flag ) {
 		$db = $this->getMockBuilder( DatabaseMysqli::class )
 			->disableOriginalConstructor()
 			->setMethods( null )
 			->getMock();
 
-		$db->setFlag( Database::DBO_IGNORE );
+		$db->setFlag( $flag );
 	}
 
 	/**
-	 * @expectedException UnexpectedValueException
+	 * @expectedException DBUnexpectedError
 	 * @covers Wikimedia\Rdbms\Database::clearFlag
+	 * @dataProvider provideImmutableDBOFlags
+	 * @param int $flag
 	 */
-	public function testDBOIgnoreClear() {
+	public function testDBOCannotClear( $flag ) {
 		$db = $this->getMockBuilder( DatabaseMysqli::class )
 			->disableOriginalConstructor()
 			->setMethods( null )
 			->getMock();
 
-		$db->clearFlag( Database::DBO_IGNORE );
+		$db->clearFlag( $flag );
 	}
 
 	/**