From: Aaron Schulz Date: Sat, 17 Feb 2018 22:09:02 +0000 (-0800) Subject: rdbms: inject the mysql index name aliases into Database X-Git-Tag: 1.31.0-rc.0~390^2 X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=4ccb228bde9294d96ddcb5c255565c89dbcbf879 rdbms: inject the mysql index name aliases into Database Also added LBFactory::setTableAlias() for consistency with this Change-Id: Ie49003ff8fd5b99f75db9fae8fe0a184444254d4 --- diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index 08d343b1b2..5131917d94 100644 --- a/includes/ServiceWiring.php +++ b/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 ) { diff --git a/includes/db/MWLBFactory.php b/includes/db/MWLBFactory.php index 5c79117f78..f0a17f7bf7 100644 --- a/includes/db/MWLBFactory.php +++ b/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', + ] ); + } + } } diff --git a/includes/libs/rdbms/database/DBConnRef.php b/includes/libs/rdbms/database/DBConnRef.php index f26b98546b..26030bc369 100644 --- a/includes/libs/rdbms/database/DBConnRef.php +++ b/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 */ diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php index 014c4af399..1161fba945 100644 --- a/includes/libs/rdbms/database/Database.php +++ b/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 diff --git a/includes/libs/rdbms/database/DatabaseMysqlBase.php b/includes/libs/rdbms/database/DatabaseMysqlBase.php index 8fb8db5c16..d1d0179d71 100644 --- a/includes/libs/rdbms/database/DatabaseMysqlBase.php +++ b/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 ); diff --git a/includes/libs/rdbms/database/IDatabase.php b/includes/libs/rdbms/database/IDatabase.php index 28a812573e..0254c9b915 100644 --- a/includes/libs/rdbms/database/IDatabase.php +++ b/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' ); diff --git a/includes/libs/rdbms/lbfactory/ILBFactory.php b/includes/libs/rdbms/lbfactory/ILBFactory.php index 98108a7df2..32d90082d3 100644 --- a/includes/libs/rdbms/lbfactory/ILBFactory.php +++ b/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 ); } diff --git a/includes/libs/rdbms/lbfactory/LBFactory.php b/includes/libs/rdbms/lbfactory/LBFactory.php index 2324553016..32886e2f06 100644 --- a/includes/libs/rdbms/lbfactory/LBFactory.php +++ b/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 ) { diff --git a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php index 8210507a14..767cc49f59 100644 --- a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php +++ b/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 ); } diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php b/includes/libs/rdbms/loadbalancer/LoadBalancer.php index 99a24c2b74..35198ac37c 100644 --- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php +++ b/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 = []; diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php index bf3689bf88..1eca89bb22 100644 --- a/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php +++ b/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 + ); + } }