Merge "rdbms: clean up non-native Database::replace() code"

diff --git a/includes/changetags/ChangeTags.php b/includes/changetags/ChangeTags.php
index 95848ea..7e4dd00 100644 (file)
--- a/includes/changetags/ChangeTags.php
+++ b/includes/changetags/ChangeTags.php
@@ -408,19 +408,24 @@ class ChangeTags {
                sort( $prevTags );
                sort( $newTags );
                if ( $prevTags == $newTags ) {
-                       // No change.
                        return false;
                }
 
                if ( !$newTags ) {
-                       // no tags left, so delete the row altogether
+                       // No tags left, so delete the row altogether
                        $dbw->delete( 'tag_summary', $tsConds, __METHOD__ );
                } else {
-                       $dbw->replace( 'tag_summary',
-                               [ 'ts_rev_id', 'ts_rc_id', 'ts_log_id' ],
-                               array_filter( array_merge( $tsConds, [ 'ts_tags' => implode( ',', $newTags ) ] ) ),
-                               __METHOD__
-                       );
+                       // Specify the non-DEFAULT value columns in the INSERT/REPLACE clause
+                       $row = array_filter( [ 'ts_tags' => implode( ',', $newTags ) ] + $tsConds );
+                       // Check the unique keys for conflicts, ignoring any NULL *_id values
+                       $uniqueKeys = [];
+                       foreach ( [ 'ts_rev_id', 'ts_rc_id', 'ts_log_id' ] as $uniqueColumn ) {
+                               if ( isset( $row[$uniqueColumn] ) ) {
+                                       $uniqueKeys[] = [ $uniqueColumn ];
+                               }
+                       }
+
+                       $dbw->replace( 'tag_summary', $uniqueKeys, $row, __METHOD__ );
                }
 
                return true;

diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index 1efa9a1..6d1cff7 100644 (file)
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -2238,49 +2238,43 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        public function replace( $table, $uniqueIndexes, $rows, $fname = __METHOD__ ) {
-               $quotedTable = $this->tableName( $table );
-
                if ( count( $rows ) == 0 ) {
                        return;
                }
 
-               # Single row case
+               // Single row case
                if ( !is_array( reset( $rows ) ) ) {
                        $rows = [ $rows ];
                }
 
-               // @FXIME: this is not atomic, but a trx would break affectedRows()
+               // @FIXME: this is not atomic, but a trx would break affectedRows()
                foreach ( $rows as $row ) {
-                       # Delete rows which collide
-                       if ( $uniqueIndexes ) {
-                               $sql = "DELETE FROM $quotedTable WHERE ";
-                               $first = true;
-                               foreach ( $uniqueIndexes as $index ) {
-                                       if ( $first ) {
-                                               $first = false;
-                                               $sql .= '( ';
-                                       } else {
-                                               $sql .= ' ) OR ( ';
-                                       }
-                                       if ( is_array( $index ) ) {
-                                               $first2 = true;
-                                               foreach ( $index as $col ) {
-                                                       if ( $first2 ) {
-                                                               $first2 = false;
-                                                       } else {
-                                                               $sql .= ' AND ';
-                                                       }
-                                                       $sql .= $col . '=' . $this->addQuotes( $row[$col] );
-                                               }
-                                       } else {
-                                               $sql .= $index . '=' . $this->addQuotes( $row[$index] );
-                                       }
+                       // 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 (' .
+                                                       implode( ', ', $indexColumns ) . ')'
+                                       );
                                }
-                               $sql .= ' )';
-                               $this->query( $sql, $fname );
+                               $indexWhereClauses[] = $this->makeList( $indexRowValues, LIST_AND );
+                       }
+
+                       if ( $indexWhereClauses ) {
+                               $this->delete( $table, $this->makeList( $indexWhereClauses, LIST_OR ), $fname );
                        }
 
-                       # Now insert the row
+                       // Now insert the row
                        $this->insert( $table, $row, $fname );
                }
        }

diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php
index 23f7865..d8ebeff 100644 (file)
--- a/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php
+++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php
@@ -560,7 +560,7 @@ class DatabaseSQLTest extends PHPUnit_Framework_TestCase {
                                        'rows' => [ 'field' => 'text', 'field2' => 'text2' ],
                                ],
                                "DELETE FROM replace_table " .
-                                       "WHERE ( field='text' ); " .
+                                       "WHERE (field = 'text'); " .
                                        "INSERT INTO replace_table " .
                                        "(field,field2) " .
                                        "VALUES ('text','text2')"
@@ -576,7 +576,7 @@ class DatabaseSQLTest extends PHPUnit_Framework_TestCase {
                                        ],
                                ],
                                "DELETE FROM module_deps " .
-                                       "WHERE ( md_module='module' AND md_skin='skin' ); " .
+                                       "WHERE (md_module = 'module' AND md_skin = 'skin'); " .
                                        "INSERT INTO module_deps " .
                                        "(md_module,md_skin,md_deps) " .
                                        "VALUES ('module','skin','deps')"
@@ -598,12 +598,12 @@ class DatabaseSQLTest extends PHPUnit_Framework_TestCase {
                                        ],
                                ],
                                "DELETE FROM module_deps " .
-                                       "WHERE ( md_module='module' AND md_skin='skin' ); " .
+                                       "WHERE (md_module = 'module' AND md_skin = 'skin'); " .
                                        "INSERT INTO module_deps " .
                                        "(md_module,md_skin,md_deps) " .
                                        "VALUES ('module','skin','deps'); " .
                                        "DELETE FROM module_deps " .
-                                       "WHERE ( md_module='module2' AND md_skin='skin2' ); " .
+                                       "WHERE (md_module = 'module2' AND md_skin = 'skin2'); " .
                                        "INSERT INTO module_deps " .
                                        "(md_module,md_skin,md_deps) " .
                                        "VALUES ('module2','skin2','deps2')"
@@ -625,12 +625,12 @@ class DatabaseSQLTest extends PHPUnit_Framework_TestCase {
                                        ],
                                ],
                                "DELETE FROM module_deps " .
-                                       "WHERE ( md_module='module' ) OR ( md_skin='skin' ); " .
+                                       "WHERE (md_module = 'module') OR (md_skin = 'skin'); " .
                                        "INSERT INTO module_deps " .
                                        "(md_module,md_skin,md_deps) " .
                                        "VALUES ('module','skin','deps'); " .
                                        "DELETE FROM module_deps " .
-                                       "WHERE ( md_module='module2' ) OR ( md_skin='skin2' ); " .
+                                       "WHERE (md_module = 'module2') OR (md_skin = 'skin2'); " .
                                        "INSERT INTO module_deps " .
                                        "(md_module,md_skin,md_deps) " .
                                        "VALUES ('module2','skin2','deps2')"