Merge "rdbms: move assertOpen() call near the top of Database::query"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Fri, 6 Apr 2018 16:46:28 +0000 (16:46 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Fri, 6 Apr 2018 16:46:28 +0000 (16:46 +0000)
20 files changed:
RELEASE-NOTES-1.31
docs/hooks.txt
includes/db/MWLBFactory.php
includes/libs/rdbms/database/Database.php
includes/libs/rdbms/database/DatabasePostgres.php
includes/libs/rdbms/database/utils/SavepointPostgres.php
includes/libs/rdbms/field/PostgresField.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
maintenance/cleanupPreferences.php
maintenance/postgres/archives/patch-ts2pagetitle.sql
maintenance/postgres/tables.sql
tests/phpunit/MediaWikiTestCase.php
tests/phpunit/includes/Storage/RevisionStoreDbTest.php
tests/phpunit/includes/db/DatabasePostgresTest.php [new file with mode: 0644]
tests/phpunit/includes/db/DatabaseTestHelper.php
tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php
tests/phpunit/maintenance/DumpTestCase.php

index 23afa4f..029eea5 100644 (file)
@@ -331,6 +331,7 @@ changes to languages because of Phabricator reports.
   can use MediaWikiTitleCodec::getTitleInvalidRegex() instead.
 * HTMLForm & VFormHTMLForm::isVForm(), deprecated in 1.25, have been removed.
 * The ProfileSection class, deprecated in 1.25 and unused, has been removed.
+* Wikimedia\Rdbms\SavepointPostgres is deprecated.
 
 == Compatibility ==
 MediaWiki 1.31 requires PHP 5.5.9 or later. Although HHVM 3.18.5 or later is supported,
index 4e8474b..f35d610 100644 (file)
@@ -1209,6 +1209,14 @@ $row: the DB row for this line
   Currently only data attributes reserved to MediaWiki are allowed
   (see Sanitizer::isReservedDataAttribute).
 
+'DeleteUnknownPreferences': Called by the cleanupPreferences.php maintenance script to build a WHERE clause with which
+to delete preferences that are not known about. This hook is used by extensions that have dynamically-named preferences
+that should not be deleted in the usual cleanup process. For example, the Gadgets extension creates preferences prefixed
+with 'gadget-', and so anything with that prefix is excluded from the deletion.
+&where: An array that will be passed as the $cond parameter to IDatabase::select() to determine what will be deleted
+  from the user_properties table.
+$db: The IDatabase object, useful for accessing $db->buildLike() etc.
+
 'DifferenceEngineAfterLoadNewText': called in DifferenceEngine::loadNewText()
 after the new revision's content has been loaded into the class member variable
 $differenceEngine->mNewContent but before returning true from this function.
index f0a17f7..82d9c1d 100644 (file)
@@ -31,6 +31,10 @@ use Wikimedia\Rdbms\DatabaseDomain;
  * @ingroup Database
  */
 abstract class MWLBFactory {
+
+       /** @var array Cache of already-logged deprecation messages */
+       private static $loggedDeprecations = [];
+
        /**
         * @param array $lbConf Config for LBFactory::__construct()
         * @param Config $mainConfig Main config object from MediaWikiServices
@@ -57,6 +61,7 @@ abstract class MWLBFactory {
                        'connLogger' => LoggerFactory::getInstance( 'DBConnection' ),
                        'perfLogger' => LoggerFactory::getInstance( 'DBPerformance' ),
                        'errorLogger' => [ MWExceptionHandler::class, 'logException' ],
+                       'deprecationLogger' => [ static::class, 'logDeprecation' ],
                        'cliMode' => $wgCommandLineMode,
                        'hostname' => wfHostname(),
                        'readOnlyReason' => $readOnlyMode->getReason(),
@@ -228,4 +233,22 @@ abstract class MWLBFactory {
                        ] );
                }
        }
+
+       /**
+        * Log a database deprecation warning
+        * @param string $msg Deprecation message
+        */
+       public static function logDeprecation( $msg ) {
+               global $wgDevelopmentWarnings;
+
+               if ( isset( self::$loggedDeprecations[$msg] ) ) {
+                       return;
+               }
+               self::$loggedDeprecations[$msg] = true;
+
+               if ( $wgDevelopmentWarnings ) {
+                       trigger_error( $msg, E_USER_DEPRECATED );
+               }
+               wfDebugLog( 'deprecated', $msg, 'private' );
+       }
 }
index d7336c1..c924e4e 100644 (file)
@@ -101,6 +101,8 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        protected $queryLogger;
        /** @var callback Error logging callback */
        protected $errorLogger;
+       /** @var callback Deprecation logging callback */
+       protected $deprecationLogger;
 
        /** @var resource|null Database connection */
        protected $conn = null;
@@ -312,6 +314,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                $this->connLogger = $params['connLogger'];
                $this->queryLogger = $params['queryLogger'];
                $this->errorLogger = $params['errorLogger'];
+               $this->deprecationLogger = $params['deprecationLogger'];
 
                if ( isset( $params['nonNativeInsertSelectBatchSize'] ) ) {
                        $this->nonNativeInsertSelectBatchSize = $params['nonNativeInsertSelectBatchSize'];
@@ -396,6 +399,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         *      includes the agent as a SQL comment.
         *   - trxProfiler: Optional TransactionProfiler instance.
         *   - errorLogger: Optional callback that takes an Exception and logs it.
+        *   - deprecationLogger: Optional callback that takes a string and logs it.
         *   - cliMode: Whether to consider the execution context that of a CLI script.
         *   - agent: Optional name used to identify the end-user in query profiling/logging.
         *   - srvCache: Optional BagOStuff instance to an APC-style cache.
@@ -437,6 +441,11 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                                        trigger_error( get_class( $e ) . ': ' . $e->getMessage(), E_USER_WARNING );
                                };
                        }
+                       if ( !isset( $p['deprecationLogger'] ) ) {
+                               $p['deprecationLogger'] = function ( $msg ) {
+                                       trigger_error( $msg, E_USER_DEPRECATED );
+                               };
+                       }
 
                        /** @var Database $conn */
                        $conn = new $class( $p );
@@ -1145,17 +1154,14 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                                        # In the first case, the only options going forward are (a) ROLLBACK, or
                                        # (b) ROLLBACK TO SAVEPOINT (if one was set). If the later case, the only
                                        # option is ROLLBACK, since the snapshots would have been released.
-                                       if ( is_object( $tempIgnore ) ) {
-                                               // Ugly hack to know that savepoints are in use for postgres
-                                               // FIXME: remove this and make DatabasePostgres use ATOMIC_CANCELABLE
-                                       } else {
-                                               $this->trxStatus = self::STATUS_TRX_ERROR;
-                                               $this->trxStatusCause =
-                                                       $this->makeQueryException( $lastError, $lastErrno, $sql, $fname );
-                                               $tempIgnore = false; // cannot recover
-                                       }
+                                       $this->trxStatus = self::STATUS_TRX_ERROR;
+                                       $this->trxStatusCause =
+                                               $this->makeQueryException( $lastError, $lastErrno, $sql, $fname );
+                                       $tempIgnore = false; // cannot recover
                                } else {
-                                       # Nothing prior was there to lose from the transaction
+                                       # Nothing prior was there to lose from the transaction,
+                                       # so just roll it back.
+                                       $this->doRollback( __METHOD__ . " ($fname)" );
                                        $this->trxStatus = self::STATUS_TRX_OK;
                                }
                        }
@@ -1318,7 +1324,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        private function handleSessionLoss() {
                // Clean up tracking of session-level things...
                // https://dev.mysql.com/doc/refman/5.7/en/implicit-commit.html
-               // https://www.postgresql.org/docs/9.1/static/sql-createtable.html (ignoring ON COMMIT)
+               // https://www.postgresql.org/docs/9.2/static/sql-createtable.html (ignoring ON COMMIT)
                $this->sessionTempTables = [];
                // https://dev.mysql.com/doc/refman/5.7/en/miscellaneous-functions.html#function_get-lock
                // https://www.postgresql.org/docs/9.4/static/functions-admin.html#FUNCTIONS-ADVISORY-LOCKS
index 9ffcc8b..525d308 100644 (file)
@@ -36,8 +36,6 @@ class DatabasePostgres extends Database {
 
        /** @var resource */
        protected $lastResultHandle = null;
-       /** @var int The number of rows affected as an integer */
-       protected $lastAffectedRowCount = null;
 
        /** @var float|string */
        private $numericVersion = null;
@@ -45,6 +43,8 @@ class DatabasePostgres extends Database {
        private $connectString;
        /** @var string */
        private $coreSchema;
+       /** @var string */
+       private $tempSchema;
        /** @var string[] Map of (reserved table name => alternate table name) */
        private $keywordTableMap = [];
 
@@ -75,15 +75,17 @@ class DatabasePostgres extends Database {
        }
 
        public function hasConstraint( $name ) {
-               $conn = $this->getBindingHandle();
-
-               $sql = "SELECT 1 FROM pg_catalog.pg_constraint c, pg_catalog.pg_namespace n " .
-                       "WHERE c.connamespace = n.oid AND conname = '" .
-                       pg_escape_string( $conn, $name ) . "' AND n.nspname = '" .
-                       pg_escape_string( $conn, $this->getCoreSchema() ) . "'";
-               $res = $this->doQuery( $sql );
-
-               return $this->numRows( $res );
+               foreach ( $this->getCoreSchemas() as $schema ) {
+                       $sql = "SELECT 1 FROM pg_catalog.pg_constraint c, pg_catalog.pg_namespace n " .
+                               "WHERE c.connamespace = n.oid AND conname = " .
+                               $this->addQuotes( $name ) . " AND n.nspname = " .
+                               $this->addQuotes( $schema );
+                       $res = $this->doQuery( $sql );
+                       if ( $res && $this->numRows( $res ) ) {
+                               return true;
+                       }
+               }
+               return false;
        }
 
        public function open( $server, $user, $password, $dbName ) {
@@ -155,9 +157,7 @@ class DatabasePostgres extends Database {
                $this->query( "SET datestyle = 'ISO, YMD'", __METHOD__ );
                $this->query( "SET timezone = 'GMT'", __METHOD__ );
                $this->query( "SET standard_conforming_strings = on", __METHOD__ );
-               if ( $this->getServerVersion() >= 9.0 ) {
-                       $this->query( "SET bytea_output = 'escape'", __METHOD__ ); // PHP bug 53127
-               }
+               $this->query( "SET bytea_output = 'escape'", __METHOD__ ); // PHP bug 53127
 
                $this->determineCoreSchema( $this->schema );
                // The schema to be used is now in the search path; no need for explicit qualification
@@ -219,7 +219,6 @@ class DatabasePostgres extends Database {
                        throw new DBUnexpectedError( $this, "Unable to post new query to PostgreSQL\n" );
                }
                $this->lastResultHandle = pg_get_result( $conn );
-               $this->lastAffectedRowCount = null;
                if ( pg_result_error( $this->lastResultHandle ) ) {
                        return false;
                }
@@ -371,10 +370,6 @@ class DatabasePostgres extends Database {
        }
 
        protected function fetchAffectedRowCount() {
-               if ( !is_null( $this->lastAffectedRowCount ) ) {
-                       // Forced result for simulated queries
-                       return $this->lastAffectedRowCount;
-               }
                if ( !$this->lastResultHandle ) {
                        return 0;
                }
@@ -437,59 +432,65 @@ class DatabasePostgres extends Database {
 
        public function indexAttributes( $index, $schema = false ) {
                if ( $schema === false ) {
-                       $schema = $this->getCoreSchema();
-               }
-               /*
-                * A subquery would be not needed if we didn't care about the order
-                * of attributes, but we do
-                */
-               $sql = <<<__INDEXATTR__
-
-                       SELECT opcname,
-                               attname,
-                               i.indoption[s.g] as option,
-                               pg_am.amname
-                       FROM
-                               (SELECT generate_series(array_lower(isub.indkey,1), array_upper(isub.indkey,1)) AS g
-                                       FROM
-                                               pg_index isub
-                                       JOIN pg_class cis
-                                               ON cis.oid=isub.indexrelid
-                                       JOIN pg_namespace ns
-                                               ON cis.relnamespace = ns.oid
-                                       WHERE cis.relname='$index' AND ns.nspname='$schema') AS s,
-                               pg_attribute,
-                               pg_opclass opcls,
-                               pg_am,
-                               pg_class ci
-                               JOIN pg_index i
-                                       ON ci.oid=i.indexrelid
-                               JOIN pg_class ct
-                                       ON ct.oid = i.indrelid
-                               JOIN pg_namespace n
-                                       ON ci.relnamespace = n.oid
-                               WHERE
-                                       ci.relname='$index' AND n.nspname='$schema'
-                                       AND     attrelid = ct.oid
-                                       AND     i.indkey[s.g] = attnum
-                                       AND     i.indclass[s.g] = opcls.oid
-                                       AND     pg_am.oid = opcls.opcmethod
+                       $schemas = $this->getCoreSchemas();
+               } else {
+                       $schemas = [ $schema ];
+               }
+
+               $eindex = $this->addQuotes( $index );
+
+               foreach ( $schemas as $schema ) {
+                       $eschema = $this->addQuotes( $schema );
+                       /*
+                        * A subquery would be not needed if we didn't care about the order
+                        * of attributes, but we do
+                        */
+                       $sql = <<<__INDEXATTR__
+
+                               SELECT opcname,
+                                       attname,
+                                       i.indoption[s.g] as option,
+                                       pg_am.amname
+                               FROM
+                                       (SELECT generate_series(array_lower(isub.indkey,1), array_upper(isub.indkey,1)) AS g
+                                               FROM
+                                                       pg_index isub
+                                               JOIN pg_class cis
+                                                       ON cis.oid=isub.indexrelid
+                                               JOIN pg_namespace ns
+                                                       ON cis.relnamespace = ns.oid
+                                               WHERE cis.relname=$eindex AND ns.nspname=$eschema) AS s,
+                                       pg_attribute,
+                                       pg_opclass opcls,
+                                       pg_am,
+                                       pg_class ci
+                                       JOIN pg_index i
+                                               ON ci.oid=i.indexrelid
+                                       JOIN pg_class ct
+                                               ON ct.oid = i.indrelid
+                                       JOIN pg_namespace n
+                                               ON ci.relnamespace = n.oid
+                                       WHERE
+                                               ci.relname=$eindex AND n.nspname=$eschema
+                                               AND     attrelid = ct.oid
+                                               AND     i.indkey[s.g] = attnum
+                                               AND     i.indclass[s.g] = opcls.oid
+                                               AND     pg_am.oid = opcls.opcmethod
 __INDEXATTR__;
-               $res = $this->query( $sql, __METHOD__ );
-               $a = [];
-               if ( $res ) {
-                       foreach ( $res as $row ) {
-                               $a[] = [
-                                       $row->attname,
-                                       $row->opcname,
-                                       $row->amname,
-                                       $row->option ];
+                       $res = $this->query( $sql, __METHOD__ );
+                       $a = [];
+                       if ( $res ) {
+                               foreach ( $res as $row ) {
+                                       $a[] = [
+                                               $row->attname,
+                                               $row->opcname,
+                                               $row->amname,
+                                               $row->option ];
+                               }
+                               return $a;
                        }
-               } else {
-                       return null;
                }
-
-               return $a;
+               return null;
        }
 
        public function indexUnique( $table, $index, $fname = __METHOD__ ) {
@@ -559,18 +560,7 @@ __INDEXATTR__;
                return parent::selectSQLText( $table, $vars, $conds, $fname, $options, $join_conds );
        }
 
-       /**
-        * INSERT wrapper, inserts an array into a table
-        *
-        * $args may be a single associative array, or an array of these with numeric keys,
-        * for multi-row insert (Postgres version 8.2 and above only).
-        *
-        * @param string $table Name of the table to insert to.
-        * @param array $args Items to insert into the table.
-        * @param string $fname Name of the function, for profiling
-        * @param array|string $options String or array. Valid options: IGNORE
-        * @return bool Success of insert operation. IGNORE always returns true.
-        */
+       /** @inheritDoc */
        public function insert( $table, $args, $fname = __METHOD__, $options = [] ) {
                if ( !count( $args ) ) {
                        return true;
@@ -586,98 +576,68 @@ __INDEXATTR__;
                }
 
                if ( isset( $args[0] ) && is_array( $args[0] ) ) {
-                       $multi = true;
+                       $rows = $args;
                        $keys = array_keys( $args[0] );
                } else {
-                       $multi = false;
+                       $rows = [ $args ];
                        $keys = array_keys( $args );
                }
 
-               // If IGNORE is set, we use savepoints to emulate mysql's behavior
-               // @todo If PostgreSQL 9.5+, we could use ON CONFLICT DO NOTHING instead
-               $savepoint = $olde = null;
-               $numrowsinserted = 0;
-               if ( in_array( 'IGNORE', $options ) ) {
-                       $savepoint = new SavepointPostgres( $this, 'mw', $this->queryLogger );
-                       $olde = error_reporting( 0 );
-                       // For future use, we may want to track the number of actual inserts
-                       // Right now, insert (all writes) simply return true/false
-               }
+               $ignore = in_array( 'IGNORE', $options );
 
                $sql = "INSERT INTO $table (" . implode( ',', $keys ) . ') VALUES ';
 
-               if ( $multi ) {
-                       if ( $this->numericVersion >= 8.2 && !$savepoint ) {
-                               $first = true;
-                               foreach ( $args as $row ) {
-                                       if ( $first ) {
-                                               $first = false;
-                                       } else {
-                                               $sql .= ',';
-                                       }
-                                       $sql .= '(' . $this->makeList( $row ) . ')';
+               if ( $this->numericVersion >= 9.5 || !$ignore ) {
+                       // No IGNORE or our PG has "ON CONFLICT DO NOTHING"
+                       $first = true;
+                       foreach ( $rows as $row ) {
+                               if ( $first ) {
+                                       $first = false;
+                               } else {
+                                       $sql .= ',';
                                }
-                               $res = (bool)$this->query( $sql, $fname, $savepoint );
-                       } else {
-                               $res = true;
-                               $origsql = $sql;
-                               foreach ( $args as $row ) {
-                                       $tempsql = $origsql;
+                               $sql .= '(' . $this->makeList( $row ) . ')';
+                       }
+                       if ( $ignore ) {
+                               $sql .= ' ON CONFLICT DO NOTHING';
+                       }
+                       $this->query( $sql, $fname );
+               } else {
+                       // Emulate IGNORE by doing each row individually, with savepoints
+                       // to roll back as necessary.
+                       $numrowsinserted = 0;
+
+                       $tok = $this->startAtomic( "$fname (outer)", self::ATOMIC_CANCELABLE );
+                       try {
+                               foreach ( $rows as $row ) {
+                                       $tempsql = $sql;
                                        $tempsql .= '(' . $this->makeList( $row ) . ')';
 
-                                       if ( $savepoint ) {
-                                               $savepoint->savepoint();
-                                       }
-
-                                       $tempres = (bool)$this->query( $tempsql, $fname, $savepoint );
-
-                                       if ( $savepoint ) {
-                                               $bar = pg_result_error( $this->lastResultHandle );
-                                               if ( $bar != false ) {
-                                                       $savepoint->rollback();
-                                               } else {
-                                                       $savepoint->release();
-                                                       $numrowsinserted++;
+                                       $this->startAtomic( "$fname (inner)", self::ATOMIC_CANCELABLE );
+                                       try {
+                                               $this->query( $tempsql, $fname );
+                                               $this->endAtomic( "$fname (inner)" );
+                                               $numrowsinserted++;
+                                       } catch ( DBQueryError $e ) {
+                                               $this->cancelAtomic( "$fname (inner)" );
+                                               // Our IGNORE is supposed to ignore duplicate key errors, but not others.
+                                               // (even though MySQL's version apparently ignores all errors)
+                                               if ( $e->errno !== '23505' ) {
+                                                       throw $e;
                                                }
                                        }
-
-                                       // If any of them fail, we fail overall for this function call
-                                       // Note that this will be ignored if IGNORE is set
-                                       if ( !$tempres ) {
-                                               $res = false;
-                                       }
-                               }
-                       }
-               } else {
-                       // Not multi, just a lone insert
-                       if ( $savepoint ) {
-                               $savepoint->savepoint();
-                       }
-
-                       $sql .= '(' . $this->makeList( $args ) . ')';
-                       $res = (bool)$this->query( $sql, $fname, $savepoint );
-                       if ( $savepoint ) {
-                               $bar = pg_result_error( $this->lastResultHandle );
-                               if ( $bar != false ) {
-                                       $savepoint->rollback();
-                               } else {
-                                       $savepoint->release();
-                                       $numrowsinserted++;
                                }
+                       } catch ( Exception $e ) {
+                               $this->cancelAtomic( "$fname (outer)", $tok );
+                               throw $e;
                        }
-               }
-               if ( $savepoint ) {
-                       error_reporting( $olde );
-                       $savepoint->commit();
+                       $this->endAtomic( "$fname (outer)" );
 
                        // Set the affected row count for the whole operation
-                       $this->lastAffectedRowCount = $numrowsinserted;
-
-                       // IGNORE always returns true
-                       return true;
+                       $this->affectedRowCount = $numrowsinserted;
                }
 
-               return $res;
+               return true;
        }
 
        /**
@@ -707,14 +667,31 @@ __INDEXATTR__;
                        $insertOptions = [ $insertOptions ];
                }
 
-               /*
-                * If IGNORE is set, use the non-native version.
-                * @todo If PostgreSQL 9.5+, we could use ON CONFLICT DO NOTHING
-                */
                if ( in_array( 'IGNORE', $insertOptions ) ) {
-                       return $this->nonNativeInsertSelect(
-                               $destTable, $srcTable, $varMap, $conds, $fname, $insertOptions, $selectOptions, $selectJoinConds
-                       );
+                       if ( $this->getServerVersion() >= 9.5 ) {
+                               // Use ON CONFLICT DO NOTHING if we have it for IGNORE
+                               $destTable = $this->tableName( $destTable );
+
+                               $selectSql = $this->selectSQLText(
+                                       $srcTable,
+                                       array_values( $varMap ),
+                                       $conds,
+                                       $fname,
+                                       $selectOptions,
+                                       $selectJoinConds
+                               );
+
+                               $sql = "INSERT INTO $destTable (" . implode( ',', array_keys( $varMap ) ) . ') ' .
+                                       $selectSql . ' ON CONFLICT DO NOTHING';
+
+                               return $this->query( $sql, $fname );
+                       } else {
+                               // IGNORE and we don't have ON CONFLICT DO NOTHING, so just use the non-native version
+                               return $this->nonNativeInsertSelect(
+                                       $destTable, $srcTable, $varMap, $conds, $fname,
+                                       $insertOptions, $selectOptions, $selectJoinConds
+                               );
+                       }
                }
 
                return parent::nativeInsertSelect( $destTable, $srcTable, $varMap, $conds, $fname,
@@ -786,17 +763,17 @@ __INDEXATTR__;
        }
 
        public function wasDeadlock() {
-               // https://www.postgresql.org/docs/8.2/static/errcodes-appendix.html
+               // https://www.postgresql.org/docs/9.2/static/errcodes-appendix.html
                return $this->lastErrno() === '40P01';
        }
 
        public function wasLockTimeout() {
-               // https://www.postgresql.org/docs/8.2/static/errcodes-appendix.html
+               // https://www.postgresql.org/docs/9.2/static/errcodes-appendix.html
                return $this->lastErrno() === '55P03';
        }
 
        public function wasConnectionError( $errno ) {
-               // https://www.postgresql.org/docs/8.2/static/errcodes-appendix.html
+               // https://www.postgresql.org/docs/9.2/static/errcodes-appendix.html
                static $codes = [ '08000', '08003', '08006', '08001', '08004', '57P01', '57P03', '53300' ];
 
                return in_array( $errno, $codes, true );
@@ -809,17 +786,81 @@ __INDEXATTR__;
        public function duplicateTableStructure(
                $oldName, $newName, $temporary = false, $fname = __METHOD__
        ) {
-               $newName = $this->addIdentifierQuotes( $newName );
-               $oldName = $this->addIdentifierQuotes( $oldName );
+               $newNameE = $this->addIdentifierQuotes( $newName );
+               $oldNameE = $this->addIdentifierQuotes( $oldName );
+
+               $ret = $this->query( 'CREATE ' . ( $temporary ? 'TEMPORARY ' : '' ) . " TABLE $newNameE " .
+                       "(LIKE $oldNameE INCLUDING DEFAULTS INCLUDING INDEXES)", $fname );
+               if ( !$ret ) {
+                       return $ret;
+               }
+
+               $res = $this->query( 'SELECT attname FROM pg_class c'
+                       . ' JOIN pg_namespace n ON (n.oid = c.relnamespace)'
+                       . ' JOIN pg_attribute a ON (a.attrelid = c.oid)'
+                       . ' JOIN pg_attrdef d ON (c.oid=d.adrelid and a.attnum=d.adnum)'
+                       . ' WHERE relkind = \'r\''
+                       . ' AND nspname = ' . $this->addQuotes( $this->getCoreSchema() )
+                       . ' AND relname = ' . $this->addQuotes( $oldName )
+                       . ' AND adsrc LIKE \'nextval(%\'',
+                       $fname
+               );
+               $row = $this->fetchObject( $res );
+               if ( $row ) {
+                       $field = $row->attname;
+                       $newSeq = "{$newName}_{$field}_seq";
+                       $fieldE = $this->addIdentifierQuotes( $field );
+                       $newSeqE = $this->addIdentifierQuotes( $newSeq );
+                       $newSeqQ = $this->addQuotes( $newSeq );
+                       $this->query( 'CREATE ' . ( $temporary ? 'TEMPORARY ' : '' ) . " SEQUENCE $newSeqE", $fname );
+                       $this->query(
+                               "ALTER TABLE $newNameE ALTER COLUMN $fieldE SET DEFAULT nextval({$newSeqQ}::regclass)",
+                               $fname
+                       );
+               }
 
-               return $this->query( 'CREATE ' . ( $temporary ? 'TEMPORARY ' : '' ) . " TABLE $newName " .
-                       "(LIKE $oldName INCLUDING DEFAULTS INCLUDING INDEXES)", $fname );
+               return $ret;
+       }
+
+       public function resetSequenceForTable( $table, $fname = __METHOD__ ) {
+               $table = $this->tableName( $table, 'raw' );
+               foreach ( $this->getCoreSchemas() as $schema ) {
+                       $res = $this->query(
+                               'SELECT c.oid FROM pg_class c JOIN pg_namespace n ON (n.oid = c.relnamespace)'
+                               . ' WHERE relkind = \'r\''
+                               . ' AND nspname = ' . $this->addQuotes( $schema )
+                               . ' AND relname = ' . $this->addQuotes( $table ),
+                               $fname
+                       );
+                       if ( !$res || !$this->numRows( $res ) ) {
+                               continue;
+                       }
+
+                       $oid = $this->fetchObject( $res )->oid;
+                       $res = $this->query( 'SELECT adsrc FROM pg_attribute a'
+                               . ' JOIN pg_attrdef d ON (a.attrelid=d.adrelid and a.attnum=d.adnum)'
+                               . " WHERE a.attrelid = $oid"
+                               . ' AND adsrc LIKE \'nextval(%\'',
+                               $fname
+                       );
+                       $row = $this->fetchObject( $res );
+                       if ( $row ) {
+                               $this->query(
+                                       'SELECT ' . preg_replace( '/^nextval\((.+)\)$/', 'setval($1,1,false)', $row->adsrc ),
+                                       $fname
+                               );
+                               return true;
+                       }
+                       return false;
+               }
+
+               return false;
        }
 
        public function listTables( $prefix = null, $fname = __METHOD__ ) {
-               $eschema = $this->addQuotes( $this->getCoreSchema() );
+               $eschemas = implode( ',', array_map( [ $this, 'addQuotes' ], $this->getCoreSchemas() ) );
                $result = $this->query(
-                       "SELECT tablename FROM pg_tables WHERE schemaname = $eschema", $fname );
+                       "SELECT DISTINCT tablename FROM pg_tables WHERE schemaname IN ($eschemas)", $fname );
                $endArray = [];
 
                foreach ( $result as $table ) {
@@ -1010,6 +1051,29 @@ __INDEXATTR__;
                return $this->coreSchema;
        }
 
+       /**
+        * Return schema names for temporary tables and core application tables
+        *
+        * @since 1.31
+        * @return string[] schema names
+        */
+       public function getCoreSchemas() {
+               if ( $this->tempSchema ) {
+                       return [ $this->tempSchema, $this->getCoreSchema() ];
+               }
+
+               $res = $this->query(
+                       "SELECT nspname FROM pg_catalog.pg_namespace n WHERE n.oid = pg_my_temp_schema()", __METHOD__
+               );
+               $row = $this->fetchObject( $res );
+               if ( $row ) {
+                       $this->tempSchema = $row->nspname;
+                       return [ $this->tempSchema, $this->getCoreSchema() ];
+               }
+
+               return [ $this->getCoreSchema() ];
+       }
+
        public function getServerVersion() {
                if ( !isset( $this->numericVersion ) ) {
                        $conn = $this->getBindingHandle();
@@ -1042,18 +1106,24 @@ __INDEXATTR__;
                        $types = [ $types ];
                }
                if ( $schema === false ) {
-                       $schema = $this->getCoreSchema();
+                       $schemas = $this->getCoreSchemas();
+               } else {
+                       $schemas = [ $schema ];
                }
                $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 "
-                       . "WHERE c.relnamespace = n.oid AND c.relname = $etable AND n.nspname = $eschema "
-                       . "AND c.relkind IN ('" . implode( "','", $types ) . "')";
-               $res = $this->query( $sql );
-               $count = $res ? $res->numRows() : 0;
+               foreach ( $schemas as $schema ) {
+                       $eschema = $this->addQuotes( $schema );
+                       $sql = "SELECT 1 FROM pg_catalog.pg_class c, pg_catalog.pg_namespace n "
+                               . "WHERE c.relnamespace = n.oid AND c.relname = $etable AND n.nspname = $eschema "
+                               . "AND c.relkind IN ('" . implode( "','", $types ) . "')";
+                       $res = $this->query( $sql );
+                       if ( $res && $res->numRows() ) {
+                               return true;
+                       }
+               }
 
-               return (bool)$count;
+               return false;
        }
 
        /**
@@ -1078,20 +1148,21 @@ __INDEXATTR__;
                        AND tgrelid=pg_class.oid
                        AND nspname=%s AND relname=%s AND tgname=%s
 SQL;
-               $res = $this->query(
-                       sprintf(
-                               $q,
-                               $this->addQuotes( $this->getCoreSchema() ),
-                               $this->addQuotes( $table ),
-                               $this->addQuotes( $trigger )
-                       )
-               );
-               if ( !$res ) {
-                       return null;
+               foreach ( $this->getCoreSchemas() as $schema ) {
+                       $res = $this->query(
+                               sprintf(
+                                       $q,
+                                       $this->addQuotes( $schema ),
+                                       $this->addQuotes( $table ),
+                                       $this->addQuotes( $trigger )
+                               )
+                       );
+                       if ( $res && $res->numRows() ) {
+                               return true;
+                       }
                }
-               $rows = $res->numRows();
 
-               return $rows;
+               return false;
        }
 
        public function ruleExists( $table, $rule ) {
@@ -1099,7 +1170,7 @@ SQL;
                        [
                                'rulename' => $rule,
                                'tablename' => $table,
-                               'schemaname' => $this->getCoreSchema()
+                               'schemaname' => $this->getCoreSchemas()
                        ]
                );
 
@@ -1107,19 +1178,19 @@ SQL;
        }
 
        public function constraintExists( $table, $constraint ) {
-               $sql = sprintf( "SELECT 1 FROM information_schema.table_constraints " .
-                       "WHERE constraint_schema = %s AND table_name = %s AND constraint_name = %s",
-                       $this->addQuotes( $this->getCoreSchema() ),
-                       $this->addQuotes( $table ),
-                       $this->addQuotes( $constraint )
-               );
-               $res = $this->query( $sql );
-               if ( !$res ) {
-                       return null;
+               foreach ( $this->getCoreSchemas() as $schema ) {
+                       $sql = sprintf( "SELECT 1 FROM information_schema.table_constraints " .
+                               "WHERE constraint_schema = %s AND table_name = %s AND constraint_name = %s",
+                               $this->addQuotes( $schema ),
+                               $this->addQuotes( $table ),
+                               $this->addQuotes( $constraint )
+                       );
+                       $res = $this->query( $sql );
+                       if ( $res && $res->numRows() ) {
+                               return true;
+                       }
                }
-               $rows = $res->numRows();
-
-               return $rows;
+               return false;
        }
 
        /**
@@ -1213,28 +1284,6 @@ SQL;
                return "'" . pg_escape_string( $conn, (string)$s ) . "'";
        }
 
-       /**
-        * Postgres specific version of replaceVars.
-        * Calls the parent version in Database.php
-        *
-        * @param string $ins SQL string, read from a stream (usually tables.sql)
-        * @return string SQL string
-        */
-       protected function replaceVars( $ins ) {
-               $ins = parent::replaceVars( $ins );
-
-               if ( $this->numericVersion >= 8.3 ) {
-                       // Thanks for not providing backwards-compatibility, 8.3
-                       $ins = preg_replace( "/to_tsvector\s*\(\s*'default'\s*,/", 'to_tsvector(', $ins );
-               }
-
-               if ( $this->numericVersion <= 8.1 ) { // Our minimum version
-                       $ins = str_replace( 'USING gin', 'USING gist', $ins );
-               }
-
-               return $ins;
-       }
-
        public function makeSelectOptions( $options ) {
                $preLimitTail = $postLimitTail = '';
                $startOpts = $useIndex = $ignoreIndex = '';
@@ -1332,7 +1381,7 @@ SQL;
                if ( !parent::lockIsFree( $lockName, $method ) ) {
                        return false; // already held
                }
-               // http://www.postgresql.org/docs/8.2/static/functions-admin.html#FUNCTIONS-ADVISORY-LOCKS
+               // http://www.postgresql.org/docs/9.2/static/functions-admin.html#FUNCTIONS-ADVISORY-LOCKS
                $key = $this->addQuotes( $this->bigintFromLockName( $lockName ) );
                $result = $this->query( "SELECT (CASE(pg_try_advisory_lock($key))
                        WHEN 'f' THEN 'f' ELSE pg_advisory_unlock($key) END) AS lockstatus", $method );
@@ -1342,7 +1391,7 @@ SQL;
        }
 
        public function lock( $lockName, $method, $timeout = 5 ) {
-               // http://www.postgresql.org/docs/8.2/static/functions-admin.html#FUNCTIONS-ADVISORY-LOCKS
+               // http://www.postgresql.org/docs/9.2/static/functions-admin.html#FUNCTIONS-ADVISORY-LOCKS
                $key = $this->addQuotes( $this->bigintFromLockName( $lockName ) );
                $loop = new WaitConditionLoop(
                        function () use ( $lockName, $key, $timeout, $method ) {
@@ -1362,7 +1411,7 @@ SQL;
        }
 
        public function unlock( $lockName, $method ) {
-               // http://www.postgresql.org/docs/8.2/static/functions-admin.html#FUNCTIONS-ADVISORY-LOCKS
+               // http://www.postgresql.org/docs/9.2/static/functions-admin.html#FUNCTIONS-ADVISORY-LOCKS
                $key = $this->addQuotes( $this->bigintFromLockName( $lockName ) );
                $result = $this->query( "SELECT pg_advisory_unlock($key) as lockstatus", $method );
                $row = $this->fetchObject( $result );
index cf5060e..edbcdfe 100644 (file)
@@ -27,6 +27,7 @@ use Psr\Log\LoggerInterface;
  * Manage savepoints within a transaction
  * @ingroup Database
  * @since 1.19
+ * @deprecated since 1.31, use IDatabase::startAtomic() and such instead.
  */
 class SavepointPostgres {
        /** @var DatabasePostgres Establish a savepoint within a transaction */
index 600f34a..53c3d33 100644 (file)
@@ -38,30 +38,34 @@ AND attname=%s;
 SQL;
 
                $table = $db->remappedTableName( $table );
-               $res = $db->query(
-                       sprintf( $q,
-                               $db->addQuotes( $db->getCoreSchema() ),
-                               $db->addQuotes( $table ),
-                               $db->addQuotes( $field )
-                       )
-               );
-               $row = $db->fetchObject( $res );
-               if ( !$row ) {
-                       return null;
+               foreach ( $db->getCoreSchemas() as $schema ) {
+                       $res = $db->query(
+                               sprintf( $q,
+                                       $db->addQuotes( $schema ),
+                                       $db->addQuotes( $table ),
+                                       $db->addQuotes( $field )
+                               )
+                       );
+                       $row = $db->fetchObject( $res );
+                       if ( !$row ) {
+                               continue;
+                       }
+                       $n = new PostgresField;
+                       $n->type = $row->typname;
+                       $n->nullable = ( $row->attnotnull == 'f' );
+                       $n->name = $field;
+                       $n->tablename = $table;
+                       $n->max_length = $row->attlen;
+                       $n->deferrable = ( $row->deferrable == 't' );
+                       $n->deferred = ( $row->deferred == 't' );
+                       $n->conname = $row->conname;
+                       $n->has_default = ( $row->atthasdef === 't' );
+                       $n->default = $row->adsrc;
+
+                       return $n;
                }
-               $n = new PostgresField;
-               $n->type = $row->typname;
-               $n->nullable = ( $row->attnotnull == 'f' );
-               $n->name = $field;
-               $n->tablename = $table;
-               $n->max_length = $row->attlen;
-               $n->deferrable = ( $row->deferrable == 't' );
-               $n->deferred = ( $row->deferred == 't' );
-               $n->conname = $row->conname;
-               $n->has_default = ( $row->atthasdef === 't' );
-               $n->default = $row->adsrc;
 
-               return $n;
+               return null;
        }
 
        function name() {
index 32d9008..1e8838e 100644 (file)
@@ -55,6 +55,7 @@ interface ILBFactory {
         *  - queryLogger: PSR-3 logger instance. [optional]
         *  - perfLogger: PSR-3 logger instance. [optional]
         *  - errorLogger: Callback that takes an Exception and logs it. [optional]
+        *  - deprecationLogger: Callback to log a deprecation warning. [optional]
         * @throws InvalidArgumentException
         */
        public function __construct( array $conf );
index bc428ec..b1ea810 100644 (file)
@@ -52,6 +52,8 @@ abstract class LBFactory implements ILBFactory {
        protected $perfLogger;
        /** @var callable Error logger */
        protected $errorLogger;
+       /** @var callable Deprecation logger */
+       protected $deprecationLogger;
        /** @var BagOStuff */
        protected $srvCache;
        /** @var BagOStuff */
@@ -109,7 +111,12 @@ abstract class LBFactory implements ILBFactory {
                $this->errorLogger = isset( $conf['errorLogger'] )
                        ? $conf['errorLogger']
                        : function ( Exception $e ) {
-                               trigger_error( E_USER_WARNING, get_class( $e ) . ': ' . $e->getMessage() );
+                               trigger_error( get_class( $e ) . ': ' . $e->getMessage(), E_USER_WARNING );
+                       };
+               $this->deprecationLogger = isset( $conf['deprecationLogger'] )
+                       ? $conf['deprecationLogger']
+                       : function ( $msg ) {
+                               trigger_error( $msg, E_USER_DEPRECATED );
                        };
 
                $this->profiler = isset( $conf['profiler'] ) ? $conf['profiler'] : null;
@@ -514,6 +521,7 @@ abstract class LBFactory implements ILBFactory {
                        'connLogger' => $this->connLogger,
                        'replLogger' => $this->replLogger,
                        'errorLogger' => $this->errorLogger,
+                       'deprecationLogger' => $this->deprecationLogger,
                        'hostname' => $this->hostname,
                        'cliMode' => $this->cliMode,
                        'agent' => $this->agent,
index 767cc49..715f4e4 100644 (file)
@@ -109,6 +109,7 @@ interface ILoadBalancer {
         *  - queryLogger: PSR-3 logger instance. [optional]
         *  - perfLogger: PSR-3 logger instance. [optional]
         *  - errorLogger : Callback that takes an Exception and logs it. [optional]
+        *  - deprecationLogger: Callback to log a deprecation warning. [optional]
         * @throws InvalidArgumentException
         */
        public function __construct( array $params );
index 7c1b9d9..db2ab1f 100644 (file)
@@ -111,6 +111,8 @@ class LoadBalancer implements ILoadBalancer {
 
        /** @var callable Exception logger */
        private $errorLogger;
+       /** @var callable Deprecation logger */
+       private $deprecationLogger;
 
        /** @var bool */
        private $disabled = false;
@@ -223,6 +225,11 @@ class LoadBalancer implements ILoadBalancer {
                        : function ( Exception $e ) {
                                trigger_error( get_class( $e ) . ': ' . $e->getMessage(), E_USER_WARNING );
                        };
+               $this->deprecationLogger = isset( $params['deprecationLogger'] )
+                       ? $params['deprecationLogger']
+                       : function ( $msg ) {
+                               trigger_error( $msg, E_USER_DEPRECATED );
+                       };
 
                foreach ( [ 'replLogger', 'connLogger', 'queryLogger', 'perfLogger' ] as $key ) {
                        $this->$key = isset( $params[$key] ) ? $params[$key] : new NullLogger();
@@ -1067,6 +1074,7 @@ class LoadBalancer implements ILoadBalancer {
                $server['connLogger'] = $this->connLogger;
                $server['queryLogger'] = $this->queryLogger;
                $server['errorLogger'] = $this->errorLogger;
+               $server['deprecationLogger'] = $this->deprecationLogger;
                $server['profiler'] = $this->profiler;
                $server['trxProfiler'] = $this->trxProfiler;
                // Use the same agent and PHP mode for all DB handles
index a21bc73..33cc0ca 100644 (file)
@@ -87,15 +87,14 @@ class CleanupPreferences extends Maintenance {
                // Remove unknown preferences. Special-case gadget- and userjs- as we can't
                // control those names.
                if ( $unknown ) {
-                       $this->deleteByWhere(
-                               $dbw,
-                               'Dropping unknown preferences',
-                               [
-                                       'up_property NOT' . $dbw->buildLike( 'gadget-', $dbw->anyString() ),
-                                       'up_property NOT' . $dbw->buildLike( 'userjs-', $dbw->anyString() ),
-                                       'up_property NOT IN (' . $dbw->makeList( array_keys( $wgDefaultUserOptions ) ) . ')',
-                               ]
-                       );
+                       $where = [
+                               'up_property NOT' . $dbw->buildLike( 'gadget-', $dbw->anyString() ),
+                               'up_property NOT' . $dbw->buildLike( 'userjs-', $dbw->anyString() ),
+                               'up_property NOT IN (' . $dbw->makeList( array_keys( $wgDefaultUserOptions ) ) . ')',
+                       ];
+                       // Allow extensions to add to the where clause to prevent deletion of their own prefs.
+                       Hooks::run( 'DeleteUnknownPreferences', [ &$where, $dbw ] );
+                       $this->deleteByWhere( $dbw, 'Dropping unknown preferences', $where );
                }
 
                // Something something phase 3
index 4ac985e..a770c91 100644 (file)
@@ -4,9 +4,9 @@ LANGUAGE plpgsql AS
 $mw$
 BEGIN
 IF TG_OP = 'INSERT' THEN
-  NEW.titlevector = to_tsvector('default',REPLACE(NEW.page_title,'/',' '));
+  NEW.titlevector = to_tsvector(REPLACE(NEW.page_title,'/',' '));
 ELSIF NEW.page_title != OLD.page_title THEN
-  NEW.titlevector := to_tsvector('default',REPLACE(NEW.page_title,'/',' '));
+  NEW.titlevector := to_tsvector(REPLACE(NEW.page_title,'/',' '));
 END IF;
 RETURN NEW;
 END;
index 271071b..d9429bc 100644 (file)
@@ -687,7 +687,6 @@ CREATE INDEX job_cmd_namespace_title ON job (job_cmd, job_namespace, job_title);
 CREATE INDEX job_timestamp_idx ON job (job_timestamp);
 
 -- Tsearch2 2 stuff. Will fail if we don't have proper access to the tsearch2 tables
--- Version 8.3 or higher only. Previous versions would need another parmeter for to_tsvector.
 -- Make sure you also change patch-tsearch2funcs.sql if the funcs below change.
 
 ALTER TABLE page ADD titlevector tsvector;
@@ -723,9 +722,6 @@ $mw$;
 CREATE TRIGGER ts2_page_text BEFORE INSERT OR UPDATE ON pagecontent
   FOR EACH ROW EXECUTE PROCEDURE ts2_page_text();
 
--- These are added by the setup script due to version compatibility issues
--- If using 8.1, we switch from "gin" to "gist"
-
 CREATE INDEX ts2_page_title ON page USING gin(titlevector);
 CREATE INDEX ts2_page_text ON pagecontent USING gin(textvector);
 
index 0d2b788..ffba861 100644 (file)
@@ -1539,6 +1539,11 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
                                        $db->delete( $tbl, '*', __METHOD__ );
                                }
 
+                               if ( $db->getType() === 'postgres' ) {
+                                       // Reset the table's sequence too.
+                                       $db->resetSequenceForTable( $tbl, __METHOD__ );
+                               }
+
                                if ( $tbl === 'page' ) {
                                        // Forget about the pages since they don't
                                        // exist in the DB.
index 719a3bf..7d6906c 100644 (file)
@@ -117,7 +117,10 @@ class RevisionStoreDbTest extends MediaWikiTestCase {
                                'trxProfiler' => new TransactionProfiler(),
                                'connLogger' => new \Psr\Log\NullLogger(),
                                'queryLogger' => new \Psr\Log\NullLogger(),
-                               'errorLogger' => new \Psr\Log\NullLogger(),
+                               'errorLogger' => function () {
+                               },
+                               'deprecationLogger' => function () {
+                               },
                                'type' => 'test',
                                'dbname' => $dbName,
                                'tablePrefix' => $dbPrefix,
diff --git a/tests/phpunit/includes/db/DatabasePostgresTest.php b/tests/phpunit/includes/db/DatabasePostgresTest.php
new file mode 100644 (file)
index 0000000..5c2aa2b
--- /dev/null
@@ -0,0 +1,177 @@
+<?php
+
+use Wikimedia\Rdbms\IDatabase;
+use Wikimedia\Rdbms\DatabasePostgres;
+use Wikimedia\ScopedCallback;
+use Wikimedia\TestingAccessWrapper;
+
+/**
+ * @group Database
+ */
+class DatabasePostgresTest extends MediaWikiTestCase {
+
+       private function doTestInsertIgnore() {
+               $reset = new ScopedCallback( function () {
+                       if ( $this->db->explicitTrxActive() ) {
+                               $this->db->rollback( __METHOD__ );
+                       }
+                       $this->db->query( 'DROP TABLE IF EXISTS ' . $this->db->tableName( 'foo' ) );
+               } );
+
+               $this->db->query(
+                       "CREATE TEMPORARY TABLE {$this->db->tableName( 'foo' )} (i INTEGER NOT NULL PRIMARY KEY)"
+               );
+               $this->db->insert( 'foo', [ [ 'i' => 1 ], [ 'i' => 2 ] ], __METHOD__ );
+
+               // Normal INSERT IGNORE
+               $this->db->begin( __METHOD__ );
+               $this->db->insert(
+                       'foo', [ [ 'i' => 3 ], [ 'i' => 2 ], [ 'i' => 5 ] ], __METHOD__, [ 'IGNORE' ]
+               );
+               $this->assertSame( 2, $this->db->affectedRows() );
+               $this->assertSame(
+                       [ '1', '2', '3', '5' ],
+                       $this->db->selectFieldValues( 'foo', 'i', [], __METHOD__, [ 'ORDER BY' => 'i' ] )
+               );
+               $this->db->rollback( __METHOD__ );
+
+               // INSERT IGNORE doesn't ignore stuff like NOT NULL violations
+               $this->db->begin( __METHOD__ );
+               $this->db->startAtomic( __METHOD__, IDatabase::ATOMIC_CANCELABLE );
+               try {
+                       $this->db->insert(
+                               'foo', [ [ 'i' => 7 ], [ 'i' => null ] ], __METHOD__, [ 'IGNORE' ]
+                       );
+                       $this->db->endAtomic( __METHOD__ );
+                       $this->fail( 'Expected exception not thrown' );
+               } catch ( DBQueryError $e ) {
+                       $this->assertSame( 0, $this->db->affectedRows() );
+                       $this->db->cancelAtomic( __METHOD__ );
+               }
+               $this->assertSame(
+                       [ '1', '2' ],
+                       $this->db->selectFieldValues( 'foo', 'i', [], __METHOD__, [ 'ORDER BY' => 'i' ] )
+               );
+               $this->db->rollback( __METHOD__ );
+       }
+
+       /**
+        * @covers Wikimedia\Rdbms\DatabasePostgres::insert
+        */
+       public function testInsertIgnoreOld() {
+               if ( !$this->db instanceof DatabasePostgres ) {
+                       $this->markTestSkipped( 'Not PostgreSQL' );
+               }
+               if ( $this->db->getServerVersion() < 9.5 ) {
+                       $this->doTestInsertIgnore();
+               } else {
+                       // Hack version to make it take the old code path
+                       $w = TestingAccessWrapper::newFromObject( $this->db );
+                       $oldVer = $w->numericVersion;
+                       $w->numericVersion = 9.4;
+                       try {
+                               $this->doTestInsertIgnore();
+                       } finally {
+                               $w->numericVersion = $oldVer;
+                       }
+               }
+       }
+
+       /**
+        * @covers Wikimedia\Rdbms\DatabasePostgres::insert
+        */
+       public function testInsertIgnoreNew() {
+               if ( !$this->db instanceof DatabasePostgres ) {
+                       $this->markTestSkipped( 'Not PostgreSQL' );
+               }
+               if ( $this->db->getServerVersion() < 9.5 ) {
+                       $this->markTestSkipped( 'PostgreSQL version is ' . $this->db->getServerVersion() );
+               }
+
+               $this->doTestInsertIgnore();
+       }
+
+       private function doTestInsertSelectIgnore() {
+               $reset = new ScopedCallback( function () {
+                       if ( $this->db->explicitTrxActive() ) {
+                               $this->db->rollback( __METHOD__ );
+                       }
+                       $this->db->query( 'DROP TABLE IF EXISTS ' . $this->db->tableName( 'foo' ) );
+                       $this->db->query( 'DROP TABLE IF EXISTS ' . $this->db->tableName( 'bar' ) );
+               } );
+
+               $this->db->query(
+                       "CREATE TEMPORARY TABLE {$this->db->tableName( 'foo' )} (i INTEGER)"
+               );
+               $this->db->query(
+                       "CREATE TEMPORARY TABLE {$this->db->tableName( 'bar' )} (i INTEGER NOT NULL PRIMARY KEY)"
+               );
+               $this->db->insert( 'bar', [ [ 'i' => 1 ], [ 'i' => 2 ] ], __METHOD__ );
+
+               // Normal INSERT IGNORE
+               $this->db->begin( __METHOD__ );
+               $this->db->insert( 'foo', [ [ 'i' => 3 ], [ 'i' => 2 ], [ 'i' => 5 ] ], __METHOD__ );
+               $this->db->insertSelect( 'bar', 'foo', [ 'i' => 'i' ], [], __METHOD__, [ 'IGNORE' ] );
+               $this->assertSame( 2, $this->db->affectedRows() );
+               $this->assertSame(
+                       [ '1', '2', '3', '5' ],
+                       $this->db->selectFieldValues( 'bar', 'i', [], __METHOD__, [ 'ORDER BY' => 'i' ] )
+               );
+               $this->db->rollback( __METHOD__ );
+
+               // INSERT IGNORE doesn't ignore stuff like NOT NULL violations
+               $this->db->begin( __METHOD__ );
+               $this->db->insert( 'foo', [ [ 'i' => 7 ], [ 'i' => null ] ], __METHOD__ );
+               $this->db->startAtomic( __METHOD__, IDatabase::ATOMIC_CANCELABLE );
+               try {
+                       $this->db->insertSelect( 'bar', 'foo', [ 'i' => 'i' ], [], __METHOD__, [ 'IGNORE' ] );
+                       $this->db->endAtomic( __METHOD__ );
+                       $this->fail( 'Expected exception not thrown' );
+               } catch ( DBQueryError $e ) {
+                       $this->assertSame( 0, $this->db->affectedRows() );
+                       $this->db->cancelAtomic( __METHOD__ );
+               }
+               $this->assertSame(
+                       [ '1', '2' ],
+                       $this->db->selectFieldValues( 'bar', 'i', [], __METHOD__, [ 'ORDER BY' => 'i' ] )
+               );
+               $this->db->rollback( __METHOD__ );
+       }
+
+       /**
+        * @covers Wikimedia\Rdbms\DatabasePostgres::nativeInsertSelect
+        */
+       public function testInsertSelectIgnoreOld() {
+               if ( !$this->db instanceof DatabasePostgres ) {
+                       $this->markTestSkipped( 'Not PostgreSQL' );
+               }
+               if ( $this->db->getServerVersion() < 9.5 ) {
+                       $this->doTestInsertSelectIgnore();
+               } else {
+                       // Hack version to make it take the old code path
+                       $w = TestingAccessWrapper::newFromObject( $this->db );
+                       $oldVer = $w->numericVersion;
+                       $w->numericVersion = 9.4;
+                       try {
+                               $this->doTestInsertSelectIgnore();
+                       } finally {
+                               $w->numericVersion = $oldVer;
+                       }
+               }
+       }
+
+       /**
+        * @covers Wikimedia\Rdbms\DatabasePostgres::nativeInsertSelect
+        */
+       public function testInsertSelectIgnoreNew() {
+               if ( !$this->db instanceof DatabasePostgres ) {
+                       $this->markTestSkipped( 'Not PostgreSQL' );
+               }
+               if ( $this->db->getServerVersion() < 9.5 ) {
+                       $this->markTestSkipped( 'PostgreSQL version is ' . $this->db->getServerVersion() );
+               }
+
+               $this->doTestInsertSelectIgnore();
+       }
+
+}
index 854479d..36254f7 100644 (file)
@@ -26,6 +26,11 @@ class DatabaseTestHelper extends Database {
        /** @var array List of row arrays */
        protected $nextResult = [];
 
+       /** @var array|null */
+       protected $nextError = null;
+       /** @var array|null */
+       protected $lastError = null;
+
        /**
         * Array of tables to be considered as existing by tableExist()
         * Use setExistingTables() to alter.
@@ -75,6 +80,16 @@ class DatabaseTestHelper extends Database {
                $this->nextResult = $res;
        }
 
+       /**
+        * @param int $errno Error number
+        * @param string $error Error text
+        * @param array $options
+        *  - wasKnownStatementRollbackError: Return value for wasKnownStatementRollbackError()
+        */
+       public function forceNextQueryError( $errno, $error, $options = [] ) {
+               $this->nextError = [ 'errno' => $errno, 'error' => $error ] + $options;
+       }
+
        protected function addSql( $sql ) {
                // clean up spaces before and after some words and the whole string
                $this->lastSqls[] = trim( preg_replace(
@@ -88,7 +103,13 @@ class DatabaseTestHelper extends Database {
                        return; // no $fname parameter
                }
 
-               if ( substr( $fname, 0, strlen( $this->testName ) ) !== $this->testName ) {
+               // Handle some internal calls from the Database class
+               $check = $fname;
+               if ( preg_match( '/^Wikimedia\\\\Rdbms\\\\Database::query \((.+)\)$/', $fname, $m ) ) {
+                       $check = $m[1];
+               }
+
+               if ( substr( $check, 0, strlen( $this->testName ) ) !== $this->testName ) {
                        throw new MWException( 'function name does not start with test class. ' .
                                $fname . ' vs. ' . $this->testName . '. ' .
                                'Please provide __METHOD__ to database methods.' );
@@ -107,7 +128,6 @@ class DatabaseTestHelper extends Database {
 
        public function query( $sql, $fname = '', $tempIgnore = false ) {
                $this->checkFunctionName( $fname );
-               $this->addSql( $sql );
 
                return parent::query( $sql, $fname, $tempIgnore );
        }
@@ -167,11 +187,17 @@ class DatabaseTestHelper extends Database {
        }
 
        function lastErrno() {
-               return -1;
+               return $this->lastError ? $this->lastError['errno'] : -1;
        }
 
        function lastError() {
-               return 'test';
+               return $this->lastError ? $this->lastError['error'] : 'test';
+       }
+
+       protected function wasKnownStatementRollbackError() {
+               return isset( $this->lastError['wasKnownStatementRollbackError'] )
+                       ? $this->lastError['wasKnownStatementRollbackError']
+                       : false;
        }
 
        function fieldInfo( $table, $field ) {
@@ -212,8 +238,18 @@ class DatabaseTestHelper extends Database {
        }
 
        protected function doQuery( $sql ) {
+               $sql = preg_replace( '< /\* .+?  \*/>', '', $sql );
+               $this->addSql( $sql );
+
+               if ( $this->nextError ) {
+                       $this->lastError = $this->nextError;
+                       $this->nextError = null;
+                       return false;
+               }
+
                $res = $this->nextResult;
                $this->nextResult = [];
+               $this->lastError = null;
 
                return new FakeResultWrapper( $res );
        }
index 5a06cc7..40e07d8 100644 (file)
@@ -4,6 +4,7 @@ use Wikimedia\Rdbms\IDatabase;
 use Wikimedia\Rdbms\LikeMatch;
 use Wikimedia\Rdbms\Database;
 use Wikimedia\TestingAccessWrapper;
+use Wikimedia\Rdbms\DBTransactionStateError;
 use Wikimedia\Rdbms\DBUnexpectedError;
 
 /**
@@ -1540,6 +1541,44 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
                $this->assertEquals( 0, $this->database->trxLevel() );
        }
 
+       /**
+        * @covers \Wikimedia\Rdbms\Database::query
+        */
+       public function testImplicitTransactionRollback() {
+               $doError = function ( $wasKnown = true ) {
+                       $this->database->forceNextQueryError( 666, 'Evilness' );
+                       try {
+                               $this->database->delete( 'error', '1', __CLASS__ . '::SomeCaller' );
+                               $this->fail( 'Expected exception not thrown' );
+                       } catch ( DBError $e ) {
+                               $this->assertSame( 666, $e->errno );
+                       }
+               };
+
+               $this->database->setFlag( Database::DBO_TRX );
+
+               // Implicit transaction gets silently rolled back
+               $this->database->begin( __METHOD__, Database::TRANSACTION_INTERNAL );
+               call_user_func( $doError, false );
+               $this->database->delete( 'x', [ 'field' => 1 ], __METHOD__ );
+               $this->database->commit( __METHOD__, Database::FLUSHING_INTERNAL );
+               // phpcs:ignore
+               $this->assertLastSql( 'BEGIN; DELETE FROM error WHERE 1; ROLLBACK; BEGIN; DELETE FROM x WHERE field = \'1\'; COMMIT' );
+
+               // ... unless there were prior writes
+               $this->database->begin( __METHOD__, Database::TRANSACTION_INTERNAL );
+               $this->database->delete( 'x', [ 'field' => 1 ], __METHOD__ );
+               call_user_func( $doError, false );
+               try {
+                       $this->database->delete( 'x', [ 'field' => 1 ], __METHOD__ );
+                       $this->fail( 'Expected exception not thrown' );
+               } catch ( DBTransactionStateError $e ) {
+               }
+               $this->database->rollback( __METHOD__, Database::FLUSHING_INTERNAL );
+               // phpcs:ignore
+               $this->assertLastSql( 'BEGIN; DELETE FROM x WHERE field = \'1\'; DELETE FROM error WHERE 1; ROLLBACK' );
+       }
+
        /**
         * @covers \Wikimedia\Rdbms\Database::close
         */
index c872993..9b90bfe 100644 (file)
@@ -8,6 +8,7 @@ use MediaWikiLangTestCase;
 use Page;
 use User;
 use XMLReader;
+use MWException;
 
 /**
  * Base TestCase for dumps