summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAlex Pott2013-06-08 17:39:36 (GMT)
committerAlex Pott2013-06-08 17:39:36 (GMT)
commitd790fb7adad1047dbdb55972aa1d2855402b98bb (patch)
treee1423e37bc3b709d9604d09c006e380d40b03520
parent1936dbc53de2440a0bcc35a6c64cab16327e2ffe (diff)
Issue #805858 by Pancho, Steven Jones, simg: Fixed Affected rows inconsistent across database engines.
-rw-r--r--core/lib/Drupal/Core/Database/Driver/mysql/Connection.php4
-rw-r--r--core/lib/Drupal/Core/Database/Driver/sqlite/Update.php49
-rw-r--r--core/lib/Drupal/Core/Database/Query/Update.php4
-rw-r--r--core/modules/system/lib/Drupal/system/Tests/Database/UpdateTest.php34
4 files changed, 30 insertions, 61 deletions
diff --git a/core/lib/Drupal/Core/Database/Driver/mysql/Connection.php b/core/lib/Drupal/Core/Database/Driver/mysql/Connection.php
index 9e0e65f..92142e8 100644
--- a/core/lib/Drupal/Core/Database/Driver/mysql/Connection.php
+++ b/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,
);
diff --git a/core/lib/Drupal/Core/Database/Driver/sqlite/Update.php b/core/lib/Drupal/Core/Database/Driver/sqlite/Update.php
index b6c2bdf..fa7d54b 100644
--- a/core/lib/Drupal/Core/Database/Driver/sqlite/Update.php
+++ b/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 { }
diff --git a/core/lib/Drupal/Core/Database/Query/Update.php b/core/lib/Drupal/Core/Database/Query/Update.php
index 326ec29..abbbc2b 100644
--- a/core/lib/Drupal/Core/Database/Query/Update.php
+++ b/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;
diff --git a/core/modules/system/lib/Drupal/system/Tests/Database/UpdateTest.php b/core/modules/system/lib/Drupal/system/Tests/Database/UpdateTest.php
index a4a45f6..fb96768 100644
--- a/core/modules/system/lib/Drupal/system/Tests/Database/UpdateTest.php
+++ b/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.');
}
/**