Issue #2146733 by andypost, chx, InternetDevels, bohart: Select queries should not use rowCount() to calculate number of rows.

8.0.x
Alex Pott 2013-12-27 11:23:31 +00:00
parent 12f895ec39
commit 588e6783d2
12 changed files with 110 additions and 14 deletions

View File

@ -544,6 +544,7 @@ abstract class Connection implements \Serializable {
case Database::RETURN_STATEMENT:
return $stmt;
case Database::RETURN_AFFECTED:
$stmt->allowRowCount = TRUE;
return $stmt->rowCount();
case Database::RETURN_INSERT_ID:
return $this->connection->lastInsertId();

View File

@ -130,6 +130,7 @@ class Connection extends DatabaseConnection {
switch ($options['return']) {
case Database::RETURN_STATEMENT:
$stmt->allowRowCount = FALSE;
return $stmt;
case Database::RETURN_AFFECTED:
return $stmt->rowCount();

View File

@ -0,0 +1,13 @@
<?php
/**
* @file
* Contains \Drupal\Core\Database\RowCountException
*/
namespace Drupal\Core\Database;
/**
* Exception thrown if a SELECT query trying to execute rowCount() on result.
*/
class RowCountException extends \RuntimeException implements DatabaseException { }

View File

@ -29,6 +29,13 @@ class Statement extends \PDOStatement implements StatementInterface {
*/
public $dbh;
/**
* Is rowCount() execution allowed.
*
* @var bool
*/
public $allowRowCount = FALSE;
protected function __construct(Connection $dbh) {
$this->dbh = $dbh;
$this->setFetchMode(\PDO::FETCH_OBJ);
@ -106,4 +113,18 @@ class Statement extends \PDOStatement implements StatementInterface {
// Call \PDOStatement::fetch to fetch the row.
return $this->fetch(\PDO::FETCH_ASSOC);
}
/**
* {@inheritdoc}
*/
public function rowCount() {
// SELECT query should not use the method.
if ($this->allowRowCount) {
return parent::rowCount();
}
else {
throw new RowCountException();
}
}
}

View File

@ -21,6 +21,13 @@ namespace Drupal\Core\Database;
*/
class StatementEmpty implements \Iterator, StatementInterface {
/**
* Is rowCount() execution allowed.
*
* @var bool
*/
public $allowRowCount = FALSE;
public function execute($args = array(), $options = array()) {
return FALSE;
}
@ -30,7 +37,10 @@ class StatementEmpty implements \Iterator, StatementInterface {
}
public function rowCount() {
return 0;
if ($this->allowRowCount) {
return 0;
}
throw new RowCountException();
}
public function setFetchMode($mode, $a1 = NULL, $a2 = array()) {

View File

@ -74,7 +74,10 @@ interface StatementInterface extends \Traversable {
*
* @return
* The number of rows affected by the last DELETE, INSERT, or UPDATE
* statement executed.
* statement executed or throws \Drupal\Core\Database\RowCountException
* if the last executed statement was SELECT.
*
* @throws \Drupal\Core\Database\RowCountException
*/
public function rowCount();

View File

@ -128,6 +128,13 @@ class StatementPrefetch implements \Iterator, StatementInterface {
'column' => 0,
);
/**
* Is rowCount() execution allowed.
*
* @var bool
*/
public $allowRowCount = FALSE;
public function __construct(\PDO $dbh, Connection $connection, $query, array $driver_options = array()) {
$this->dbh = $dbh;
$this->connection = $connection;
@ -174,9 +181,11 @@ class StatementPrefetch implements \Iterator, StatementInterface {
$this->throwPDOException();
}
if ($options['return'] == Database::RETURN_AFFECTED) {
$this->rowCount = $statement->rowCount();
}
// Fetch all the data from the reply, in order to release any lock
// as soon as possible.
$this->rowCount = $statement->rowCount();
$this->data = $statement->fetchAll(\PDO::FETCH_ASSOC);
// Destroy the statement as soon as possible. See
// DatabaseConnection_sqlite::PDOPrepare() for explanation.

View File

@ -150,7 +150,7 @@ class LocaleImportFunctionalTest extends WebTestBase {
// The database should now contain 6 customized strings (two imported
// strings are not translated).
$count = db_query('SELECT lid FROM {locales_target} WHERE customized = :custom', array(':custom' => 1))->rowCount();
$count = db_query('SELECT COUNT(*) FROM {locales_target} WHERE customized = :custom', array(':custom' => 1))->fetchField();
$this->assertEqual($count, 6, 'Customized translations successfully imported.');
// Try importing a .po file with overriding strings, and ensure existing

View File

@ -54,17 +54,17 @@ function locale_translation_get_projects($project_names = array()) {
if (empty($projects)) {
// Get project data from the database.
$result = db_query('SELECT name, project_type, core, version, server_pattern, status FROM {locale_project}');
$row_count = db_query_range('SELECT 1 FROM {locale_project}', 0, 1)->fetchField();
// http://drupal.org/node/1777106 is a follow-up issue to make the check for
// possible out-of-date project information more robust.
if ($result->rowCount() == 0 && \Drupal::moduleHandler()->moduleExists('update')) {
if ($row_count == 0 && \Drupal::moduleHandler()->moduleExists('update')) {
module_load_include('compare.inc', 'locale');
// At least the core project should be in the database, so we build the
// data if none are found.
locale_translation_build_projects();
$result = db_query('SELECT name, project_type, core, version, server_pattern, status FROM {locale_project}');
}
$result = db_query('SELECT name, project_type, core, version, server_pattern, status FROM {locale_project}');
foreach ($result as $project) {
$projects[$project->name] = $project;

View File

@ -7,6 +7,7 @@
namespace Drupal\migrate\Tests;
use Drupal\Core\Database\RowCountException;
use Drupal\Core\Database\StatementInterface;
/**
@ -32,7 +33,7 @@ class FakeStatement extends \ArrayIterator implements StatementInterface {
* {@inheritdoc}
*/
public function rowCount() {
return $this->count();
throw new RowCountException();
}
/**

View File

@ -2,11 +2,12 @@
/**
* @file
* Definition of Drupal\system\Tests\Database\FetchTest.
* Contains \Drupal\system\Tests\Database\FetchTest.
*/
namespace Drupal\system\Tests\Database;
use Drupal\Core\Database\RowCountException;
use Drupal\Core\Database\StatementInterface;
/**
@ -136,4 +137,20 @@ class FetchTest extends DatabaseTestBase {
$this->assertIdentical($record->name, $column[$i++], 'Column matches direct accesss.');
}
}
/**
* Tests that rowCount() throws exception on SELECT query.
*/
public function testRowCount() {
$result = db_query('SELECT name FROM {test}');
try {
$result->rowCount();
$exception = FALSE;
}
catch (RowCountException $e) {
$exception = TRUE;
}
$this->assertTrue($exception, 'Exception was thrown');
}
}

View File

@ -2,11 +2,13 @@
/**
* @file
* Definition of Drupal\system\Tests\Database\SelectComplexTest.
* Contains \Drupal\system\Tests\Database\SelectComplexTest.
*/
namespace Drupal\system\Tests\Database;
use \Drupal\Core\Database\RowCountException;
/**
* Tests more complex select statements.
*/
@ -156,9 +158,9 @@ class SelectComplexTest extends DatabaseTestBase {
$query->addField('test', 'name');
$query->addField('test', 'age', 'age');
$query->range(0, 2);
$result = $query->execute();
$query_result = $query->countQuery()->execute()->fetchField();
$this->assertEqual($result->rowCount(), 2, 'Returned the correct number of rows.');
$this->assertEqual($query_result, 2, 'Returned the correct number of rows.');
}
/**
@ -168,9 +170,9 @@ class SelectComplexTest extends DatabaseTestBase {
$query = db_select('test_task');
$query->addField('test_task', 'task');
$query->distinct();
$result = $query->execute();
$query_result = $query->countQuery()->execute()->fetchField();
$this->assertEqual($result->rowCount(), 6, 'Returned the correct number of rows.');
$this->assertEqual($query_result, 6, 'Returned the correct number of rows.');
}
/**
@ -363,4 +365,22 @@ class SelectComplexTest extends DatabaseTestBase {
$pos2 = strpos($str, 'db_condition_placeholder_0', $pos + 1);
$this->assertFalse($pos2, 'Condition placeholder is not repeated.');
}
/**
* Tests that rowCount() throws exception on SELECT query.
*/
function testSelectWithRowCount() {
$query = db_select('test');
$query->addField('test', 'name');
$result = $query->execute();
try {
$result->rowCount();
$exception = FALSE;
}
catch (RowCountException $e) {
$exception = TRUE;
}
$this->assertTrue($exception, 'Exception was thrown');
}
}