diff --git a/CHANGELOG-WIP.md b/CHANGELOG-WIP.md index 36fce589ea7..70da5815ac3 100644 --- a/CHANGELOG-WIP.md +++ b/CHANGELOG-WIP.md @@ -182,6 +182,8 @@ - Added `craft\base\conditions\ConditionInterface::createConditionRule()`. - Added `craft\behaviors\EventBehavior`. - Added `craft\controllers\EntryTypesController`. +- Added `craft\db\CallbackExpressionBuilder`. +- Added `craft\db\CallbackExpression`. - Added `craft\db\Connection::getIsMaria()`. - Added `craft\db\QueryParam`. - Added `craft\db\Table::ELEMENTS_OWNERS`. diff --git a/CHANGELOG.md b/CHANGELOG.md index 7714b7f1b73..e47ae7f7a21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Added `craft\db\CallbackExpressionBuilder`. +- Added `craft\db\CallbackExpression`. - Fixed a bug where querying elements with `eagerly()` wasn’t working across element types. - Fixed a SQL error that could occur when querying relations with `eagerly()`. - Fixed a bug where element actions weren’t working for elements that were added to a relational field. diff --git a/src/db/CallbackExpression.php b/src/db/CallbackExpression.php new file mode 100644 index 00000000000..f84b19f1eec --- /dev/null +++ b/src/db/CallbackExpression.php @@ -0,0 +1,44 @@ +andWhere( + * new CallbackExpression(function(array &$params): string { + * $params['foo'] = 'bar'; + * return '[[column_name]] = :foo'; + * }) + * ); + * ``` + * + * @author Pixel & Tonic, Inc. + * @since 5.0.0 + */ +class CallbackExpression extends BaseObject implements ExpressionInterface +{ + /** + * Constructor + * + * @param callable $callback the DB expression callback + * @param array $config name-value pairs that will be used to initialize the object properties + */ + public function __construct( + public $callback, + array $config = [], + ) { + parent::__construct($config); + } +} diff --git a/src/db/CallbackExpressionBuilder.php b/src/db/CallbackExpressionBuilder.php new file mode 100644 index 00000000000..ed4856f0ecc --- /dev/null +++ b/src/db/CallbackExpressionBuilder.php @@ -0,0 +1,36 @@ + + * @since 5.0.0 + */ +class CallbackExpressionBuilder implements ExpressionBuilderInterface +{ + use ExpressionBuilderTrait; + + /** + * @inheritdoc + */ + public function build(ExpressionInterface $expression, array &$params = []) + { + if (!$expression instanceof CallbackExpression) { + throw new NotSupportedException('$expression must be an instance of CallbackExpression.'); + } + + return call_user_func($expression->callback, $this->queryBuilder, $params); + } +} diff --git a/src/elements/db/ElementQuery.php b/src/elements/db/ElementQuery.php index 6503b545099..feac69212e5 100644 --- a/src/elements/db/ElementQuery.php +++ b/src/elements/db/ElementQuery.php @@ -16,8 +16,12 @@ use craft\behaviors\DraftBehavior; use craft\behaviors\RevisionBehavior; use craft\cache\ElementQueryTagDependency; +use craft\db\CallbackExpression; +use craft\db\CallbackExpressionBuilder; use craft\db\Connection; use craft\db\FixedOrderExpression; +use craft\db\mysql\QueryBuilder as MysqlQueryBuilder; +use craft\db\pgsql\QueryBuilder as PgsqlQueryBuilder; use craft\db\Query; use craft\db\QueryAbortedException; use craft\db\Table; @@ -366,13 +370,6 @@ class ElementQuery extends Query implements ElementQueryInterface */ public mixed $ref = null; - /** - * @inheritdoc - * @used-by orderBy() - * @used-by addOrderBy() - */ - public $orderBy = ''; - // Eager-loading // ------------------------------------------------------------------------- @@ -567,8 +564,14 @@ public function __construct(string $elementType, array $config = []) { $this->elementType = $elementType; - // Use ** as a placeholder for "all the default columns" - $config['select'] = $config['select'] ?? ['**' => '**']; + // Set the default `select` and `orderBy` params to callback expressions, + // so we can determine the SQL dynamically only once the query is being executed + if (!isset($this->select)) { + $this->select(new CallbackExpression(fn(QueryBuilder $builder) => $this->defaultSelectSql($builder))); + } + if (!isset($this->orderBy)) { + $this->orderBy(new CallbackExpression(fn(QueryBuilder $builder) => $this->defaultOrderBySql($builder))); + } parent::__construct($config); } @@ -1376,6 +1379,11 @@ public function cache($duration = true, $dependency = null): \yii\db\Query|Eleme */ public function prepare($builder): Query { + // Set our callback expression builder + $builder->setExpressionBuilders([ + CallbackExpression::class => CallbackExpressionBuilder::class, + ]); + // Log a warning if the app isn't fully initialized yet if (!Craft::$app->getIsInitialized()) { Craft::warning( @@ -2959,33 +2967,6 @@ private function _applyOrderByParams(YiiConnection $db): void return; } - // Any other empty value means we should set it - if (empty($this->orderBy)) { - if ($this->fixedOrder) { - if (empty($this->id)) { - throw new QueryAbortedException(); - } - - $ids = $this->id; - if (!is_array($ids)) { - $ids = is_string($ids) ? StringHelper::split($ids) : [$ids]; - } - - if (!$db instanceof Connection) { - throw new Exception('The database connection doesn’t support fixed ordering.'); - } - $this->orderBy = [new FixedOrderExpression('elements.id', $ids, $db)]; - } elseif ($this->revisions) { - $this->orderBy = ['num' => SORT_DESC]; - } elseif ($this->_shouldJoinStructureData()) { - $this->orderBy = ['structureelements.lft' => SORT_ASC] + $this->defaultOrderBy; - } elseif (!empty($this->defaultOrderBy)) { - $this->orderBy = $this->defaultOrderBy; - } else { - return; - } - } - // Rename orderBy keys based on the real column name mapping // (yes this is awkward but we need to preserve the order of the keys!) /** @var array $orderBy */ @@ -3052,6 +3033,57 @@ private function _applyOrderByParams(YiiConnection $db): void $this->subQuery->orderBy($orderBy); } + private function defaultOrderBySql(QueryBuilder $builder): string + { + $db = $builder->db; + + if ($this->fixedOrder) { + if (empty($this->id)) { + throw new QueryAbortedException(); + } + + $ids = $this->id; + if (!is_array($ids)) { + $ids = is_string($ids) ? StringHelper::split($ids) : [$ids]; + } + + if (!($builder instanceof MysqlQueryBuilder || $builder instanceof PgsqlQueryBuilder)) { + throw new Exception('The database connection doesn’t support fixed ordering.'); + } + + if ($this->inReverse) { + $ids = array_reverse($ids); + } + + return $builder->fixedOrder('elements.id', $ids); + } + + if ($this->revisions) { + $columns = ['num' => SORT_DESC]; + } elseif ($this->_shouldJoinStructureData()) { + $columns = ['structureelements.lft' => SORT_ASC] + $this->defaultOrderBy; + } elseif (!empty($this->defaultOrderBy)) { + $columns = $this->defaultOrderBy; + } else { + return ''; + } + + $orders = []; + foreach ($columns as $name => $direction) { + if ($direction instanceof ExpressionInterface) { + $orders[] = $builder->buildExpression($direction); + } else { + if ($this->inReverse) { + $direction = $direction === SORT_DESC ? SORT_ASC : SORT_DESC; + } + + $orders[] = $db->quoteColumnName($name) . ($direction === SORT_DESC ? ' DESC' : ''); + } + } + + return implode(', ', $orders); + } + /** * Applies the 'select' param to the query being prepared. */ @@ -3059,60 +3091,66 @@ private function _applySelectParam(): void { // Select all columns defined by [[select]], swapping out any mapped column names $select = []; - $includeDefaults = false; foreach ((array)$this->select as $alias => $column) { - if ($alias === '**') { - $includeDefaults = true; - } else { - // Is this a mapped column name (without a custom alias)? - if ($alias === $column && isset($this->_columnMap[$alias])) { - $column = $this->_columnMap[$alias]; + // Is this a mapped column name (without a custom alias)? + if ($alias === $column && isset($this->_columnMap[$alias])) { + $column = $this->_columnMap[$alias]; - // Completely ditch the mapped name if instantiated elements are going to be returned - if (!$this->asArray) { - $alias = $this->_columnMap[$alias]; - } + // Completely ditch the mapped name if instantiated elements are going to be returned + if (!$this->asArray) { + $alias = $this->_columnMap[$alias]; } - - $select[$alias] = $column; } + + $select[$alias] = $column; } - // Is there still a ** placeholder param? - if ($includeDefaults) { - // Merge in the default columns - $select = array_merge($select, [ - 'elements.id' => 'elements.id', - 'elements.canonicalId' => 'elements.canonicalId', - 'elements.fieldLayoutId' => 'elements.fieldLayoutId', - 'elements.uid' => 'elements.uid', - 'elements.enabled' => 'elements.enabled', - 'elements.archived' => 'elements.archived', - 'elements.dateLastMerged' => 'elements.dateLastMerged', - 'elements.dateCreated' => 'elements.dateCreated', - 'elements.dateUpdated' => 'elements.dateUpdated', - 'siteSettingsId' => 'elements_sites.id', - 'elements_sites.siteId' => 'elements_sites.siteId', - 'elements_sites.title' => 'elements_sites.title', - 'elements_sites.slug' => 'elements_sites.slug', - 'elements_sites.uri' => 'elements_sites.uri', - 'elements_sites.content' => 'elements_sites.content', - 'enabledForSite' => 'elements_sites.enabled', - ]); + // If the query already specifies any columns, merge those in too + if (!empty($this->query->select)) { + $select = array_merge($select, $this->query->select); + } - // If the query includes soft-deleted elements, include the date deleted - if ($this->trashed !== false) { - $select['elements.dateDeleted'] = 'elements.dateDeleted'; - } + $this->query->select = $select; + } - // If the query already specifies any columns, merge those in too - if (!empty($this->query->select)) { - $select = array_merge($select, $this->query->select); + private function defaultSelectSql(QueryBuilder $builder): string + { + $columns = [ + 'elements.id' => 'elements.id', + 'elements.canonicalId' => 'elements.canonicalId', + 'elements.fieldLayoutId' => 'elements.fieldLayoutId', + 'elements.uid' => 'elements.uid', + 'elements.enabled' => 'elements.enabled', + 'elements.archived' => 'elements.archived', + 'elements.dateLastMerged' => 'elements.dateLastMerged', + 'elements.dateCreated' => 'elements.dateCreated', + 'elements.dateUpdated' => 'elements.dateUpdated', + 'siteSettingsId' => 'elements_sites.id', + 'elements_sites.siteId' => 'elements_sites.siteId', + 'elements_sites.title' => 'elements_sites.title', + 'elements_sites.slug' => 'elements_sites.slug', + 'elements_sites.uri' => 'elements_sites.uri', + 'elements_sites.content' => 'elements_sites.content', + 'enabledForSite' => 'elements_sites.enabled', + ]; + + // If the query includes soft-deleted elements, include the date deleted + if ($this->trashed !== false) { + $columns['elements.dateDeleted'] = 'elements.dateDeleted'; + } + + $db = $builder->db; + + foreach ($columns as $alias => &$column) { + if ($alias === $column) { + $column = $db->quoteColumnName($column); + } else { + $column = sprintf('%s AS %s', $db->quoteColumnName($column), $db->quoteColumnName($alias)); } } - $this->query->select = $select; + return implode(', ', $columns); } /** diff --git a/src/fields/BaseRelationField.php b/src/fields/BaseRelationField.php index f3c57dbcc26..d4d9cc1e576 100644 --- a/src/fields/BaseRelationField.php +++ b/src/fields/BaseRelationField.php @@ -16,6 +16,7 @@ use craft\base\InlineEditableFieldInterface; use craft\base\NestedElementInterface; use craft\behaviors\EventBehavior; +use craft\db\CallbackExpression; use craft\db\Query; use craft\db\Table as DbTable; use craft\elements\conditions\ElementCondition; @@ -644,9 +645,9 @@ public function normalizeValue(mixed $value, ?ElementInterface $element): mixed } } - $relationsAlias = sprintf('relations_%s', StringHelper::randomString(10)); + // Make our query customizations via EVENT_AFTER_PREPARE/EVENT_AFTER_PREPARE, + // so they get applied for cloned queries as well - // join the relations table via EVENT_BEFORE_PREPARE so it gets joined for cloned queries as well $query->attachBehavior(sprintf('%s-once', self::class), new EventBehavior([ ElementQuery::EVENT_BEFORE_PREPARE => function(CancelableEvent $event, ElementQuery $query) { if ($this->maintainHierarchy && $query->id === null) { @@ -667,10 +668,19 @@ public function normalizeValue(mixed $value, ?ElementInterface $element): mixed $query->id(array_map(fn(ElementInterface $element) => $element->id, $structureElements)); } }, + ], true)); + + $relationsAlias = sprintf('relations_%s', StringHelper::randomString(10)); + + $query->attachBehavior(self::class, new EventBehavior([ ElementQuery::EVENT_AFTER_PREPARE => function( CancelableEvent $event, ElementQuery $query, ) use ($element, $relationsAlias) { + // Make these changes directly on the prepared queries, so `sortOrder` doesn't ever make it into + // the criteria. Otherwise, if the query ends up A) getting executed normally, then B) getting + // eager-loaded with eagerly(), the `orderBy` value referencing the join table will get applied + // to the eager-loading query and cause a SQL error. foreach ([$query->query, $query->subQuery] as $q) { $q->innerJoin( [$relationsAlias => DbTable::RELATIONS], @@ -689,7 +699,14 @@ public function normalizeValue(mixed $value, ?ElementInterface $element): mixed ] ); - if ($this->sortable && !$this->maintainHierarchy && !$q->orderBy) { + if ( + $this->sortable && + !$this->maintainHierarchy && + ( + !$query->orderBy || + (count($query->orderBy) === 1) && ($query->orderBy[0] ?? null) instanceof CallbackExpression + ) + ) { $q->orderBy(["$relationsAlias.sortOrder" => SORT_ASC]); } }