Postgres updater fixes to make update.php able to run
authorAaron Schulz <aschulz@wikimedia.org>
Tue, 25 Oct 2016 18:56:41 +0000 (11:56 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Sat, 29 Oct 2016 00:13:56 +0000 (17:13 -0700)
* Remove redundant schema prefix from relname=x query. The
  schema filtering is already done via the JOIN. The relname
  portion is just the table name not <schema>.<table name>.
* Avoid explicit table schema qualification and rely on the
  search path, as MW 1.27 did. Previously it only used the
  global $wgDBschema var to pass to determineCoreSchema()
  instead of keeping it in mSchema.
* Clean up some code duplication in Database::tableName() and
  make the code comments clearer.
* Make DatabasePostgres::tableName() use parent::tableName()
  instead of a method that just wraps this method. The intent
  seems clearer this way.
* Remove unused return value in
  PostgresUpdater::rebuildTextSearch().

Bug: T148628
Change-Id: Id11d9576b7c2fdad22ff7f90727c12997217a632

includes/installer/PostgresUpdater.php
includes/libs/rdbms/database/Database.php
includes/libs/rdbms/database/DatabasePostgres.php
includes/libs/rdbms/field/PostgresField.php

index f3d2860..790fbe7 100644 (file)
@@ -975,7 +975,7 @@ END;
        protected function rebuildTextSearch() {
                if ( $this->updateRowExists( 'patch-textsearch_bug66650.sql' ) ) {
                        $this->output( "...bug 66650 already fixed or not applicable.\n" );
-                       return true;
+                       return;
                };
                $this->applyPatch( 'patch-textsearch_bug66650.sql', false,
                        'Rebuilding text search for bug 66650' );
index ba63432..ee4524f 100644 (file)
@@ -1721,9 +1721,9 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                } elseif ( count( $dbDetails ) == 2 ) {
                        list( $database, $table ) = $dbDetails;
                        # We don't want any prefix added in this case
+                       $prefix = '';
                        # In dbs that support it, $database may actually be the schema
                        # but that doesn't affect any of the functionality here
-                       $prefix = '';
                        $schema = '';
                } else {
                        list( $table ) = $dbDetails;
@@ -1745,29 +1745,35 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                # Quote $table and apply the prefix if not quoted.
                # $tableName might be empty if this is called from Database::replaceVars()
                $tableName = "{$prefix}{$table}";
-               if ( $format == 'quoted'
-                       && !$this->isQuotedIdentifier( $tableName ) && $tableName !== ''
+               if ( $format === 'quoted'
+                       && !$this->isQuotedIdentifier( $tableName )
+                       && $tableName !== ''
                ) {
                        $tableName = $this->addIdentifierQuotes( $tableName );
                }
 
-               # Quote $schema and merge it with the table name if needed
-               if ( strlen( $schema ) ) {
-                       if ( $format == 'quoted' && !$this->isQuotedIdentifier( $schema ) ) {
-                               $schema = $this->addIdentifierQuotes( $schema );
-                       }
-                       $tableName = $schema . '.' . $tableName;
-               }
+               # Quote $schema and $database and merge them with the table name if needed
+               $tableName = $this->prependDatabaseOrSchema( $schema, $tableName, $format );
+               $tableName = $this->prependDatabaseOrSchema( $database, $tableName, $format );
+
+               return $tableName;
+       }
 
-               # Quote $database and merge it with the table name if needed
-               if ( $database !== '' ) {
-                       if ( $format == 'quoted' && !$this->isQuotedIdentifier( $database ) ) {
-                               $database = $this->addIdentifierQuotes( $database );
+       /**
+        * @param string|null $namespace Database or schema
+        * @param string $relation Name of table, view, sequence, etc...
+        * @param string $format One of (raw, quoted)
+        * @return string Relation name with quoted and merged $namespace as needed
+        */
+       private function prependDatabaseOrSchema( $namespace, $relation, $format ) {
+               if ( strlen( $namespace ) ) {
+                       if ( $format === 'quoted' && !$this->isQuotedIdentifier( $namespace ) ) {
+                               $namespace = $this->addIdentifierQuotes( $namespace );
                        }
-                       $tableName = $database . '.' . $tableName;
+                       $relation = $namespace . '.' . $relation;
                }
 
-               return $tableName;
+               return $relation;
        }
 
        public function tableNames() {
index 7acd8dc..e0333a6 100644 (file)
@@ -152,6 +152,8 @@ class DatabasePostgres extends Database {
                }
 
                $this->determineCoreSchema( $this->mSchema );
+               // The schema to be used is now in the search path; no need for explicit qualification
+               $this->mSchema = null;
 
                return $this->mConn;
        }
@@ -768,19 +770,33 @@ __INDEXATTR__;
        }
 
        function tableName( $name, $format = 'quoted' ) {
-               # Replace reserved words with better ones
-               switch ( $name ) {
-                       case 'user':
-                               return $this->realTableName( 'mwuser', $format );
-                       case 'text':
-                               return $this->realTableName( 'pagecontent', $format );
-                       default:
-                               return $this->realTableName( $name, $format );
+               // Replace reserved words with better ones
+               $name = $this->remappedTableName( $name );
+
+               return parent::tableName( $name, $format );
+       }
+
+       /**
+        * @param string $name
+        * @return string Value of $name or remapped name if $name is a reserved keyword
+        * @TODO: dependency inject these...
+        */
+       public function remappedTableName( $name ) {
+               if ( $name === 'user' ) {
+                       return 'mwuser';
+               } elseif ( $name === 'text' ) {
+                       return 'pagecontent';
                }
+
+               return $name;
        }
 
-       /* Don't cheat on installer */
-       function realTableName( $name, $format = 'quoted' ) {
+       /**
+        * @param string $name
+        * @param string $format
+        * @return string Qualified and encoded (if requested) table name
+        */
+       public function realTableName( $name, $format = 'quoted' ) {
                return parent::tableName( $name, $format );
        }
 
@@ -1090,7 +1106,6 @@ __INDEXATTR__;
                if ( $schema === false ) {
                        $schema = $this->getCoreSchema();
                }
-               $table = $this->realTableName( $table, 'raw' );
                $etable = $this->addQuotes( $table );
                $eschema = $this->addQuotes( $schema );
                $sql = "SELECT 1 FROM pg_catalog.pg_class c, pg_catalog.pg_namespace n "
index 36337e2..d34c125 100644 (file)
@@ -9,7 +9,7 @@ class PostgresField implements Field {
         * @param string $field
         * @return null|PostgresField
         */
-       static function fromText( $db, $table, $field ) {
+       static function fromText( DatabasePostgres $db, $table, $field ) {
                $q = <<<SQL
 SELECT
  attnotnull, attlen, conname AS conname,
@@ -34,7 +34,7 @@ AND relname=%s
 AND attname=%s;
 SQL;
 
-               $table = $db->tableName( $table, 'raw' );
+               $table = $db->remappedTableName( $table );
                $res = $db->query(
                        sprintf( $q,
                                $db->addQuotes( $db->getCoreSchema() ),