rdbms: inject the mysql index name aliases into Database

Also added LBFactory::setTableAlias() for consistency with this

Change-Id: Ie49003ff8fd5b99f75db9fae8fe0a184444254d4

includes/ServiceWiring.php

@@ -61,7 +61,10 @@ return [
 		);
 		$class = MWLBFactory::getLBFactoryClass( $lbConf );
 
-		return new $class( $lbConf );
+		$instance = new $class( $lbConf );
+		MWLBFactory::setSchemaAliases( $instance );
+
+		return $instance;
 	},
 
 	'DBLoadBalancer' => function ( MediaWikiServices $services ) {

includes/db/MWLBFactory.php

@@ -23,6 +23,7 @@
 
 use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\MediaWikiServices;
+use Wikimedia\Rdbms\LBFactory;
 use Wikimedia\Rdbms\DatabaseDomain;
 
 /**
@@ -201,4 +202,30 @@ abstract class MWLBFactory {
 
 		return $class;
 	}
+
+	public static function setSchemaAliases( LBFactory $lbFactory ) {
+		$mainLB = $lbFactory->getMainLB();
+		$masterType = $mainLB->getServerType( $mainLB->getWriterIndex() );
+		if ( $masterType === 'mysql' ) {
+			/**
+			 * When SQLite indexes were introduced in r45764, it was noted that
+			 * SQLite requires index names to be unique within the whole database,
+			 * not just within a schema. As discussed in CR r45819, to avoid the
+			 * need for a schema change on existing installations, the indexes
+			 * were implicitly mapped from the new names to the old names.
+			 *
+			 * This mapping can be removed if DB patches are introduced to alter
+			 * the relevant tables in existing installations. Note that because
+			 * this index mapping applies to table creation, even new installations
+			 * of MySQL have the old names (except for installations created during
+			 * a period where this mapping was inappropriately removed, see
+			 * T154872).
+			 */
+			$lbFactory->setIndexAliases( [
+				'ar_usertext_timestamp' => 'usertext_timestamp',
+				'un_user_id' => 'user_id',
+				'un_user_ip' => 'user_ip',
+			] );
+		}
+	}
 }

includes/libs/rdbms/database/DBConnRef.php

@@ -618,6 +618,10 @@ class DBConnRef implements IDatabase {
 		return $this->__call( __FUNCTION__, func_get_args() );
 	}
 
+	public function setIndexAliases( array $aliases ) {
+		return $this->__call( __FUNCTION__, func_get_args() );
+	}
+
 	/**
 	 * Clean up the connection when out of scope
 	 */

includes/libs/rdbms/database/Database.php

@@ -76,8 +76,10 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 	protected $password;
 	/** @var string */
 	protected $dbName;
-	/** @var array[] $aliases Map of (table => (dbname, schema, prefix) map) */
+	/** @var array[] Map of (table => (dbname, schema, prefix) map) */
 	protected $tableAliases = [];
+	/** @var string[] Map of (index alias => index) */
+	protected $indexAliases = [];
 	/** @var bool Whether this PHP instance is for a CLI script */
 	protected $cliMode;
 	/** @var string Agent name for query profiling */
@@ -2220,7 +2222,9 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 	 * @return string
 	 */
 	protected function indexName( $index ) {
-		return $index;
+		return isset( $this->indexAliases[$index] )
+			? $this->indexAliases[$index]
+			: $index;
 	}
 
 	public function addQuotes( $s ) {
@@ -3875,6 +3879,10 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 		$this->tableAliases = $aliases;
 	}
 
+	public function setIndexAliases( array $aliases ) {
+		$this->indexAliases = $aliases;
+	}
+
 	/**
 	 * @return bool Whether a DB user is required to access the DB
 	 * @since 1.28

includes/libs/rdbms/database/DatabaseMysqlBase.php

@@ -1418,40 +1418,6 @@ abstract class DatabaseMysqlBase extends Database {
 		return in_array( $name, $this->listViews( $prefix ) );
 	}
 
-	/**
-	 * Allows for index remapping in queries where this is not consistent across DBMS
-	 *
-	 * @param string $index
-	 * @return string
-	 */
-	protected function indexName( $index ) {
-		/**
-		 * When SQLite indexes were introduced in r45764, it was noted that
-		 * SQLite requires index names to be unique within the whole database,
-		 * not just within a schema. As discussed in CR r45819, to avoid the
-		 * need for a schema change on existing installations, the indexes
-		 * were implicitly mapped from the new names to the old names.
-		 *
-		 * This mapping can be removed if DB patches are introduced to alter
-		 * the relevant tables in existing installations. Note that because
-		 * this index mapping applies to table creation, even new installations
-		 * of MySQL have the old names (except for installations created during
-		 * a period where this mapping was inappropriately removed, see
-		 * T154872).
-		 */
-		$renamed = [
-			'ar_usertext_timestamp' => 'usertext_timestamp',
-			'un_user_id' => 'user_id',
-			'un_user_ip' => 'user_ip',
-		];
-
-		if ( isset( $renamed[$index] ) ) {
-			return $renamed[$index];
-		} else {
-			return $index;
-		}
-	}
-
 	protected function isTransactableQuery( $sql ) {
 		return parent::isTransactableQuery( $sql ) &&
 			!preg_match( '/^SELECT\s+(GET|RELEASE|IS_FREE)_LOCK\(/', $sql );

includes/libs/rdbms/database/IDatabase.php

@@ -1927,6 +1927,21 @@ interface IDatabase {
 	 * @since 1.28
 	 */
 	public function setTableAliases( array $aliases );
+
+	/**
+	 * Convert certain index names to alternative names before querying the DB
+	 *
+	 * Note that this applies to indexes regardless of the table they belong to.
+	 *
+	 * This can be employed when an index was renamed X => Y in code, but the new Y-named
+	 * indexes were not yet built on all DBs. After all the Y-named ones are added by the DBA,
+	 * the aliases can be removed, and then the old X-named indexes dropped.
+	 *
+	 * @param string[] $aliases
+	 * @return mixed
+	 * @since 1.31
+	 */
+	public function setIndexAliases( array $aliases );
 }
 
 class_alias( IDatabase::class, 'IDatabase' );

includes/libs/rdbms/lbfactory/ILBFactory.php

@@ -42,19 +42,19 @@ interface ILBFactory {
 	 *
 	 * @param array $conf Array with keys:
 	 *  - localDomain: A DatabaseDomain or domain ID string.
-	 *  - readOnlyReason : Reason the master DB is read-only if so [optional]
-	 *  - srvCache : BagOStuff object for server cache [optional]
-	 *  - memStash : BagOStuff object for cross-datacenter memory storage [optional]
-	 *  - wanCache : WANObjectCache object [optional]
-	 *  - hostname : The name of the current server [optional]
+	 *  - readOnlyReason: Reason the master DB is read-only if so [optional]
+	 *  - srvCache: BagOStuff object for server cache [optional]
+	 *  - memStash: BagOStuff object for cross-datacenter memory storage [optional]
+	 *  - wanCache: WANObjectCache object [optional]
+	 *  - hostname: The name of the current server [optional]
 	 *  - cliMode: Whether the execution context is a CLI script. [optional]
-	 *  - profiler : Class name or instance with profileIn()/profileOut() methods. [optional]
+	 *  - profiler: Class name or instance with profileIn()/profileOut() methods. [optional]
 	 *  - trxProfiler: TransactionProfiler instance. [optional]
 	 *  - replLogger: PSR-3 logger instance. [optional]
 	 *  - connLogger: PSR-3 logger instance. [optional]
 	 *  - queryLogger: PSR-3 logger instance. [optional]
 	 *  - perfLogger: PSR-3 logger instance. [optional]
-	 *  - errorLogger : Callback that takes an Exception and logs it. [optional]
+	 *  - errorLogger: Callback that takes an Exception and logs it. [optional]
 	 * @throws InvalidArgumentException
 	 */
 	public function __construct( array $conf );
@@ -323,4 +323,34 @@ interface ILBFactory {
 	 *   - ChronologyPositionIndex: timestamp used to get up-to-date DB positions for the agent
 	 */
 	public function setRequestInfo( array $info );
+
+	/**
+	 * Make certain table names use their own database, schema, and table prefix
+	 * when passed into SQL queries pre-escaped and without a qualified database name
+	 *
+	 * For example, "user" can be converted to "myschema.mydbname.user" for convenience.
+	 * Appearances like `user`, somedb.user, somedb.someschema.user will used literally.
+	 *
+	 * Calling this twice will completely clear any old table aliases. Also, note that
+	 * callers are responsible for making sure the schemas and databases actually exist.
+	 *
+	 * @param array[] $aliases Map of (table => (dbname, schema, prefix) map)
+	 * @since 1.31
+	 */
+	public function setTableAliases( array $aliases );
+
+	/**
+	 * Convert certain index names to alternative names before querying the DB
+	 *
+	 * Note that this applies to indexes regardless of the table they belong to.
+	 *
+	 * This can be employed when an index was renamed X => Y in code, but the new Y-named
+	 * indexes were not yet built on all DBs. After all the Y-named ones are added by the DBA,
+	 * the aliases can be removed, and then the old X-named indexes dropped.
+	 *
+	 * @param string[] $aliases
+	 * @return mixed
+	 * @since 1.31
+	 */
+	public function setIndexAliases( array $aliases );
 }

includes/libs/rdbms/lbfactory/LBFactory.php

@@ -75,6 +75,11 @@ abstract class LBFactory implements ILBFactory {
 	/** @var callable[] */
 	protected $replicationWaitCallbacks = [];
 
+	/** @var array[] $aliases Map of (table => (dbname, schema, prefix) map) */
+	protected $tableAliases = [];
+	/** @var string[] Map of (index alias => index) */
+	protected $indexAliases = [];
+
 	/** @var bool Whether this PHP instance is for a CLI script */
 	protected $cliMode;
 	/** @var string Agent name for query profiling */
@@ -523,6 +528,17 @@ abstract class LBFactory implements ILBFactory {
 		if ( $this->trxRoundId !== false ) {
 			$lb->beginMasterChanges( $this->trxRoundId ); // set DBO_TRX
 		}
+
+		$lb->setTableAliases( $this->tableAliases );
+		$lb->setIndexAliases( $this->indexAliases );
+	}
+
+	public function setTableAliases( array $aliases ) {
+		$this->tableAliases = $aliases;
+	}
+
+	public function setIndexAliases( array $aliases ) {
+		$this->indexAliases = $aliases;
 	}
 
 	public function setDomainPrefix( $prefix ) {

includes/libs/rdbms/loadbalancer/ILoadBalancer.php

@@ -615,4 +615,19 @@ interface ILoadBalancer {
 	 * @param array[] $aliases Map of (table => (dbname, schema, prefix) map)
 	 */
 	public function setTableAliases( array $aliases );
+
+	/**
+	 * Convert certain index names to alternative names before querying the DB
+	 *
+	 * Note that this applies to indexes regardless of the table they belong to.
+	 *
+	 * This can be employed when an index was renamed X => Y in code, but the new Y-named
+	 * indexes were not yet built on all DBs. After all the Y-named ones are added by the DBA,
+	 * the aliases can be removed, and then the old X-named indexes dropped.
+	 *
+	 * @param string[] $aliases
+	 * @return mixed
+	 * @since 1.31
+	 */
+	public function setIndexAliases( array $aliases );
 }

includes/libs/rdbms/loadbalancer/LoadBalancer.php

@@ -54,6 +54,8 @@ class LoadBalancer implements ILoadBalancer {
 	private $loadMonitorConfig;
 	/** @var array[] $aliases Map of (table => (dbname, schema, prefix) map) */
 	private $tableAliases = [];
+	/** @var string[] Map of (index alias => index) */
+	private $indexAliases = [];
 
 	/** @var ILoadMonitor */
 	private $loadMonitor;
@@ -1088,6 +1090,7 @@ class LoadBalancer implements ILoadBalancer {
 			$this->getLazyConnectionRef( self::DB_MASTER, [], $db->getDomainID() )
 		);
 		$db->setTableAliases( $this->tableAliases );
+		$db->setIndexAliases( $this->indexAliases );
 
 		if ( $server['serverIndex'] === $this->getWriterIndex() ) {
 			if ( $this->trxRoundId !== false ) {
@@ -1757,6 +1760,10 @@ class LoadBalancer implements ILoadBalancer {
 		$this->tableAliases = $aliases;
 	}
 
+	public function setIndexAliases( array $aliases ) {
+		$this->indexAliases = $aliases;
+	}
+
 	public function setDomainPrefix( $prefix ) {
 		// Find connections to explicit foreign domains still marked as in-use...
 		$domainsInUse = [];

tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php

@@ -517,4 +517,69 @@ class DatabaseMysqlBaseTest extends PHPUnit\Framework\TestCase {
 		$this->assertSame( 'CAST( fieldName AS SIGNED )', $output );
 	}
 
+	/*
+	 * @covers Wikimedia\Rdbms\Database::setIndexAliases
+	 */
+	public function testIndexAliases() {
+		$db = $this->getMockBuilder( DatabaseMysqli::class )
+			->disableOriginalConstructor()
+			->setMethods( [ 'mysqlRealEscapeString' ] )
+			->getMock();
+		$db->method( 'mysqlRealEscapeString' )->willReturnCallback(
+			function ( $s ) {
+				return str_replace( "'", "\\'", $s );
+			}
+		);
+
+		$db->setIndexAliases( [ 'a_b_idx' => 'a_c_idx' ] );
+		$sql = $db->selectSQLText(
+			'zend', 'field', [ 'a' => 'x' ], __METHOD__, [ 'USE INDEX' => 'a_b_idx' ] );
+
+		$this->assertEquals(
+			"SELECT  field  FROM `zend`  FORCE INDEX (a_c_idx)  WHERE a = 'x'  ",
+			$sql
+		);
+
+		$db->setIndexAliases( [] );
+		$sql = $db->selectSQLText(
+			'zend', 'field', [ 'a' => 'x' ], __METHOD__, [ 'USE INDEX' => 'a_b_idx' ] );
+
+		$this->assertEquals(
+			"SELECT  field  FROM `zend`  FORCE INDEX (a_b_idx)  WHERE a = 'x'  ",
+			$sql
+		);
+	}
+
+	/**
+	 * @covers Wikimedia\Rdbms\Database::setTableAliases
+	 */
+	public function testTableAliases() {
+		$db = $this->getMockBuilder( DatabaseMysqli::class )
+			->disableOriginalConstructor()
+			->setMethods( [ 'mysqlRealEscapeString' ] )
+			->getMock();
+		$db->method( 'mysqlRealEscapeString' )->willReturnCallback(
+			function ( $s ) {
+				return str_replace( "'", "\\'", $s );
+			}
+		);
+
+		$db->setTableAliases( [
+			'meow' => [ 'dbname' => 'feline', 'schema' => null, 'prefix' => 'cat_' ]
+		] );
+		$sql = $db->selectSQLText( 'meow', 'field', [ 'a' => 'x' ], __METHOD__ );
+
+		$this->assertEquals(
+			"SELECT  field  FROM `feline`.`cat_meow`    WHERE a = 'x'  ",
+			$sql
+		);
+
+		$db->setTableAliases( [] );
+		$sql = $db->selectSQLText( 'meow', 'field', [ 'a' => 'x' ], __METHOD__ );
+
+		$this->assertEquals(
+			"SELECT  field  FROM `meow`    WHERE a = 'x'  ",
+			$sql
+		);
+	}
 }