diff options
| -rw-r--r-- | includes/database/query.inc | 104 | ||||
| -rw-r--r-- | includes/database/schema.inc | 23 | ||||
| -rw-r--r-- | includes/database/select.inc | 142 | ||||
| -rw-r--r-- | modules/simpletest/tests/database_test.test | 34 |
4 files changed, 234 insertions, 69 deletions
diff --git a/includes/database/query.inc b/includes/database/query.inc index 23b652f9b..c7363f238 100644 --- a/includes/database/query.inc +++ b/includes/database/query.inc @@ -142,7 +142,15 @@ interface QueryConditionInterface { * 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(); } @@ -239,12 +247,17 @@ interface QueryAlterableInterface { 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(); } /** @@ -284,6 +297,11 @@ abstract class Query implements QueryPlaceholderInterface { protected $queryOptions; /** + * A unique identifier for this query object. + */ + protected $uniqueIdentifier; + + /** * The placeholder counter. */ protected $nextPlaceholder = 0; @@ -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,13 +341,20 @@ abstract class Query implements QueryPlaceholderInterface { } /** - * 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. */ abstract protected function execute(); @@ -344,6 +371,13 @@ abstract class Query implements QueryPlaceholderInterface { 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. * * @return int @@ -790,8 +824,15 @@ class DeleteQuery extends Query implements QueryConditionInterface { /** * 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 @@ class TruncateQuery extends Query { /** * 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 @@ class UpdateQuery extends Query implements QueryConditionInterface { /** * 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 @@ class MergeQuery extends Query implements QueryConditionInterface { /** * 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(); } /** @@ -1609,6 +1671,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. * * @param string $conjunction @@ -1718,8 +1785,12 @@ class DatabaseCondition implements QueryConditionInterface, Countable { /** * 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(); @@ -1792,6 +1863,13 @@ class DatabaseCondition implements QueryConditionInterface, Countable { } /** + * Implements QueryConditionInterface::compiled(). + */ + public function compiled() { + return !$this->changed; + } + + /** * Implements PHP magic __toString method to convert the conditions to string. * * @return string diff --git a/includes/database/schema.inc b/includes/database/schema.inc index de1b2f5b9..e2a1c4caa 100644 --- a/includes/database/schema.inc +++ b/includes/database/schema.inc @@ -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++; } diff --git a/includes/database/select.inc b/includes/database/select.inc index 53be20adc..9b587aebe 100644 --- a/includes/database/select.inc +++ b/includes/database/select.inc @@ -549,17 +549,31 @@ 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 @@ class SelectQueryExtender implements SelectQueryInterface { 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 @@ class SelectQueryExtender implements SelectQueryInterface { } 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 @@ class SelectQuery extends Query implements SelectQueryInterface { } 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 @@ class SelectQuery extends Query implements SelectQueryInterface { 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 @@ class SelectQuery extends Query implements SelectQueryInterface { 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 @@ class SelectQuery extends Query implements SelectQueryInterface { } 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 @@ class SelectQuery extends Query implements SelectQueryInterface { // 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 @@ class SelectQuery extends Query implements SelectQueryInterface { // HAVING if (count($this->having)) { - $this->having->compile($this->connection, $this); // There is an implicit string cast on $this->having. $query .= "\nHAVING " . $this->having; } diff --git a/modules/simpletest/tests/database_test.test b/modules/simpletest/tests/database_test.test index e27693c97..76ca1038e 100644 --- a/modules/simpletest/tests/database_test.test +++ b/modules/simpletest/tests/database_test.test @@ -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.')); + } } /** |