Make the conditions in joins in dynamic queries use Condition objects
Closes #3398773
Merge request reports
Activity
127 127 return $this; 128 128 } 129 129 130 /** 131 * Compare two database fields with each other. I picked
field
, because in the rest of the existing classDrupal\Core\Database\Query\Condition
it is all about fields and not about columns. If this was a greenfield solution, I would go with column, only it is not. Again: if you after reading this still think we should change something, then I will do that.
135 * 136 * @param string $field 137 * The name of the field to compare. 138 * @param string $field2 139 * The name of the other field to compare. 140 * @param string|null $operator 141 * (optional) The operator to use. The supported operators are: =, <>, <, 142 * <=, >, >=, <>. 143 * 144 * @return $this 145 * The called object. 146 * 147 * @throws \Drupal\Core\Database\InvalidQueryException 148 * If passed invalid arguments, such as an empty array as $value. 149 */ 150 public function compare(string $field, string $field2, ?string $operator = '=') { Again, terminology..., and I would suggest to have the operator explicit, not optional, and between the two column names... so that things get much more readable.
then it would get to
->compareColumns("base_table.$revision_field", '<', "base_table_2.$revision_field"));
Alternative to
compareColumns()
-->columnsCondition()
, or even simplycolumns()
?I do not think it's a big ask to developers to be clear what they want to do.
I picked
compare()
, because it is essentially similar to the methodcondition()
only with 2 fields instead of an field and a value. I pickedfield
, because in the rest of the existing classDrupal\Core\Database\Query\Condition
it is all about fields and not about columns. If this was a greenfield solution, I would go with column, only it is not. I made the operator the last parameter, because the method is mostly going to be used in join conditions and in >90% the cases the operator used is going to be an equal sign. When the operator is the last parameter and it defaults to an equal sign, the operator in >90% does not need to be set. Just like it is with the methodcondition()
. My idea was that the new method would be the same as the methodcondition()
, only for 2 fields instead of a field and a value. That is what Drupal developers know and expect. For me that would be the best developer experience (DX). If you after reading this still think we should change something, then I will do that.changed this line in version 18 of the diff
142 * <=, >, >=, <>. 143 * 144 * @return $this 145 * The called object. 146 * 147 * @throws \Drupal\Core\Database\InvalidQueryException 148 * If passed invalid arguments, such as an empty array as $value. 149 */ 150 public function compare(string $field, string $field2, ?string $operator = '=') { 151 if (!in_array($operator, ['=', '<', '>', '>=', '<=', '<>'], TRUE)) { 152 throw new InvalidQueryException(sprintf("In a query compare '%s %s %s' the operator must be one of the following: '=', '<', '>', '>=', '<=', '<>'.", $field, $operator, $field2)); 153 } 154 155 $this->conditions[] = [ 156 'field' => $field, 157 'field2' => $field2, - Comment on lines +156 to +157
I picked
field2
, because in the rest of the existing classDrupal\Core\Database\Query\Condition
it is all about fields and not about columns. If this was a greenfield solution, I would go with column, only it is not. Again: if you after reading this still think we should change something, then I will do that.
161 $this->changed = TRUE; 162 163 return $this; 164 } 165 166 /** 167 * Update the alias placeholder in the condition and its children. 168 * 169 * @param string $placeholder 170 * The value of the placeholder. 171 * @param string $alias 172 * The value to replace the placeholder. 173 * 174 * @internal 175 */ 176 public function updateAliasPlaceholder(string $placeholder, string $alias): void { changed this line in version 12 of the diff
631 631 * {@inheritdoc} 632 632 */ 633 633 public function addJoin($type, $table, $alias = NULL, $condition = NULL, $arguments = []) { 634 if (!empty($condition) && !$condition instanceof ConditionInterface) { changed this line in version 12 of the diff
437 438 // Only the data table follows the entity language key, dedicated field 438 439 // tables have a hard-coded 'langcode' column. 439 440 $langcode_key = $entity_type->getDataTable() == $table ? $entity_type->getKey('langcode') : 'langcode'; 440 $placeholder = ':langcode' . $this->sqlQuery->nextPlaceholder(); 441 $join_condition .= ' AND [%alias].[' . $langcode_key . '] = ' . $placeholder; 442 $arguments[$placeholder] = $langcode; 441 if ($join_condition instanceof ConditionInterface) { 442 $join_condition->condition('%alias.' . $langcode_key, $langcode); 443 } 444 // Start of BC layer. 445 else { 446 $placeholder = ':langcode' . $this->sqlQuery->nextPlaceholder(); 447 $join_condition .= ' AND [%alias].[' . $langcode_key . '] = ' . $placeholder; 448 $arguments[$placeholder] = $langcode; 449 } 450 // End of BC layer. - Comment on lines +441 to +450
changed this line in version 12 of the diff
132 132 // Run a select query to return 'last_prefix' from {simpletest_test_id} and 133 133 // 'test_class' from {simpletest}. 134 134 $select = $this->connection->select($max_message_id_subquery, 'st_sub'); 135 $select->join('simpletest', 'st', '[st].[message_id] = [st_sub].[max_message_id]'); 136 $select->join('simpletest_test_id', 'sttid', '[st].[test_id] = [sttid].[test_id]'); 135 $select->join('simpletest', 'st', $this->connection->condition('AND')->compare('st.message_id', 'st_sub.max_message_id')); 136 $select->join('simpletest_test_id', 'sttid', $this->connection->condition('AND')->compare('st.test_id', 'sttid.test_id')); - Comment on lines +135 to +136
changed this line in version 3 of the diff
32 43 $query = $this->select('upload', 'u') 33 44 ->distinct() 34 45 ->fields('u', ['nid', 'vid']); 35 $query->innerJoin('node', 'n', static::JOIN); 46 47 // Start of BC layer. 48 if (is_string(static::JOIN)) { @mondrake: Thank you for the review.
added 1 commit
- b85b1f75 - Added deprecations to the methods: join(), innerJoin() and leftJoin()
added 1 commit
- 20326dc7 - Added the method joinCondition to the selectextender
added 1 commit
- 3a235aff - Fixed wrong change in Drupal\migrate\Plugin\migrate\id_map\Sql
added 1 commit
- a4a32dd5 - Removed the added deprecation of query conditions as strings and removed the...