rdbms: inject the mysql index name aliases into Database
authorAaron Schulz <aschulz@wikimedia.org>
Sat, 17 Feb 2018 22:09:02 +0000 (14:09 -0800)
committerAaron Schulz <aschulz@wikimedia.org>
Mon, 12 Mar 2018 18:51:53 +0000 (18:51 +0000)
Also added LBFactory::setTableAlias() for consistency with this

Change-Id: Ie49003ff8fd5b99f75db9fae8fe0a184444254d4

includes/ServiceWiring.php
includes/db/MWLBFactory.php
includes/libs/rdbms/database/DBConnRef.php
includes/libs/rdbms/database/Database.php
includes/libs/rdbms/database/DatabaseMysqlBase.php
includes/libs/rdbms/database/IDatabase.php
includes/libs/rdbms/lbfactory/ILBFactory.php
includes/libs/rdbms/lbfactory/LBFactory.php
includes/libs/rdbms/loadbalancer/ILoadBalancer.php
includes/libs/rdbms/loadbalancer/LoadBalancer.php
tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php

index 08d343b..5131917 100644 (file)
@@ -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 ) {
index 5c79117..f0a17f7 100644 (file)
@@ -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',
+                       ] );
+               }
+       }
 }
index f26b985..26030bc 100644 (file)
@@ -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
         */
index 014c4af..1161fba 100644 (file)
@@ -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
index 8fb8db5..d1d0179 100644 (file)
@@ -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 );
index 28a8125..0254c9b 100644 (file)
@@ -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' );
index 98108a7..32d9008 100644 (file)
@@ -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 );
 }
index 2324553..32886e2 100644 (file)
@@ -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 ) {
index 8210507..767cc49 100644 (file)
@@ -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 );
 }
index 99a24c2..35198ac 100644 (file)
@@ -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 = [];
index bf3689b..1eca89b 100644 (file)
@@ -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
+               );
+       }
 }