diff --git a/core/lib/Drupal/Core/Database/Query/Condition.php b/core/lib/Drupal/Core/Database/Query/Condition.php index cb39dfd..ec541c1 100644 --- a/core/lib/Drupal/Core/Database/Query/Condition.php +++ b/core/lib/Drupal/Core/Database/Query/Condition.php @@ -40,6 +40,11 @@ class Condition implements ConditionInterface, \Countable { protected $queryPlaceholderIdentifier; /** + * @var string Contains the string version of the Condition. + */ + protected $stringVersion; + + /** * Constructs a Condition object. * * @param string $conjunction @@ -158,86 +163,124 @@ public function compile(Connection $connection, PlaceholderInterface $queryPlace $conjunction = $conditions['#conjunction']; unset($conditions['#conjunction']); foreach ($conditions as $condition) { - if (empty($condition['operator'])) { - // This condition is a literal string, so let it through as is. - $condition_fragments[] = ' (' . $condition['field'] . ') '; + // Process field. + if ($condition['field'] instanceof ConditionInterface) { + // Left hand part is a structured condition or a subquery: compile, + // put brackets around it (if it is a query), and collect any + // arguments. + $condition['field']->compile($connection, $queryPlaceholder); + $field_fragment = (string) $condition['field']; + if ($condition['field'] instanceof SelectInterface) { + $field_fragment = '(' . $field_fragment . ')'; + } + $arguments += $condition['field']->arguments(); + } + else if (!isset($condition['operator'])) { + // Left hand part is a literal string added with the + // @see ConditionInterface::where() method. Put brackets around + // the snippet and collect the arguments from the value part. + $field_fragment = '(' . $condition['field'] . ')'; $arguments += $condition['value']; } else { - // It's a structured condition, so parse it out accordingly. - // Note that $condition['field'] will only be an object for a dependent - // DatabaseCondition object, not for a dependent subquery. - if ($condition['field'] instanceof ConditionInterface) { - // Compile the sub-condition recursively and add it to the list. - $condition['field']->compile($connection, $queryPlaceholder); - $condition_fragments[] = '(' . (string) $condition['field'] . ')'; - $arguments += $condition['field']->arguments(); + // Left hand part is a normal field: add it as is. + $field_fragment = $condition['field']; + } + + // Process operator. + if ($condition['value'] === NULL && $condition['field'] instanceof ConditionInterface && $condition['operator'] === '=') { + // If the operator and value were not passed in (and thus have the + // default value as defined by the condition() method) and the field + // is a Condition, it is assumed to be a valid condition on its own: + // ignore the operator and value parts. + $operator = array('operator' => '', 'use_value' => FALSE); + } + else { + // Remove potentially dangerous characters. + // If something passed in an invalid character stop early, so we + // don't rely on a broken SQL statement when we would just replace + // those characters. + if (stripos($condition['operator'], 'UNION') !== FALSE || strpbrk($condition['operator'], '[-\'"();') !== FALSE) { + $this->changed = TRUE; + $this->arguments = []; + // Provide a string which will result into an empty query result. + $this->stringVersion = '( AND 1 = 0 )'; + + // Conceptually throwing an exception caused by user input is bad + // as you result into a WSOD, which depending on your webserver + // configuration can result into the assumption that your site is + // broken. + // On top of that the database API relies on __toString() which + // does not allow to throw exceptions. + trigger_error('Invalid characters in query operator: ' . $condition['operator'], E_USER_ERROR); + return; } - else { - // For simplicity, we treat all operators as the same data structure. - // In the typical degenerate case, this won't get changed. - $operator_defaults = array( - 'prefix' => '', - 'postfix' => '', - 'delimiter' => '', - 'operator' => $condition['operator'], - 'use_value' => TRUE, - ); - // Remove potentially dangerous characters. - // If something passed in an invalid character stop early, so we - // don't rely on a broken SQL statement when we would just replace - // those characters. - if (stripos($condition['operator'], 'UNION') !== FALSE || strpbrk($condition['operator'], '[-\'"();') !== FALSE) { - $this->changed = TRUE; - $this->arguments = []; - // Provide a string which will result into an empty query result. - $this->stringVersion = '( AND 1 = 0 )'; - - // Conceptually throwing an exception caused by user input is bad - // as you result into a WSOD, which depending on your webserver - // configuration can result into the assumption that your site is - // broken. - // On top of that the database API relies on __toString() which - // does not allow to throw exceptions. - trigger_error('Invalid characters in query operator: ' . $condition['operator'], E_USER_ERROR); - return; - } - $operator = $connection->mapConditionOperator($condition['operator']); - if (!isset($operator)) { - $operator = $this->mapConditionOperator($condition['operator']); - } - $operator += $operator_defaults; - - $placeholders = array(); - if ($condition['value'] instanceof SelectInterface) { - $condition['value']->compile($connection, $queryPlaceholder); - $placeholders[] = (string) $condition['value']; - $arguments += $condition['value']->arguments(); - // Subqueries are the actual value of the operator, we don't - // need to add another below. - $operator['use_value'] = FALSE; + + // For simplicity, we convert all operators to a data structure to + // allow to specify a prefix, a delimiter and such. Find the + // associated data structure by first doing a database specific + // lookup, followed by a specification according to the SQL standard. + $operator = $connection->mapConditionOperator($condition['operator']); + if (!isset($operator)) { + $operator = $this->mapConditionOperator($condition['operator']); + } + $operator += array('operator' => $condition['operator']); + } + // Add defaults. + $operator += array( + 'prefix' => '', + 'postfix' => '', + 'delimiter' => '', + 'use_value' => TRUE, + ); + $operator_fragment = $operator['operator']; + + // Process value. + $value_fragment = ''; + if ($operator['use_value']) { + // For simplicity, we first convert to an array, so that we can handle + // the single and multi value cases the same. + if (!is_array($condition['value'])) { + if ($operator['operator'] == 'IN' && $condition['value'] instanceof ConditionInterface) { + // Special case: IN is followed by a select query instead of a set + // of values: unset prefix and postfix to prevent double brackets. + $operator['prefix'] = ''; + $operator['postfix'] = ''; + } - // We assume that if there is a delimiter, then the value is an - // array. If not, it is a scalar. For simplicity, we first convert - // up to an array so that we can build the placeholders in the same way. - elseif (!$operator['delimiter'] && !is_array($condition['value'])) { - $condition['value'] = array($condition['value']); + + $condition['value'] = array($condition['value']); + } + // Process all individual values. + $value_fragment = array(); + foreach ($condition['value'] as $value) { + if ($value instanceof ConditionInterface) { + // Right hand part is a subquery: compile, assure that brackets + // are put around it and collect any arguments. + $value->compile($connection, $queryPlaceholder); + $value_fragment[] = '(' . (string) $value . ')'; + $arguments += $value->arguments(); } - if ($operator['use_value']) { - foreach ($condition['value'] as $value) { - $placeholder = ':db_condition_placeholder_' . $queryPlaceholder->nextPlaceholder(); - $arguments[$placeholder] = $value; - $placeholders[] = $placeholder; - } + else { + // Right hand part is a normal value: replace the value with a + // placeholder and add the value as an argument. + $placeholder = ':db_condition_placeholder_' . $queryPlaceholder->nextPlaceholder(); + $value_fragment[] = $placeholder; + $arguments[$placeholder] = $value; } - $condition_fragments[] = ' (' . $connection->escapeField($condition['field']) . ' ' . $operator['operator'] . ' ' . $operator['prefix'] . implode($operator['delimiter'], $placeholders) . $operator['postfix'] . ') '; } + $value_fragment = $operator['prefix'] . implode($operator['delimiter'], $value_fragment) . $operator['postfix']; } + + // Concatenate the left hand part, operator and right hand part. + $condition_fragments[] = trim(implode(' ', array($field_fragment, $operator_fragment, $value_fragment))); } - $this->changed = FALSE; - $this->stringVersion = implode($conjunction, $condition_fragments); + // Concatenate all conditions using the conjunction and brackets around + // the individual conditions to assure the proper evaluation order. + $this->stringVersion = count($condition_fragments) > 1 ? '(' . implode(") $conjunction (", $condition_fragments) . ')' : implode($condition_fragments); $this->arguments = $arguments; + $this->changed = FALSE; } } @@ -289,7 +332,7 @@ function __clone() { * the value data they pass in is not a simple value. This is a simple * overridable lookup function. * - * @param $operator + * @param string $operator * The condition operator, such as "IN", "BETWEEN", etc. Case-sensitive. * * @return array @@ -300,10 +343,10 @@ protected function mapConditionOperator($operator) { // $specials does not use drupal_static as its value never changes. static $specials = array( 'BETWEEN' => array('delimiter' => ' AND '), - 'IN' => array('delimiter' => ', ', 'prefix' => ' (', 'postfix' => ')'), - 'NOT IN' => array('delimiter' => ', ', 'prefix' => ' (', 'postfix' => ')'), - 'EXISTS' => array('prefix' => ' (', 'postfix' => ')'), - 'NOT EXISTS' => array('prefix' => ' (', 'postfix' => ')'), + 'IN' => array('delimiter' => ', ', 'prefix' => '(', 'postfix' => ')'), + 'NOT IN' => array('delimiter' => ', ', 'prefix' => '(', 'postfix' => ')'), + 'EXISTS' => array('prefix' => '(', 'postfix' => ')'), + 'NOT EXISTS' => array('prefix' => '(', 'postfix' => ')'), 'IS NULL' => array('use_value' => FALSE), 'IS NOT NULL' => array('use_value' => FALSE), // Use backslash for escaping wildcard characters. diff --git a/core/lib/Drupal/Core/Database/Query/ConditionInterface.php b/core/lib/Drupal/Core/Database/Query/ConditionInterface.php index bd0b52b..c9b6333 100644 --- a/core/lib/Drupal/Core/Database/Query/ConditionInterface.php +++ b/core/lib/Drupal/Core/Database/Query/ConditionInterface.php @@ -12,13 +12,21 @@ /** * Helper function: builds the most common conditional clauses. * - * This method can take a variable number of parameters. If called with two - * parameters, they are taken as $field and $value with $operator having a - * value of =. + * This method takes 1 to 3 parameters. + * + * If called with 1 parameter, it should be a ConditionInterface that in + * itself forms a valid where clause. Use e.g. to build clauses with nested + * AND's and OR's. + * + * If called with 2 parameters, they are taken as $field and $value with + * $operator having a value of =. * * Do not use this method to test for NULL values. Instead, use * QueryConditionInterface::isNull() or QueryConditionInterface::isNotNull(). * + * To improve readability, the operators EXISTS and NOT EXISTS have their own + * utility method defined. + * * Drupal considers LIKE case insensitive and the following is often used * to tell the database that case insensitive equivalence is desired: * @code @@ -32,33 +40,43 @@ * be case sensitive and when a case insensitive collation is used, the = * operator will also be case insensitive. * - * @param $field - * The name of the field to check. If you would like to add a more complex - * condition involving operators or functions, use where(). - * @param $value - * The value to test the field against. In most cases, this is a scalar. - * For more complex options, it is an array. The meaning of each element in - * the array is dependent on the $operator. - * @param $operator - * The comparison operator, such as =, <, or >=. It also accepts more - * complex options such as IN, LIKE, LIKE BINARY, or BETWEEN. Defaults to =. + * @param string|\Drupal\Core\Database\Query\ConditionInterface $field + * The name of the field to check. This can also be QueryConditionInterface + * in itself. Use where(), if you would like to add a more complex condition + * involving operators or functions, or an already compiled condition. + * @param string|array|\Drupal\Core\Database\Query\SelectInterface|null $value + * The value to test the field against. In most cases, and depending on the + * operator, this will be a scalar or an array. As SQL accepts select + * queries on any place where a scalar value or set is expected, $value may + * also be a(n array of) SelectInterface(s). If $operator is a unary + * operator, e.g. EXISTS, $value will be ignored and should be null. + * @param string|null $operator + * The operator to use. Supported for all supported databases are at least: + * - The comparison operators =, <>, <, <=, >, >=. + * - The operators (NOT) BETWEEN, (NOT) IN, (NOT) EXISTS, (NOT) LIKE. + * Other operators (e.g. LIke BINARY) may or may not work. Defaults to =. * * @return \Drupal\Core\Database\Query\ConditionInterface * The called object. * * @see \Drupal\Core\Database\Query\ConditionInterface::isNull() * @see \Drupal\Core\Database\Query\ConditionInterface::isNotNull() + * @see \Drupal\Core\Database\Query\ConditionInterface::exists() + * @see \Drupal\Core\Database\Query\ConditionInterface::notExist() + * @see \Drupal\Core\Database\Query\ConditionInterface::where() */ public function condition($field, $value = NULL, $operator = '='); /** * Adds an arbitrary WHERE clause to the query. * - * @param $snippet + * @param string $snippet * A portion of a WHERE clause as a prepared statement. It must use named - * placeholders, not ? placeholders. - * @param $args - * An associative array of arguments. + * placeholders, not ? placeholders. The caller is responsible for providing + * unique placeholders that do not interfere with the placeholders generated + * by this QueryConditionInterface object. + * @param array $args + * An associative array of arguments keyed by the named placeholders. * * @return \Drupal\Core\Database\Query\ConditionInterface * The called object. @@ -68,8 +86,8 @@ public function where($snippet, $args = array()); /** * Sets a condition that the specified field be NULL. * - * @param $field - * The name of the field to check. + * @param string|\Drupal\Core\Database\Query\SelectInterface $field + * The name of the field or a subquery to check. * * @return \Drupal\Core\Database\Query\ConditionInterface * The called object. @@ -79,8 +97,8 @@ public function isNull($field); /** * Sets a condition that the specified field be NOT NULL. * - * @param $field - * The name of the field to check. + * @param string|\Drupal\Core\Database\Query\SelectInterface $field + * The name of the field or a subquery to check. * * @return \Drupal\Core\Database\Query\ConditionInterface * The called object. @@ -110,7 +128,7 @@ public function exists(SelectInterface $select); public function notExists(SelectInterface $select); /** - * Gets a complete list of all conditions in this conditional clause. + * Gets the, possibly nested, list of conditions in this conditional clause. * * This method returns by reference. That allows alter hooks to access the * data structure directly and manipulate it before it gets compiled. @@ -131,6 +149,9 @@ public function notExists(SelectInterface $select); * * There will also be a single array entry of #conjunction, which is the * conjunction that will be applied to the array, such as AND. + * + * @return array + * The, possibly nested, list of all conditions (by reference). */ public function &conditions(); diff --git a/core/tests/Drupal/KernelTests/Core/Database/SelectSubqueryTest.php b/core/tests/Drupal/KernelTests/Core/Database/SelectSubqueryTest.php index 4877ca2..9a0a6ba 100644 --- a/core/tests/Drupal/KernelTests/Core/Database/SelectSubqueryTest.php +++ b/core/tests/Drupal/KernelTests/Core/Database/SelectSubqueryTest.php @@ -70,7 +70,7 @@ function testFromSubquerySelectWithLimit() { } /** - * Tests that we can use a subquery in a WHERE clause. + * Tests that we can use a subquery with an IN operator in a WHERE clause. */ function testConditionSubquerySelect() { // Create a subquery, which is just a normal query object. @@ -93,6 +93,55 @@ function testConditionSubquerySelect() { } /** + * Test that we can use a subquery with a relational operator in a WHERE clause. + */ + function testConditionSubquerySelect2() { + // Create a subquery, which is just a normal query object. + $subquery = db_select('test', 't2'); + $subquery->addExpression('AVG(t2.age)'); + + // Create another query that adds a clause using the subquery. + $select = db_select('test', 't'); + $select->addField('t', 'name'); + $select->condition('t.age', $subquery, '<'); + + // The resulting query should be equivalent to: + // SELECT t.name + // FROM test t + // WHERE t.age < (SELECT AVG(t2.age) FROM test t2) + $people = $select->execute()->fetchCol(); + $this->assertEqual(count($people), 2, t('Returned the correct number of rows.')); + $this->assertTrue(in_array('Paul', $people) && in_array('John', $people), t('Returned Paul and John.')); + } + + /** + * Test that we can use 2 subqueries with a relational operator in a WHERE clause. + */ + function testConditionSubquerySelect3() { + // Create subquery 1, which is just a normal query object. + $subquery1 = db_select('test_task', 'tt'); + $subquery1->addExpression('AVG(tt.priority)'); + $subquery1->where('tt.pid = t.id'); + + // Create subquery 2, which is just a normal query object. + $subquery2 = db_select('test_task', 'tt2'); + $subquery2->addExpression('AVG(tt2.priority)'); + + // Create another query that adds a clause using the subqueries. + $select = db_select('test', 't'); + $select->addField('t', 'name'); + $select->condition($subquery1, $subquery2, '>'); + + // The resulting query should be equivalent to: + // SELECT t.name + // FROM test t + // WHERE (SELECT AVG(tt.priority) FROM test_task tt WHERE tt.pid = t.id) > (SELECT AVG(tt2.priority) FROM test_task tt2) + $people = $select->execute()->fetchCol(); + $this->assertEqual(count($people), 1, t('Returned the correct number of rows.')); + $this->assertTrue(in_array('John', $people), t('Returned John.')); + } + + /** * Tests that we can use a subquery in a JOIN clause. */ function testJoinSubquerySelect() {