Commit e8164832 authored by webchick's avatar webchick

Issue #1112854 by dereine, chx, Damien Tournoud: Fixed Subqueries use wrong arguments.

parent fa456923
......@@ -142,7 +142,15 @@ public function arguments();
* The query this condition belongs to. If not given, the current query is
* used.
*/
public function compile(DatabaseConnection $connection, QueryPlaceholderInterface $queryPlaceholder = NULL);
public function compile(DatabaseConnection $connection, QueryPlaceholderInterface $queryPlaceholder);
/**
* Check whether a condition has been previously compiled.
*
* @return
* TRUE if the condition has been previously compiled.
*/
public function compiled();
}
......@@ -238,13 +246,18 @@ public function getMetaData($key);
*/
interface QueryPlaceholderInterface {
/**
* Returns a unique identifier for this object.
*/
public function uniqueIdentifier();
/**
* Returns the next placeholder ID for the query.
*
* @return
* The next available placeholder ID as an integer.
*/
function nextPlaceholder();
public function nextPlaceholder();
}
/**
......@@ -283,6 +296,11 @@ abstract class Query implements QueryPlaceholderInterface {
*/
protected $queryOptions;
/**
* A unique identifier for this query object.
*/
protected $uniqueIdentifier;
/**
* The placeholder counter.
*/
......@@ -304,6 +322,8 @@ abstract class Query implements QueryPlaceholderInterface {
* Array of query options.
*/
public function __construct(DatabaseConnection $connection, $options) {
$this->uniqueIdentifier = uniqid('', TRUE);
$this->connection = $connection;
$this->connectionKey = $this->connection->getKey();
$this->connectionTarget = $this->connection->getTarget();
......@@ -321,12 +341,19 @@ public function __sleep() {
}
/**
* Implements the magic __wakeup function to reconnect to the database.
* Implements the magic __wakeup function to reconnect to the database.
*/
public function __wakeup() {
$this->connection = Database::getConnection($this->connectionTarget, $this->connectionKey);
}
/**
* Implements the magic __clone function.
*/
public function __clone() {
$this->uniqueIdentifier = uniqid('', TRUE);
}
/**
* Runs the query against the database.
*/
......@@ -343,6 +370,13 @@ public function __wakeup() {
*/
abstract public function __toString();
/**
* Returns a unique identifier for this object.
*/
public function uniqueIdentifier() {
return $this->uniqueIdentifier;
}
/**
* Gets the next placeholder value for this query object.
*
......@@ -790,8 +824,15 @@ public function where($snippet, $args = array()) {
/**
* Implements QueryConditionInterface::compile().
*/
public function compile(DatabaseConnection $connection, QueryPlaceholderInterface $queryPlaceholder = NULL) {
return $this->condition->compile($connection, isset($queryPlaceholder) ? $queryPlaceholder : $this);
public function compile(DatabaseConnection $connection, QueryPlaceholderInterface $queryPlaceholder) {
return $this->condition->compile($connection, $queryPlaceholder);
}
/**
* Implements QueryConditionInterface::compiled().
*/
public function compiled() {
return $this->condition->compiled();
}
/**
......@@ -864,8 +905,15 @@ public function __construct(DatabaseConnection $connection, $table, array $optio
/**
* Implements QueryConditionInterface::compile().
*/
public function compile(DatabaseConnection $connection, QueryPlaceholderInterface $queryPlaceholder = NULL) {
return $this->condition->compile($connection, isset($queryPlaceholder) ? $queryPlaceholder : $this);
public function compile(DatabaseConnection $connection, QueryPlaceholderInterface $queryPlaceholder) {
return $this->condition->compile($connection, $queryPlaceholder);
}
/**
* Implements QueryConditionInterface::compiled().
*/
public function compiled() {
return $this->condition->compiled();
}
/**
......@@ -1025,8 +1073,15 @@ public function where($snippet, $args = array()) {
/**
* Implements QueryConditionInterface::compile().
*/
public function compile(DatabaseConnection $connection, QueryPlaceholderInterface $queryPlaceholder = NULL) {
return $this->condition->compile($connection, isset($queryPlaceholder) ? $queryPlaceholder : $this);
public function compile(DatabaseConnection $connection, QueryPlaceholderInterface $queryPlaceholder) {
return $this->condition->compile($connection, $queryPlaceholder);
}
/**
* Implements QueryConditionInterface::compiled().
*/
public function compiled() {
return $this->condition->compiled();
}
/**
......@@ -1510,8 +1565,15 @@ public function where($snippet, $args = array()) {
/**
* Implements QueryConditionInterface::compile().
*/
public function compile(DatabaseConnection $connection, QueryPlaceholderInterface $queryPlaceholder = NULL) {
return $this->condition->compile($connection, isset($queryPlaceholder) ? $queryPlaceholder : $this);
public function compile(DatabaseConnection $connection, QueryPlaceholderInterface $queryPlaceholder) {
return $this->condition->compile($connection, $queryPlaceholder);
}
/**
* Implements QueryConditionInterface::compiled().
*/
public function compiled() {
return $this->condition->compiled();
}
/**
......@@ -1608,6 +1670,11 @@ class DatabaseCondition implements QueryConditionInterface, Countable {
*/
protected $changed = TRUE;
/**
* The identifier of the query placeholder this condition has been compiled against.
*/
protected $queryPlaceholderIdentifier;
/**
* Constructs a DataBaseCondition object.
*
......@@ -1718,8 +1785,12 @@ public function arguments() {
/**
* Implements QueryConditionInterface::compile().
*/
public function compile(DatabaseConnection $connection, QueryPlaceholderInterface $queryPlaceholder = NULL) {
if ($this->changed) {
public function compile(DatabaseConnection $connection, QueryPlaceholderInterface $queryPlaceholder) {
// Re-compile if this condition changed or if we are compiled against a
// different query placeholder object.
if ($this->changed || isset($this->queryPlaceholderIdentifier) && ($this->queryPlaceholderIdentifier != $queryPlaceholder->uniqueIdentifier())) {
$this->queryPlaceholderIdentifier = $queryPlaceholder->uniqueIdentifier();
$condition_fragments = array();
$arguments = array();
......@@ -1791,6 +1862,13 @@ public function compile(DatabaseConnection $connection, QueryPlaceholderInterfac
}
}
/**
* Implements QueryConditionInterface::compiled().
*/
public function compiled() {
return !$this->changed;
}
/**
* Implements PHP magic __toString method to convert the conditions to string.
*
......
......@@ -176,10 +176,33 @@ abstract class DatabaseSchema implements QueryPlaceholderInterface {
*/
protected $defaultSchema = 'public';
/**
* A unique identifier for this query object.
*/
protected $uniqueIdentifier;
public function __construct($connection) {
$this->uniqueIdentifier = uniqid('', TRUE);
$this->connection = $connection;
}
/**
* Implements the magic __clone function.
*/
public function __clone() {
$this->uniqueIdentifier = uniqid('', TRUE);
}
/**
* Implements QueryPlaceHolderInterface::uniqueIdentifier().
*/
public function uniqueIdentifier() {
return $this->uniqueIdentifier;
}
/**
* Implements QueryPlaceHolderInterface::nextPlaceholder().
*/
public function nextPlaceholder() {
return $this->placeholder++;
}
......
......@@ -548,18 +548,32 @@ class SelectQueryExtender implements SelectQueryInterface {
*/
protected $connection;
/**
* A unique identifier for this query object.
*/
protected $uniqueIdentifier;
/**
* The placeholder counter.
*/
protected $placeholder = 0;
public function __construct(SelectQueryInterface $query, DatabaseConnection $connection) {
$this->uniqueIdentifier = uniqid('', TRUE);
$this->query = $query;
$this->connection = $connection;
}
/* Implementations of QueryPlaceholderInterface. */
/**
* Implements QueryPlaceholderInterface::uniqueIdentifier().
*/
public function uniqueIdentifier() {
return $this->uniqueIdentifier;
}
/**
* Implements QueryPlaceholderInterface::nextPlaceholder().
*/
public function nextPlaceholder() {
return $this->placeholder++;
}
......@@ -612,8 +626,12 @@ public function where($snippet, $args = array()) {
return $this;
}
public function compile(DatabaseConnection $connection, QueryPlaceholderInterface $queryPlaceholder = NULL) {
return $this->condition->compile($connection, isset($queryPlaceholder) ? $queryPlaceholder : $this);
public function compile(DatabaseConnection $connection, QueryPlaceholderInterface $queryPlaceholder) {
return $this->query->compile($connection, $queryPlaceholder);
}
public function compiled() {
return $this->query->compiled();
}
/* Implementations of QueryConditionInterface for the HAVING clause. */
......@@ -824,6 +842,8 @@ public function __toString() {
}
public function __clone() {
$this->uniqueIdentifier = uniqid('', TRUE);
// We need to deep-clone the query we're wrapping, which in turn may
// deep-clone other objects. Exciting!
$this->query = clone($this->query);
......@@ -1013,7 +1033,35 @@ public function &conditions() {
}
public function arguments() {
return $this->where->arguments();
if (!$this->compiled()) {
return NULL;
}
$args = $this->where->arguments() + $this->having->arguments();
foreach ($this->tables as $table) {
if ($table['arguments']) {
$args += $table['arguments'];
}
// If this table is a subquery, grab its arguments recursively.
if ($table['table'] instanceof SelectQueryInterface) {
$args += $table['table']->arguments();
}
}
foreach ($this->expressions as $expression) {
if ($expression['arguments']) {
$args += $expression['arguments'];
}
}
// If there are any dependent queries to UNION,
// incorporate their arguments recursively.
foreach ($this->union as $union) {
$args += $union['query']->arguments();
}
return $args;
}
public function where($snippet, $args = array()) {
......@@ -1041,8 +1089,44 @@ public function notExists(SelectQueryInterface $select) {
return $this;
}
public function compile(DatabaseConnection $connection, QueryPlaceholderInterface $queryPlaceholder = NULL) {
return $this->where->compile($connection, isset($queryPlaceholder) ? $queryPlaceholder : $this);
public function compile(DatabaseConnection $connection, QueryPlaceholderInterface $queryPlaceholder) {
$this->where->compile($connection, $queryPlaceholder);
$this->having->compile($connection, $queryPlaceholder);
foreach ($this->tables as $table) {
// If this table is a subquery, compile it recursively.
if ($table['table'] instanceof SelectQueryInterface) {
$table['table']->compile($connection, $queryPlaceholder);
}
}
// If there are any dependent queries to UNION, compile it recursively.
foreach ($this->union as $union) {
$union['query']->compile($connection, $queryPlaceholder);
}
}
public function compiled() {
if (!$this->where->compiled() || !$this->having->compiled()) {
return FALSE;
}
foreach ($this->tables as $table) {
// If this table is a subquery, check its status recursively.
if ($table['table'] instanceof SelectQueryInterface) {
if (!$table['table']->compiled()) {
return FALSE;
}
}
}
foreach ($this->union as $union) {
if (!$union['query']->compiled()) {
return FALSE;
}
}
return TRUE;
}
/* Implementations of QueryConditionInterface for the HAVING clause. */
......@@ -1136,33 +1220,8 @@ public function getArguments(QueryPlaceholderInterface $queryPlaceholder = NULL)
if (!isset($queryPlaceholder)) {
$queryPlaceholder = $this;
}
$this->where->compile($this->connection, $queryPlaceholder);
$this->having->compile($this->connection, $queryPlaceholder);
$args = $this->where->arguments() + $this->having->arguments();
foreach ($this->tables as $table) {
if ($table['arguments']) {
$args += $table['arguments'];
}
// If this table is a subquery, grab its arguments recursively.
if ($table['table'] instanceof SelectQueryInterface) {
$args += $table['table']->getArguments($queryPlaceholder);
}
}
foreach ($this->expressions as $expression) {
if ($expression['arguments']) {
$args += $expression['arguments'];
}
}
// If there are any dependent queries to UNION,
// incorporate their arguments recursively.
foreach ($this->union as $union) {
$args += $union['query']->getArguments($queryPlaceholder);
}
return $args;
$this->compile($this->connection, $queryPlaceholder);
return $this->arguments();
}
/**
......@@ -1439,6 +1498,14 @@ public function countQuery() {
}
public function __toString() {
// For convenience, we compile the query ourselves if the caller forgot
// to do it. This allows constructs like "(string) $query" to work. When
// the query will be executed, it will be recompiled using the proper
// placeholder generator anyway.
if (!$this->compiled()) {
$this->compile($this->connection, $this);
}
// Create a sanitized comment string to prepend to the query.
$comments = $this->connection->makeComment($this->comments);
......@@ -1496,14 +1563,6 @@ public function __toString() {
// WHERE
if (count($this->where)) {
// The following line will not generate placeholders correctly if there
// is a subquery. Fortunately, it is also called from getArguments() first
// so it's not a problem in practice... unless you try to call __toString()
// before calling getArguments(). That is a problem that we will have to
// fix in Drupal 8, because it requires more refactoring than we are
// able to do in Drupal 7.
// @todo Move away from __toString() For SelectQuery compilation at least.
$this->where->compile($this->connection, $this);
// There is an implicit string cast on $this->condition.
$query .= "\nWHERE " . $this->where;
}
......@@ -1515,7 +1574,6 @@ public function __toString() {
// HAVING
if (count($this->having)) {
$this->having->compile($this->connection, $this);
// There is an implicit string cast on $this->having.
$query .= "\nHAVING " . $this->having;
}
......
......@@ -1629,22 +1629,28 @@ class DatabaseSelectSubqueryTestCase extends DatabaseTestCase {
$subquery->addField('tt', 'task', 'task');
$subquery->condition('priority', 1);
// Create another query that joins against the virtual table resulting
// from the subquery.
$select = db_select($subquery, 'tt2');
$select->join('test', 't', 't.id=tt2.pid');
$select->addField('t', 'name');
for ($i = 0; $i < 2; $i++) {
// Create another query that joins against the virtual table resulting
// from the subquery.
$select = db_select($subquery, 'tt2');
$select->join('test', 't', 't.id=tt2.pid');
$select->addField('t', 'name');
if ($i) {
// Use a different number of conditions here to confuse the subquery
// placeholder counter, testing http://drupal.org/node/1112854.
$select->condition('name', 'John');
}
$select->condition('task', 'code');
$select->condition('task', 'code');
// The resulting query should be equivalent to:
// SELECT t.name
// FROM (SELECT tt.pid AS pid, tt.task AS task FROM test_task tt WHERE priority=1) tt
// INNER JOIN test t ON t.id=tt.pid
// WHERE tt.task = 'code'
$people = $select->execute()->fetchCol();
// The resulting query should be equivalent to:
// SELECT t.name
// FROM (SELECT tt.pid AS pid, tt.task AS task FROM test_task tt WHERE priority=1) tt
// INNER JOIN test t ON t.id=tt.pid
// WHERE tt.task = 'code'
$people = $select->execute()->fetchCol();
$this->assertEqual(count($people), 1, t('Returned the correct number of rows.'));
$this->assertEqual(count($people), 1, t('Returned the correct number of rows.'));
}
}
/**
......
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