Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

rdbms: Remove support for PostgreSQL < 9.2, and improve INSERT IGNORE for 9.5

Browse source 
MediaWiki doesn't support PostgreSQL < 9.2, so drop the support for
older versions.

At the same time, since we're messing with the DatabasePostgres::insert()
code anyway, let's start using ON CONFLICT DO NOTHING for PG >= 9.5.

And since we're doing that, let's do the same for
DatabasePostgres::nativeInsertSelect().

Change-Id: I7bf13c3272917ebafeaff11eb116714a099afdf3
This commit is contained in:
Brad Jorsch 2018-03-17 20:29:31 -04:00 committed by Aaron Schulz
parent ea62a9780c
commit cc0473766a
7 changed files with 261 additions and 146 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
RELEASE-NOTES-1.31
View 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,

15
includes/libs/rdbms/database/Database.php
View file

@ -1145,15 +1145,10 @@ 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
$this->trxStatus = self::STATUS_TRX_OK;
@ -1318,7 +1313,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

205
includes/libs/rdbms/database/DatabasePostgres.php
View 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;
@ -155,9 +153,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 +215,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 +366,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;
}
@ -559,18 +550,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 +566,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;
}
}
} catch ( Exception $e ) {
$this->cancelAtomic( "$fname (outer)", $tok );
throw $e;
}
} 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++;
}
}
}
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 +657,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 +753,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 );
@ -1213,28 +1180,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 +1277,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 +1287,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 +1307,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 );

1
includes/libs/rdbms/database/utils/SavepointPostgres.php
View 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 */

4
maintenance/postgres/archives/patch-ts2pagetitle.sql
View 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;

4
maintenance/postgres/tables.sql
View 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);

177
tests/phpunit/includes/db/DatabasePostgresTest.php Normal file
View file

@ -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();
}
}