Issue #805858 by Pancho, Steven Jones, simg: Fixed Affected rows inconsistent across database engines.

core/lib/Drupal/Core/Database/Driver/mysql/Connection.php

@@ -74,6 +74,10 @@ class Connection extends DatabaseConnection {
       PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
       // So we don't have to mess around with cursors and unbuffered queries by default.
       PDO::MYSQL_ATTR_USE_BUFFERED_QUERY => TRUE,
+      // Make sure MySQL returns all matched rows on update queries including
+      // rows that actually didn't have to be updated because the values didn't
+      // change. This matches common behaviour among other database systems.
+      PDO::MYSQL_ATTR_FOUND_ROWS => TRUE,
       // Because MySQL's prepared statements skip the query cache, because it's dumb.
       PDO::ATTR_EMULATE_PREPARES => TRUE,
     );

core/lib/Drupal/Core/Database/Driver/sqlite/Update.php

@@ -7,53 +7,6 @@
 
 namespace Drupal\Core\Database\Driver\sqlite;
 
-use Drupal\Core\Database\Query\Condition;
-use Drupal\Core\Database\Query\ConditionInterface;
 use Drupal\Core\Database\Query\Update as QueryUpdate;
 
-/**
- * SQLite specific implementation of UpdateQuery.
- *
- * SQLite counts all the rows that match the conditions as modified, even if they
- * will not be affected by the query. We workaround this by ensuring that
- * we don't select those rows.
- *
- * A query like this one:
- *   UPDATE test SET col1 = 'newcol1', col2 = 'newcol2' WHERE tid = 1
- * will become:
- *   UPDATE test SET col1 = 'newcol1', col2 = 'newcol2' WHERE tid = 1 AND (col1 <> 'newcol1' OR col2 <> 'newcol2')
- */
-class Update extends QueryUpdate {
-  public function execute() {
-    if (!empty($this->queryOptions['sqlite_return_matched_rows'])) {
-      return parent::execute();
-    }
-
-    // Get the fields used in the update query.
-    $fields = $this->expressionFields + $this->fields;
-
-    // Add the inverse of the fields to the condition.
-    $condition = new Condition('OR');
-    foreach ($fields as $field => $data) {
-      if (is_array($data)) {
-        // The field is an expression.
-        $condition->where($field . ' <> ' . $data['expression']);
-        $condition->isNull($field);
-      }
-      elseif (!isset($data)) {
-        // The field will be set to NULL.
-        $condition->isNotNull($field);
-      }
-      else {
-        $condition->condition($field, $data, '<>');
-        $condition->isNull($field);
-      }
-    }
-    if (count($condition)) {
-      $condition->compile($this->connection, $this);
-      $this->condition->where((string) $condition, $condition->arguments());
-    }
-    return parent::execute();
-  }
-
-}
+class Update extends QueryUpdate { }

core/lib/Drupal/Core/Database/Query/Update.php

@@ -200,10 +200,10 @@ class Update extends Query implements ConditionInterface {
    * Executes the UPDATE query.
    *
    * @return
-   *   The number of rows affected by the update.
+   *   The number of rows matched by the update query. This includes rows that
+   *   actually didn't have to be updated because the values didn't change.
    */
   public function execute() {
-
     // Expressions take priority over literal fields, so we process those first
     // and remove any literal fields that conflict.
     $fields = $this->fields;

core/modules/system/lib/Drupal/system/Tests/Database/UpdateTest.php

@@ -110,20 +110,32 @@ class UpdateTest extends DatabaseTestBase {
    * Tests updating with expressions.
    */
   function testExpressionUpdate() {
-    // Set age = 1 for a single row for this test to work.
-    db_update('test')
-      ->condition('id', 1)
-      ->fields(array('age' => 1))
-      ->execute();
-
-    // Ensure that expressions are handled properly.  This should set every
-    // record's age to a square of itself, which will change only three of the
-    // four records in the table since 1*1 = 1. That means only three records
-    // are modified, so we should get back 3, not 4, from execute().
+    // Ensure that expressions are handled properly. This should set every
+    // record's age to a square of itself.
     $num_rows = db_update('test')
       ->expression('age', 'age * age')
       ->execute();
-    $this->assertIdentical($num_rows, 3, 'Number of affected rows are returned.');
+    $this->assertIdentical($num_rows, 4, 'Updated 4 records.');
+
+    $saved_name = db_query('SELECT name FROM {test} WHERE age = :age', array(':age' => pow(26, 2)))->fetchField();
+    $this->assertIdentical($saved_name, 'Paul', t('Successfully updated values using an algebraic expression.'));
+  }
+
+  /**
+   * Tests return value on update.
+   */
+  function testUpdateAffectedRows() {
+    // At 5am in the morning, all band members but those with a priority 1 task
+    // are sleeping. So we set their tasks to 'sleep'. 5 records match the
+    // condition and therefore are affected by the query, even though two of
+    // them actually don't have to be changed because their value was already
+    // 'sleep'. Still, execute() should return 5 affected rows, not only 3,
+    // because that's cross-db expected behaviour.
+    $num_rows = db_update('test_task')
+      ->condition('priority', 1, '<>')
+      ->fields(array('task' => 'sleep'))
+      ->execute();
+    $this->assertIdentical($num_rows, 5, 'Correctly returned 5 affected rows.');
   }
 
   /**