Commit 0d100b57 authored by Dries's avatar Dries

- Patch #332002 by Crell et al: MergeQuery should refuse to execute if there...

- Patch #332002 by Crell et al: MergeQuery should refuse to execute if there are no key fields.  With tests.
parent fc063806
......@@ -1169,6 +1169,14 @@ public static function ignoreTarget($key, $target) {
*/
class TransactionsNotSupportedException extends PDOException { }
/**
* Exception thrown for merge queries that do not make semantic sense.
*
* There are many ways that a merge query could be malformed. They should all
* throw this exception and set an appropriately descriptive message.
*/
class InvalidMergeQueryException extends Exception {}
/**
* A wrapper class for creating and managing database transactions.
*
......
......@@ -80,6 +80,12 @@ public function __toString() {
class MergeQuery_mysql extends MergeQuery {
public function execute() {
// A merge query without any key field is invalid.
if (count($this->keyFields) == 0) {
throw new InvalidMergeQueryException("You need to specify key fields before executing a merge query");
}
// Set defaults.
if ($this->updateFields) {
$update_fields = $this->updateFields;
......
......@@ -635,6 +635,11 @@ public function expression($field, $expression, array $arguments = NULL) {
public function execute() {
// A merge query without any key field is invalid.
if (count($this->keyFields) == 0) {
throw new InvalidMergeQueryException("You need to specify key fields before executing a merge query");
}
// In the degenerate case of this query type, we have to run multiple
// queries as there is no universal single-query mechanism that will work.
// Our degenerate case is not designed for performance efficiency but
......
......@@ -957,6 +957,26 @@ class DatabaseMergeTestCase extends DatabaseTestCase {
$this->assertEqual($person->age, $age_before + 4, t('Age updated correctly.'));
$this->assertEqual($person->job, 'Speaker', t('Job set correctly.'));
}
/**
* Test that an invalid merge query throws an exception like it is supposed to.
*/
function testInvalidMerge() {
try {
// This query should die because there is no key field specified.
db_merge('test_people')
->fields(array(
'age' => 31,
'name' => 'Tiffany',
))
->execute();
}
catch (InvalidMergeQueryException $e) {
$this->pass(t('InvalidMergeQueryException thrown for invalid query.'));
return;
}
$this->fail(t('No InvalidMergeQueryException thrown'));
}
}
/**
......
......@@ -51,14 +51,13 @@ function statistics_exit() {
// We are counting content views.
if ((arg(0) == 'node') && is_numeric(arg(1)) && arg(2) == '') {
// A node has been viewed, so update the node's counters.
$fields = array(
'daycount' => 1,
'totalcount' => 1,
'nid' => arg(1),
'timestamp' => REQUEST_TIME,
);
db_merge('node_counter')
->fields($fields)
->key(array('nid' => arg(1)))
->fields(array(
'daycount' => 1,
'totalcount' => 1,
'timestamp' => REQUEST_TIME,
))
->expression('daycount', 'daycount + 1')
->expression('totalcount', 'totalcount + 1')
->execute();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment