rdbms: make sure non-native replace() uses one transaction

This is similar to what upsert() already does

Change-Id: Ide83eefe0d937fb2cdc20aa3c7dc9654c4d34beb

diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index 9a8996c..572a798 100644 (file)
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -2244,37 +2244,51 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                        $rows = [ $rows ];
                }
 
-               $affectedRowCount = 0;
-               foreach ( $rows as $row ) {
-                       // Delete rows which collide with this one
-                       $indexWhereClauses = [];
-                       foreach ( $uniqueIndexes as $index ) {
-                               $indexColumns = (array)$index;
-                               $indexRowValues = array_intersect_key( $row, array_flip( $indexColumns ) );
-                               if ( count( $indexRowValues ) != count( $indexColumns ) ) {
-                                       throw new DBUnexpectedError(
-                                               $this,
-                                               'New record does not provide all values for unique key (' .
+               $useTrx = !$this->trxLevel;
+               if ( $useTrx ) {
+                       $this->begin( $fname, self::TRANSACTION_INTERNAL );
+               }
+               try {
+                       $affectedRowCount = 0;
+                       foreach ( $rows as $row ) {
+                               // Delete rows which collide with this one
+                               $indexWhereClauses = [];
+                               foreach ( $uniqueIndexes as $index ) {
+                                       $indexColumns = (array)$index;
+                                       $indexRowValues = array_intersect_key( $row, array_flip( $indexColumns ) );
+                                       if ( count( $indexRowValues ) != count( $indexColumns ) ) {
+                                               throw new DBUnexpectedError(
+                                                       $this,
+                                                       'New record does not provide all values for unique key (' .
                                                        implode( ', ', $indexColumns ) . ')'
-                                       );
-                               } elseif ( in_array( null, $indexRowValues, true ) ) {
-                                       throw new DBUnexpectedError(
-                                               $this,
-                                               'New record has a null value for unique key (' .
+                                               );
+                                       } elseif ( in_array( null, $indexRowValues, true ) ) {
+                                               throw new DBUnexpectedError(
+                                                       $this,
+                                                       'New record has a null value for unique key (' .
                                                        implode( ', ', $indexColumns ) . ')'
-                                       );
+                                               );
+                                       }
+                                       $indexWhereClauses[] = $this->makeList( $indexRowValues, LIST_AND );
                                }
-                               $indexWhereClauses[] = $this->makeList( $indexRowValues, LIST_AND );
-                       }
 
-                       if ( $indexWhereClauses ) {
-                               $this->delete( $table, $this->makeList( $indexWhereClauses, LIST_OR ), $fname );
+                               if ( $indexWhereClauses ) {
+                                       $this->delete( $table, $this->makeList( $indexWhereClauses, LIST_OR ), $fname );
+                                       $affectedRowCount += $this->affectedRows();
+                               }
+
+                               // Now insert the row
+                               $this->insert( $table, $row, $fname );
                                $affectedRowCount += $this->affectedRows();
                        }
-
-                       // Now insert the row
-                       $this->insert( $table, $row, $fname );
-                       $affectedRowCount += $this->affectedRows();
+               } catch ( Exception $e ) {
+                       if ( $useTrx ) {
+                               $this->rollback( $fname, self::FLUSHING_INTERNAL );
+                       }
+                       throw $e;
+               }
+               if ( $useTrx ) {
+                       $this->commit( $fname, self::FLUSHING_INTERNAL );
                }
 
                $this->affectedRowCount = $affectedRowCount;

diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php
index 184d626..ebf6e45 100644 (file)
--- a/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php
+++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php
@@ -559,11 +559,11 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
                                        'uniqueIndexes' => [ 'field' ],
                                        'rows' => [ 'field' => 'text', 'field2' => 'text2' ],
                                ],
-                               "DELETE FROM replace_table " .
+                               "BEGIN; DELETE FROM replace_table " .
                                        "WHERE (field = 'text'); " .
                                        "INSERT INTO replace_table " .
                                        "(field,field2) " .
-                                       "VALUES ('text','text2')"
+                                       "VALUES ('text','text2'); COMMIT"
                        ],
                        [
                                [
@@ -575,11 +575,11 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
                                                'md_deps' => 'deps',
                                        ],
                                ],
-                               "DELETE FROM module_deps " .
+                               "BEGIN; DELETE FROM module_deps " .
                                        "WHERE (md_module = 'module' AND md_skin = 'skin'); " .
                                        "INSERT INTO module_deps " .
                                        "(md_module,md_skin,md_deps) " .
-                                       "VALUES ('module','skin','deps')"
+                                       "VALUES ('module','skin','deps'); COMMIT"
                        ],
                        [
                                [
@@ -597,7 +597,7 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
                                                ],
                                        ],
                                ],
-                               "DELETE FROM module_deps " .
+                               "BEGIN; DELETE FROM module_deps " .
                                        "WHERE (md_module = 'module' AND md_skin = 'skin'); " .
                                        "INSERT INTO module_deps " .
                                        "(md_module,md_skin,md_deps) " .
@@ -606,7 +606,7 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
                                        "WHERE (md_module = 'module2' AND md_skin = 'skin2'); " .
                                        "INSERT INTO module_deps " .
                                        "(md_module,md_skin,md_deps) " .
-                                       "VALUES ('module2','skin2','deps2')"
+                                       "VALUES ('module2','skin2','deps2'); COMMIT"
                        ],
                        [
                                [
@@ -624,7 +624,7 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
                                                ],
                                        ],
                                ],
-                               "DELETE FROM module_deps " .
+                               "BEGIN; DELETE FROM module_deps " .
                                        "WHERE (md_module = 'module') OR (md_skin = 'skin'); " .
                                        "INSERT INTO module_deps " .
                                        "(md_module,md_skin,md_deps) " .
@@ -633,7 +633,7 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
                                        "WHERE (md_module = 'module2') OR (md_skin = 'skin2'); " .
                                        "INSERT INTO module_deps " .
                                        "(md_module,md_skin,md_deps) " .
-                                       "VALUES ('module2','skin2','deps2')"
+                                       "VALUES ('module2','skin2','deps2'); COMMIT"
                        ],
                        [
                                [
@@ -645,9 +645,9 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
                                                'md_deps' => 'deps',
                                        ],
                                ],
-                               "INSERT INTO module_deps " .
+                               "BEGIN; INSERT INTO module_deps " .
                                        "(md_module,md_skin,md_deps) " .
-                                       "VALUES ('module','skin','deps')"
+                                       "VALUES ('module','skin','deps'); COMMIT"
                        ],
                ];
        }