Merge "rdbms: Remove support for PostgreSQL < 9.2, and improve INSERT IGNORE for...
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Thu, 5 Apr 2018 21:33:26 +0000 (21:33 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Thu, 5 Apr 2018 21:33:26 +0000 (21:33 +0000)
includes/libs/rdbms/database/Database.php
tests/phpunit/includes/db/DatabaseTestHelper.php
tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php

index 51d5466..2ef8822 100644 (file)
@@ -1150,7 +1150,9 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                                                $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;
                                }
                        }
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
         */