From: Aaron Schulz <aschulz@wikimedia.org>
Date: Sat, 6 Jul 2019 19:36:54 +0000 (-0700)
Subject: rdbms: cleanup some Database error message wording for consistency
X-Git-Tag: 1.34.0-rc.0~1055^2
X-Git-Url: http://git.cyclocoop.org/%24image?a=commitdiff_plain;h=c351a9cab09277b0a0517624ddd2aa1bdb264c4e;p=lhc%2Fweb%2Fwiklou.git

rdbms: cleanup some Database error message wording for consistency

Change-Id: I7b338e6e856c62ecaab2ef97f76431c2220b430d
---

diff --git a/includes/db/DatabaseOracle.php b/includes/db/DatabaseOracle.php
index a123d00f27..501f01a3e9 100644
--- a/includes/db/DatabaseOracle.php
+++ b/includes/db/DatabaseOracle.php
@@ -62,7 +62,7 @@ class DatabaseOracle extends Database {
 	 * @param array $params Additional parameters include:
 	 *   - keywordTableMap : Map of reserved table names to alternative table names to use
 	 */
-	function __construct( array $params ) {
+	public function __construct( array $params ) {
 		$this->keywordTableMap = $params['keywordTableMap'] ?? [];
 		$params['tablePrefix'] = strtoupper( $params['tablePrefix'] );
 		parent::__construct( $params );
@@ -97,6 +97,15 @@ class DatabaseOracle extends Database {
 					"and database)\n" );
 		}
 
+		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->close();
 		$this->user = $user;
 		$this->password = $password;
@@ -1028,7 +1037,11 @@ class DatabaseOracle extends Database {
 	protected function doSelectDomain( DatabaseDomain $domain ) {
 		if ( $domain->getSchema() !== null ) {
 			// We use the *database* aspect of $domain for schema, not the domain schema
-			throw new DBExpectedError( $this, __CLASS__ . ": domain schemas are not supported." );
+			throw new DBExpectedError(
+				$this,
+				__CLASS__ . ": domain '{$domain->getId()}' has a schema component; " .
+				"the database component is actually interpreted as the Oracle schema."
+			);
 		}
 
 		$database = $domain->getDatabase();
diff --git a/includes/libs/rdbms/database/DatabaseMssql.php b/includes/libs/rdbms/database/DatabaseMssql.php
index 69174f96fa..d06bcb9274 100644
--- a/includes/libs/rdbms/database/DatabaseMssql.php
+++ b/includes/libs/rdbms/database/DatabaseMssql.php
@@ -1165,7 +1165,10 @@ class DatabaseMssql extends Database {
 
 	protected function doSelectDomain( DatabaseDomain $domain ) {
 		if ( $domain->getSchema() !== null ) {
-			throw new DBExpectedError( $this, __CLASS__ . ": domain schemas are not supported." );
+			throw new DBExpectedError(
+				$this,
+				__CLASS__ . ": domain '{$domain->getId()}' has a schema component"
+			);
 		}
 
 		$database = $domain->getDatabase();
diff --git a/includes/libs/rdbms/database/DatabaseMysqlBase.php b/includes/libs/rdbms/database/DatabaseMysqlBase.php
index 417b464b42..4774390986 100644
--- a/includes/libs/rdbms/database/DatabaseMysqlBase.php
+++ b/includes/libs/rdbms/database/DatabaseMysqlBase.php
@@ -96,7 +96,7 @@ abstract class DatabaseMysqlBase extends Database {
 	 *   - sslCiphers : array list of allowable ciphers [default: null]
 	 * @param array $params
 	 */
-	function __construct( array $params ) {
+	public function __construct( array $params ) {
 		$this->lagDetectionMethod = $params['lagDetectionMethod'] ?? 'Seconds_Behind_Master';
 		$this->lagDetectionOptions = $params['lagDetectionOptions'] ?? [];
 		$this->useGTIDs = !empty( $params['useGTIDs' ] );
@@ -125,7 +125,7 @@ abstract class DatabaseMysqlBase extends Database {
 		$this->close();
 
 		if ( $schema !== null ) {
-			throw new DBExpectedError( $this, __CLASS__ . ": domain schemas are not supported." );
+			throw new DBExpectedError( $this, __CLASS__ . ": cannot use schemas ('$schema')" );
 		}
 
 		$this->server = $server;
@@ -194,7 +194,10 @@ abstract class DatabaseMysqlBase extends Database {
 
 	protected function doSelectDomain( DatabaseDomain $domain ) {
 		if ( $domain->getSchema() !== null ) {
-			throw new DBExpectedError( $this, __CLASS__ . ": domain schemas are not supported." );
+			throw new DBExpectedError(
+				$this,
+				__CLASS__ . ": domain '{$domain->getId()}' has a schema component"
+			);
 		}
 
 		$database = $domain->getDatabase();
diff --git a/includes/libs/rdbms/database/DatabasePostgres.php b/includes/libs/rdbms/database/DatabasePostgres.php
index 08987d98dd..954d400cf0 100644
--- a/includes/libs/rdbms/database/DatabasePostgres.php
+++ b/includes/libs/rdbms/database/DatabasePostgres.php
@@ -97,6 +97,8 @@ class DatabasePostgres extends Database {
 			);
 		}
 
+		$this->close();
+
 		$this->server = $server;
 		$this->user = $user;
 		$this->password = $password;
@@ -120,9 +122,8 @@ class DatabasePostgres extends Database {
 		}
 
 		$this->connectString = $this->makeConnectionString( $connectVars );
-		$this->close();
-		$this->installErrorHandler();
 
+		$this->installErrorHandler();
 		try {
 			// Use new connections to let LoadBalancer/LBFactory handle reuse
 			$this->conn = pg_connect( $this->connectString, PGSQL_CONNECT_FORCE_NEW );
@@ -130,7 +131,6 @@ class DatabasePostgres extends Database {
 			$this->restoreErrorHandler();
 			throw $ex;
 		}
-
 		$phpError = $this->restoreErrorHandler();
 
 		if ( !$this->conn ) {
@@ -1053,7 +1053,7 @@ __INDEXATTR__;
 			// See https://www.postgresql.org/docs/8.3/sql-set.html
 			throw new DBUnexpectedError(
 				$this,
-				__METHOD__ . ": a transaction is currently active."
+				__METHOD__ . ": a transaction is currently active"
 			);
 		}