From 2e88fd0b3c6c8951681955e784be8b1af98202f3 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 15 Oct 2026 22:59:21 +1300 Subject: [PATCH 1/3] fix(query): count grouped, distinct and aggregate reads by their rows count() handed the adapter only the filters and joins, so a list total for a groupBy, distinct or aggregate query counted the matching documents instead of the rows aggregate() or find() returns: 9 instead of 4 for a groupBy, 9 instead of 1 for a plain aggregate, 8 instead of 3 for a left join plus groupBy, 7 instead of 5 for distinct over a join, with or without row-level permissions. sum() dropped the same queries. For those shapes the Database now builds the adapter queries and collection through the same helpers fetch() uses (readQueries(), readCollection(), assertReadable()), with the same join authorisation and permission skip. The SQL adapter builds the statement find() builds through a shared plan() and the same builder setup, without order, cursor and page, bounded by LIMIT max, and counts it as SELECT COUNT(1) AS sum FROM (...) table_count. That keeps the permission and join hooks identical between results and count, including the emulated full outer join paths on MariaDB, MySQL and SQLite. sum() over grouped or distinct rows adds up a groupBy or selected attribute of those rows. Adapters without aggregate support (Mongo, Memory, Redis) now refuse these counts with the QueryException find() and aggregate() already throw, rather than return the document count. Plain and join-only counts are unchanged. Co-Authored-By: Claude Opus 5.5 --- src/Database/Adapter/SQL.php | 520 ++++++++++++------ src/Database/Adapter/SQL/Plan.php | 33 ++ src/Database/Trait/Documents.php | 256 +++++++-- tests/e2e/Adapter/Scopes/AggregationTests.php | 200 +++++++ tests/unit/RowCountTest.php | 228 ++++++++ 5 files changed, 1018 insertions(+), 219 deletions(-) create mode 100644 src/Database/Adapter/SQL/Plan.php create mode 100644 tests/unit/RowCountTest.php diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index acc9c983a..a0d3fbc82 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -20,6 +20,7 @@ use Utopia\Database\Adapter\SQL\Hook\Tenant; use Utopia\Database\Adapter\SQL\Hook\WriteContext; use Utopia\Database\Adapter\SQL\JoinAlias; +use Utopia\Database\Adapter\SQL\Plan; use Utopia\Database\Adapter\SQL\Scope\Filter as FilterScope; use Utopia\Database\Adapter\SQL\Scope\Raw as RawScope; use Utopia\Database\Attribute; @@ -1560,70 +1561,16 @@ public function find(Document $collection, array $queries = [], ?int $limit = 25 return $documents; } - // Single pass partitioning: pull vector queries out for ORDER BY and - // detect aggregation/join shape in the same walk. Each Method::value - // is checked once per query rather than three times. - // Defer the defensive `clone` until we know the query path will mutate - // the Query objects (joins or aggregations-with-joins). The vast - // majority of finds take neither path and don't need a per-query - // clone allocation. - $vectorQueries = []; - $otherQueries = []; - $adapterFilterQueries = []; - $hasAggregation = false; - $hasJoins = false; - $hasDistinct = false; - - foreach ($queries as $query) { - $method = $query->getMethod(); - - if ($method->isVector()) { - $vectorQueries[] = $query; - - continue; - } - - if ($this->isAdapterFilterQuery($query)) { - $adapterFilterQueries[] = $query; - - continue; - } - - $otherQueries[] = $query; - - if ($method->isAggregate() || $method === Method::GroupBy) { - $hasAggregation = true; - } - if ($method->isJoin()) { - $hasJoins = true; - } - if ($method === Method::Distinct) { - $hasDistinct = true; - } - } - - $queries = $otherQueries; - - if ($hasJoins) { - $queries = \array_map(static fn ($query) => clone $query, $queries); - } - - $joinTablePrefixes = []; - if ($hasJoins) { - $joinTablePrefixes = $this->remapJoinQueries($queries); - } - $unindexed = $this->unindexedJoins($collectionDoc, $queries, $joinTablePrefixes); - - $hasPreservingOuterJoin = false; - if ($hasJoins) { - foreach ($queries as $query) { - $method = $query->getMethod(); - if ($method === Method::RightJoin || $method === Method::FullOuterJoin) { - $hasPreservingOuterJoin = true; - break; - } - } - } + $plan = $this->plan($collectionDoc, $queries); + $queries = $plan->queries; + $vectorQueries = $plan->vectorQueries; + $adapterFilterQueries = $plan->adapterFilterQueries; + $joinTablePrefixes = $plan->joinTablePrefixes; + $unindexed = $plan->unindexed; + $hasAggregation = $plan->hasAggregation; + $hasJoins = $plan->hasJoins; + $hasDistinct = $plan->hasDistinct; + $hasPreservingOuterJoin = $plan->hasPreservingOuterJoin; if ($joinTablePrefixes !== []) { [$orderAttributes, $cursor] = $this->qualifyJoinedOrders($orderAttributes, $cursor, $collectionDoc, $joinTablePrefixes); @@ -1636,10 +1583,8 @@ public function find(Document $collection, array $queries = [], ?int $limit = 25 ??= $this->qualifyOrderAttribute($attribute, $joinAliases); }; - $emulatesFullOuterJoin = $this->needsFullOuterJoinEmulation($this->dialectBuilder(), $queries); - - if ($emulatesFullOuterJoin && $hasAggregation) { - $results = $this->findFullOuterJoinAggregate( + if ($plan->emulatesFullOuterJoin && $hasAggregation) { + [$aggregation, $columns, $qualifiedGroups] = $this->fullOuterJoinAggregation( $collectionDoc, $queries, $joinTablePrefixes, @@ -1657,89 +1602,13 @@ public function find(Document $collection, array $queries = [], ?int $limit = 25 $cursorDirection, $resolveInternalKey, ); - } elseif ($emulatesFullOuterJoin) { - if ($hasDistinct) { - $this->assertDistinctOrderIsSelected($queries, $orderAttributes, $orderTypes, $joinAliases); - } - - [$leftQueries, $rightQueries] = $this->emulateFullOuterJoin($queries, $alias); - $leftPreserving = $this->keepsUnmatchedRows($leftQueries); - - $left = $this->newBuilder($name, $alias, $leftPreserving, unindexed: $unindexed); - $leftProjected = $this->configureFindBuilder( - $left, - $collectionDoc, - $leftQueries, - $joinTablePrefixes, - $hasAggregation, - $hasDistinct, - $adapterFilterQueries, - $name, - $alias, - $roles, - $forPermission, - orderAttributes: $orderAttributes, - ); - $this->applyFullOuterJoinOrderProjection( - $left, - $collectionDoc, - $alias, - $orderAttributes, - $orderTypes, - $leftProjected, - $joinTablePrefixes, - ); - $this->applyFindCursor( - $left, - $orderAttributes, - $orderTypes, - $cursor, - $cursorDirection, - $resolveInternalKey, - nullable: true, - ); - - $right = $this->newBuilder($name, $alias, true, unindexed: $unindexed); - $rightProjected = $this->configureFindBuilder( - $right, - $collectionDoc, - $rightQueries, - $joinTablePrefixes, - $hasAggregation, - $hasDistinct, - $adapterFilterQueries, + $results = $this->fullOuterJoinResultNames($this->executeSelect($aggregation, Event::DocumentFind, $name), $columns, $qualifiedGroups); + } elseif ($plan->emulatesFullOuterJoin) { + $results = $this->executeSelect( + $this->fullOuterJoinUnion($collectionDoc, $plan, $name, $alias, $roles, $forPermission, $orderAttributes, $orderTypes, $limit, $offset, $cursor, $cursorDirection, $resolveInternalKey), + Event::DocumentFind, $name, - $alias, - $roles, - $forPermission, - orderAttributes: $orderAttributes, ); - $this->applyFullOuterJoinOrderProjection( - $right, - $collectionDoc, - $alias, - $orderAttributes, - $orderTypes, - $rightProjected, - $joinTablePrefixes, - ); - $this->applyFindCursor( - $right, - $orderAttributes, - $orderTypes, - $cursor, - $cursorDirection, - $resolveInternalKey, - nullable: true, - ); - - if ($hasDistinct) { - $left->union($right); - } else { - $left->unionAll($right); - } - $this->applyFindPage($left, $orderAttributes, $orderTypes, $limit, $offset, $cursorDirection, afterUnion: true); - $results = $this->executeSelect($left, Event::DocumentFind, $name); } else { $bound = $hasJoins && ! $hasAggregation && ! $hasDistinct && $vectorQueries === [] && $this->boundsJoinedSort() ? $this->boundedPage($collectionDoc, $queries, $adapterFilterQueries, $joinTablePrefixes, $orderAttributes, $orderTypes, $limit, $offset, $cursor) @@ -1761,16 +1630,7 @@ public function find(Document $collection, array $queries = [], ?int $limit = 25 orderAttributes: $orderAttributes, ); - $vectorDistance = null; - $vectorQuery = $vectorQueries[0] ?? null; - if ($vectorQuery !== null) { - $vectorDistance = $this->getVectorOrderRaw($vectorQuery, $alias); - } - - if ($vectorDistance !== null && $vectorQuery !== null) { - $vectorAttribute = $this->quote($this->filter($vectorQuery->getAttribute())); - $builder->whereRaw($this->quote($alias).".{$vectorAttribute} IS NOT NULL"); - } + $vectorDistance = $this->vectorDistance($builder, $vectorQueries[0] ?? null, $alias); if (! empty($cursor) && $vectorDistance !== null && ! $hasDistinct) { $distance = $cursor[Document::DISTANCE] ?? null; @@ -1868,6 +1728,102 @@ public function find(Document $collection, array $queries = [], ?int $limit = 25 return $documents; } + /** + * Split the queries of a find() or a row count by how the statement reads them. Join queries are cloned before + * their columns are qualified, so the caller's queries are left as they were. + * + * @param array $queries + * + * @throws QueryException + */ + private function plan(Document $collection, array $queries): Plan + { + $vectorQueries = []; + $otherQueries = []; + $adapterFilterQueries = []; + $hasAggregation = false; + $hasJoins = false; + $hasDistinct = false; + + foreach ($queries as $query) { + $method = $query->getMethod(); + + if ($method->isVector()) { + $vectorQueries[] = $query; + + continue; + } + + if ($this->isAdapterFilterQuery($query)) { + $adapterFilterQueries[] = $query; + + continue; + } + + $otherQueries[] = $query; + + if ($method->isAggregate() || $method === Method::GroupBy) { + $hasAggregation = true; + } + if ($method->isJoin()) { + $hasJoins = true; + } + if ($method === Method::Distinct) { + $hasDistinct = true; + } + } + + $queries = $otherQueries; + + $joinTablePrefixes = []; + if ($hasJoins) { + $queries = \array_map(static fn ($query) => clone $query, $queries); + $joinTablePrefixes = $this->remapJoinQueries($queries); + } + $unindexed = $this->unindexedJoins($collection, $queries, $joinTablePrefixes); + + $hasPreservingOuterJoin = false; + if ($hasJoins) { + foreach ($queries as $query) { + $method = $query->getMethod(); + if ($method === Method::RightJoin || $method === Method::FullOuterJoin) { + $hasPreservingOuterJoin = true; + break; + } + } + } + + return new Plan( + queries: $queries, + vectorQueries: $vectorQueries, + adapterFilterQueries: $adapterFilterQueries, + joinTablePrefixes: $joinTablePrefixes, + unindexed: $unindexed, + hasAggregation: $hasAggregation, + hasJoins: $hasJoins, + hasDistinct: $hasDistinct, + hasPreservingOuterJoin: $hasPreservingOuterJoin, + emulatesFullOuterJoin: $this->needsFullOuterJoinEmulation($this->dialectBuilder(), $queries), + ); + } + + /** + * Keep the rows a vector query can rank, those with a vector, and return the distance it ranks them by. + */ + private function vectorDistance(SQLBuilder $builder, ?Query $vectorQuery, string $alias): ?Expression + { + if ($vectorQuery === null) { + return null; + } + + $distance = $this->getVectorOrderRaw($vectorQuery, $alias); + if ($distance !== null) { + $builder->whereRaw($this->quote($alias).'.'.$this->quote($this->filter($vectorQuery->getAttribute())).' IS NOT NULL'); + } + + return $distance; + } + /** * @param array $bindings * @return array @@ -1914,6 +1870,10 @@ public function count(Document $collection, array $queries = [], ?int $max = nul $roles = $this->authorization->getRoles(); $alias = Query::DEFAULT_ALIAS; + if ($this->shapesRows($queries)) { + return $this->executeWrappedCount($this->rowsBuilder($collectionDoc, $this->plan($collectionDoc, $queries), $name, $alias, $roles, $max), $name); + } + $otherQueries = []; $hasJoins = false; foreach ($queries as $query) { @@ -1986,6 +1946,17 @@ public function sum(Document $collection, string $attribute, array $queries = [] $roles = $this->authorization->getRoles(); $alias = Query::DEFAULT_ALIAS; + if ($this->shapesRows($queries)) { + $plan = $this->plan($collectionDoc, $queries); + $column = $this->rowsColumn($attribute, $queries, $plan->hasAggregation); + + $outerBuilder = $this->dialectBuilder(); + $outerBuilder->fromSub($this->rowsBuilder($collectionDoc, $plan, $name, $alias, $roles, $max), 'table_count'); + $outerBuilder->selectRaw('SUM('.$this->quote($column).') AS '.$this->quote('sum')); + + return $this->sumOf($this->fetchAggregateRow($outerBuilder, Event::DocumentSum, $name)); + } + $otherQueries = []; $hasJoins = false; foreach ($queries as $query) { @@ -2048,6 +2019,117 @@ public function sum(Document $collection, string $attribute, array $queries = [] return $this->executeWrappedSum($builder, $attribute, $name); } + /** + * Whether the queries read rows other than one per matching document: groups, aggregates or distinct values. + * + * @param array $queries + */ + private function shapesRows(array $queries): bool + { + foreach ($queries as $query) { + $method = $query->getMethod(); + if ($method->isAggregate() || $method === Method::GroupBy || $method === Method::Distinct) { + return true; + } + } + + return false; + } + + /** + * The column grouped rows hold a groupBy attribute in, or distinct rows hold a selected attribute in. + * + * @param array $queries + * + * @throws QueryException When the attribute is not one of the groups or selections + */ + private function rowsColumn(string $attribute, array $queries, bool $grouped): string + { + foreach ($queries as $query) { + $method = $query->getMethod(); + if ($method !== ($grouped ? Method::GroupBy : Method::Select)) { + continue; + } + + /** @var array $values */ + $values = $query->getValues(); + $index = \array_search($attribute, $values, true); + if ($index === false) { + continue; + } + + $dot = \strpos($attribute, '.'); + $column = $this->filter($this->getInternalKeyForAttribute($dot === false ? $attribute : \substr($attribute, $dot + 1))); + + if ($grouped) { + return $this->qualifiedGroupNames($values)[$index] ?? $column; + } + + return $dot === false ? $column : \substr($attribute, 0, $dot).'.'.$column; + } + + throw new QueryException('Invalid query: sum() over '.($grouped ? 'grouped' : 'distinct').' rows adds up one of their '.($grouped ? 'groupBy' : 'selected').' attributes, and '.$attribute.' is not one of them'); + } + + /** + * The statement find() reads for the plan, unordered and unpaged, bounded to the first $max rows. + * + * @param array $roles + * + * @throws DatabaseException + */ + private function rowsBuilder(Document $collection, Plan $plan, string $name, string $alias, array $roles, ?int $max): SQLBuilder + { + $joinAliases = \array_column($plan->joinTablePrefixes, 'alias'); + $resolveInternalKey = fn (string $attribute): string => $this->qualifyOrderAttribute($attribute, $joinAliases); + + if ($plan->emulatesFullOuterJoin && $plan->hasAggregation) { + [$aggregation] = $this->fullOuterJoinAggregation( + $collection, + $plan->queries, + $plan->joinTablePrefixes, + $plan->hasDistinct, + $plan->adapterFilterQueries, + $name, + $alias, + $roles, + PermissionType::Read, + [], + [], + $max, + null, + [], + CursorDirection::After, + $resolveInternalKey, + ); + + return $aggregation; + } + + if ($plan->emulatesFullOuterJoin) { + return $this->fullOuterJoinUnion($collection, $plan, $name, $alias, $roles, PermissionType::Read, [], [], $max, null, [], CursorDirection::After, $resolveInternalKey); + } + + $builder = $this->newBuilder($name, $alias, $plan->hasPreservingOuterJoin, unindexed: $plan->unindexed); + $this->configureFindBuilder( + $builder, + $collection, + $plan->queries, + $plan->joinTablePrefixes, + $plan->hasAggregation, + $plan->hasDistinct, + $plan->adapterFilterQueries, + $name, + $alias, + $roles, + PermissionType::Read, + ); + $this->vectorDistance($builder, $plan->vectorQueries[0] ?? null, $alias); + $this->applyFindPage($builder, [], [], $max, null, joinAliases: $joinAliases); + + return $builder; + } + /** * @param array $queries * @param array $roles @@ -5104,6 +5186,118 @@ private function quoteOrderColumn(string $key, string $alias): string return $this->quote(\substr($key, 0, $dot)).'.'.$this->quote(\substr($key, $dot + 1)); } + /** + * The rows of an emulated full outer join: its left and right halves, each filtered, projected and past the + * cursor, as one union, ordered and paged. + * + * @param array $roles + * @param array $orderAttributes + * @param array $orderTypes + * @param array $cursor + * @param callable(string): string $resolveInternalKey + * + * @throws DatabaseException + */ + private function fullOuterJoinUnion( + Document $collectionDoc, + Plan $plan, + string $name, + string $alias, + array $roles, + PermissionType $forPermission, + array $orderAttributes, + array $orderTypes, + ?int $limit, + ?int $offset, + array $cursor, + CursorDirection $cursorDirection, + callable $resolveInternalKey, + ): SQLBuilder { + if ($plan->hasDistinct) { + $this->assertDistinctOrderIsSelected($plan->queries, $orderAttributes, $orderTypes, \array_column($plan->joinTablePrefixes, 'alias')); + } + + [$leftQueries, $rightQueries] = $this->emulateFullOuterJoin($plan->queries, $alias); + $leftPreserving = $this->keepsUnmatchedRows($leftQueries); + + $left = $this->newBuilder($name, $alias, $leftPreserving, unindexed: $plan->unindexed); + $leftProjected = $this->configureFindBuilder( + $left, + $collectionDoc, + $leftQueries, + $plan->joinTablePrefixes, + $plan->hasAggregation, + $plan->hasDistinct, + $plan->adapterFilterQueries, + $name, + $alias, + $roles, + $forPermission, + orderAttributes: $orderAttributes, + ); + $this->applyFullOuterJoinOrderProjection( + $left, + $collectionDoc, + $alias, + $orderAttributes, + $orderTypes, + $leftProjected, + $plan->joinTablePrefixes, + ); + $this->applyFindCursor( + $left, + $orderAttributes, + $orderTypes, + $cursor, + $cursorDirection, + $resolveInternalKey, + nullable: true, + ); + + $right = $this->newBuilder($name, $alias, true, unindexed: $plan->unindexed); + $rightProjected = $this->configureFindBuilder( + $right, + $collectionDoc, + $rightQueries, + $plan->joinTablePrefixes, + $plan->hasAggregation, + $plan->hasDistinct, + $plan->adapterFilterQueries, + $name, + $alias, + $roles, + $forPermission, + orderAttributes: $orderAttributes, + ); + $this->applyFullOuterJoinOrderProjection( + $right, + $collectionDoc, + $alias, + $orderAttributes, + $orderTypes, + $rightProjected, + $plan->joinTablePrefixes, + ); + $this->applyFindCursor( + $right, + $orderAttributes, + $orderTypes, + $cursor, + $cursorDirection, + $resolveInternalKey, + nullable: true, + ); + + if ($plan->hasDistinct) { + $left->union($right); + } else { + $left->unionAll($right); + } + $this->applyFindPage($left, $orderAttributes, $orderTypes, $limit, $offset, $cursorDirection, afterUnion: true); + + return $left; + } + /** * Aggregate an emulated full outer join once, over the rows of both halves. Each half keeps its own * joins, filters, tenant and permission conditions and projects the columns the aggregation reads; @@ -5118,11 +5312,13 @@ private function quoteOrderColumn(string $key, string $alias): string * @param array $orderTypes * @param array $cursor * @param callable(string): string $resolveInternalKey - * @return array> + * @return array{SQLBuilder, array, array} The aggregation, the columns the halves + * project and the joined groups qualified + * by alias (see fullOuterJoinResultNames()) * * @throws DatabaseException */ - private function findFullOuterJoinAggregate( + private function fullOuterJoinAggregation( Document $collection, array $queries, array $joinTablePrefixes, @@ -5213,7 +5409,7 @@ private function findFullOuterJoinAggregate( } } - return $this->fullOuterJoinResultNames($this->executeSelect($aggregation, Event::DocumentFind, $name), $columns, $qualifiedGroups); + return [$aggregation, $columns, $qualifiedGroups]; } private function shapesAggregatedRows(Method $method): bool diff --git a/src/Database/Adapter/SQL/Plan.php b/src/Database/Adapter/SQL/Plan.php new file mode 100644 index 000000000..35e909c10 --- /dev/null +++ b/src/Database/Adapter/SQL/Plan.php @@ -0,0 +1,33 @@ + $queries Every query but the vector and adapter filter ones, with the join columns qualified + * @param array $vectorQueries + * @param array $adapterFilterQueries + * @param list $joinTablePrefixes + * @param list $unindexed The join aliases a join cannot look its rows up by an index for + */ + public function __construct( + public array $queries, + public array $vectorQueries, + public array $adapterFilterQueries, + public array $joinTablePrefixes, + public array $unindexed, + public bool $hasAggregation, + public bool $hasJoins, + public bool $hasDistinct, + public bool $hasPreservingOuterJoin, + public bool $emulatesFullOuterJoin, + ) { + } +} diff --git a/src/Database/Trait/Documents.php b/src/Database/Trait/Documents.php index 03f030f1b..724cb2a00 100644 --- a/src/Database/Trait/Documents.php +++ b/src/Database/Trait/Documents.php @@ -4245,9 +4245,6 @@ private function fetch(Collection $collection, array $queries, PermissionType $f $parsed = Query::groupByType($queries); $filters = $parsed->filters; $selects = $parsed->selections; - $aggregations = $parsed->aggregations; - $groupByAttrs = $parsed->groupBy; - $having = $parsed->having; $joins = $parsed->joins; // Skipping authorization would also skip the joined collections' permission filters, // so with joins the main collection's grant travels to the adapter instead. @@ -4268,19 +4265,7 @@ private function fetch(Collection $collection, array $queries, PermissionType $f : 'find() does not run aggregate or groupBy queries: use aggregate()'); } - if ($isAggregation && ! $this->adapter->supports(Capability::Aggregations)) { - throw new QueryException('Aggregation queries are not supported by this adapter'); - } - - if ($distinct && ! $this->adapter->supports(Capability::Aggregations)) { - throw new QueryException('Distinct queries are not supported by this adapter'); - } - - if (! empty($joins) && ! $this->adapter->supports(Capability::Joins)) { - throw new QueryException('Join queries are not supported by this adapter'); - } - - $this->assertJoinCount($joins); + $this->assertReadable($parsed); $joinedCollectionsById ??= $this->resolveJoinedCollections($joins); $joinDocumentSecurity = []; @@ -4348,20 +4333,7 @@ private function fetch(Collection $collection, array $queries, PermissionType $f $outerJoinIds = $distinct ? [] : $this->outerJoinIdSelections($selects, $joins, $joinedCollections); - /** @var array $queries */ - $queries = \array_merge( - $selects, - $outerJoinIds === [] ? [] : [Query::select($outerJoinIds)], - $this->convertQueries($collection, \array_merge($filters, $aggregations, $having, $joins), $joinedByAlias), - ); - - if (! empty($groupByAttrs)) { - $queries[] = Query::groupBy($groupByAttrs); - } - - if ($distinct) { - $queries[] = Query::distinct(); - } + $queries = $this->readQueries($collection, $parsed, $joinedByAlias, $outerJoinIds); $selections = $this->validateSelections($collection, $selects); @@ -4446,7 +4418,7 @@ private function fetch(Collection $collection, array $queries, PermissionType $f } if (! isset($results)) { - $adapterCollection = $this->withJoinIndexed($this->withJoinAttributes($this->withJoinAuthorization($collection, $joinDocumentSecurity, $collectionGranted), $joins, $joinedCollectionsById), $joins, $joinedCollectionsById); + $adapterCollection = $this->readCollection($collection, $joins, $joinedCollectionsById, $joinDocumentSecurity, $collectionGranted); $find = fn (): array => $this->adapter->find( $adapterCollection, @@ -4556,15 +4528,17 @@ public function findOne(string $collection, array $queries = []): Document /** * Count Documents * - * Count the number of documents matching the given queries. + * Count the number of documents matching the given queries, or with aggregates, groupBy, having or distinct, the + * number of rows aggregate() or find() returns for them before ordering and paging. * * @param string $collection The collection identifier - * @param array $queries Queries for filtering - * @param int|null $max The most documents to count, greater than 0, or null for every match - * @return int The document count + * @param array $queries Queries for filtering, joining, aggregating, grouping and distinct; order, limit, + * offset and cursor queries are ignored + * @param int|null $max The most documents or rows to count, greater than 0, or null for every match + * @return int The document or row count * * @throws DatabaseException - * @throws QueryException When $max is not greater than 0 + * @throws QueryException When $max is not greater than 0, or the adapter cannot read the queries */ public function count(string $collection, array $queries = [], ?int $max = null): int { @@ -4590,18 +4564,24 @@ public function count(string $collection, array $queries = [], ?int $max = null) throw new AuthorizationException($this->authorization->getDescription()); } - $relationships = self::relationshipAttributes($collection); + $parsed = Query::groupByType($queries); + if (self::shapesRows($parsed)) { + if ($this->validation()->get()) { + $this->validateRelationshipSelects($collection, $queries); + } - $prepared = $this->prepareFilterJoinQueries($collection, $queries, $relationships, $collectionGranted, $joinedCollections); - if ($prepared === null) { - return 0; + $count = $this->readRows( + $collection, + Query::groupByType(self::aliasAggregates($queries)), + $collectionGranted, + $joinedCollections, + fn (Document $adapterCollection, array $adapterQueries): int => $this->adapter->count($adapterCollection, $adapterQueries, $max), + 0, + ); + } else { + $count = $this->countDocuments($collection, $queries, $collectionGranted, $joinedCollections, $max); } - [$collection, $queries, $skipAuth] = $prepared; - - $getCount = fn () => $this->adapter->count($collection, $queries, $max); - $count = $skipAuth ? $this->authorization->skip($getCount) : $getCount(); - $listeners = $this->listens(Event::DocumentCount); if ($listeners !== []) { $this->dispatch(new Event\Document\Counted($collection->getId(), $count), $listeners); @@ -4610,20 +4590,175 @@ public function count(string $collection, array $queries = [], ?int $max = null) return $count; } + /** + * @param array $queries + * @param array|null $joinedCollections The collection each join names, by its id + * + * @throws DatabaseException + * @throws QueryException + */ + private function countDocuments(Collection $collection, array $queries, bool $collectionGranted, ?array $joinedCollections, ?int $max): int + { + $prepared = $this->prepareFilterJoinQueries($collection, $queries, self::relationshipAttributes($collection), $collectionGranted, $joinedCollections); + if ($prepared === null) { + return 0; + } + + [$adapterCollection, $adapterQueries, $skipAuth] = $prepared; + + $count = fn (): int => $this->adapter->count($adapterCollection, $adapterQueries, $max); + + return $skipAuth ? $this->authorization->skip($count) : $count(); + } + + /** + * @param array $queries + * @param array|null $joinedCollections The collection each join names, by its id + * + * @throws DatabaseException + * @throws QueryException + */ + private function sumDocuments(Collection $collection, string $attribute, array $queries, bool $collectionGranted, ?array $joinedCollections, ?int $max): int|float + { + $prepared = $this->prepareFilterJoinQueries($collection, $queries, self::relationshipAttributes($collection), $collectionGranted, $joinedCollections); + if ($prepared === null) { + return 0; + } + + [$adapterCollection, $adapterQueries, $skipAuth] = $prepared; + + $sum = fn (): int|float => $this->adapter->sum($adapterCollection, $attribute, $adapterQueries, $max); + + return $skipAuth ? $this->authorization->skip($sum) : $sum(); + } + + /** + * Reads the rows of the statement aggregate() or find() reads for the queries, built and authorized as they + * build and authorize it, without its order and page. + * + * @template T of int|float + * + * @param array|null $joinedCollections The collection each join names, by its id + * @param Closure(Document, array): T $read Reads the rows of the adapter's collection and queries + * @param T $none What the read gives when no row can match + * @return T + * + * @throws DatabaseException + * @throws QueryException + */ + private function readRows(Collection $collection, ParsedQuery $parsed, bool $collectionGranted, ?array $joinedCollections, Closure $read, int|float $none): int|float + { + $this->assertReadable($parsed); + + $joins = $parsed->joins; + $joinedCollections ??= $this->resolveJoinedCollections($joins); + $joinDocumentSecurity = $joins === [] ? [] : $this->authorizeJoins($joins, PermissionType::Read, $joinedCollections); + + $queries = $this->readQueries($collection, $parsed, $this->joinedCollectionsByAlias($joins, $joinedCollections), []); + + if (! $parsed->isAggregation() && $this->relationshipHook !== null) { + $queries = $this->relationshipHook->convertQueries(self::relationshipAttributes($collection), $queries, $collection); + if ($queries === null) { + return $none; + } + } + + $adapterCollection = $this->readCollection($collection, $joins, $joinedCollections, $joinDocumentSecurity, $collectionGranted); + + $rows = fn (): int|float => $read($adapterCollection, $queries); + + return $collectionGranted && $joins === [] ? $this->authorization->skip($rows) : $rows(); + } + + /** + * Whether the queries read rows other than one per matching document: groups, aggregates or distinct values. + */ + private static function shapesRows(ParsedQuery $parsed): bool + { + return $parsed->isAggregation() || $parsed->distinct; + } + + /** + * @throws QueryException When the adapter cannot read the aggregates, distinct or joins the queries hold + */ + private function assertReadable(ParsedQuery $parsed): void + { + if ($parsed->isAggregation() && ! $this->adapter->supports(Capability::Aggregations)) { + throw new QueryException('Aggregation queries are not supported by this adapter'); + } + + if ($parsed->distinct && ! $this->adapter->supports(Capability::Aggregations)) { + throw new QueryException('Distinct queries are not supported by this adapter'); + } + + if ($parsed->joins !== [] && ! $this->adapter->supports(Capability::Joins)) { + throw new QueryException('Join queries are not supported by this adapter'); + } + + $this->assertJoinCount($parsed->joins); + } + + /** + * The queries the adapter reads for aggregate() and find(), and for count() over the same rows. + * + * @param array $joinedByAlias The collection each join alias reads + * @param list $outerJoinIds The joined ids selected to tell an unmatched outer row (see outerJoinIdSelections()) + * @return array + * + * @throws QueryException + */ + private function readQueries(Collection $collection, ParsedQuery $parsed, array $joinedByAlias, array $outerJoinIds): array + { + $queries = \array_merge( + $parsed->selections, + $outerJoinIds === [] ? [] : [Query::select($outerJoinIds)], + $this->convertQueries($collection, \array_merge($parsed->filters, $parsed->aggregations, $parsed->having, $parsed->joins), $joinedByAlias), + ); + + if ($parsed->groupBy !== []) { + $queries[] = Query::groupBy($parsed->groupBy); + } + + if ($parsed->distinct) { + $queries[] = Query::distinct(); + } + + return $queries; + } + + /** + * The collection handed to the adapter for a read, carrying what it needs to authorize and qualify each join. + * + * @param array $joins + * @param array $joinedCollections The collection each join names, by its id + * @param array $joinDocumentSecurity + */ + private function readCollection(Collection $collection, array $joins, array $joinedCollections, array $joinDocumentSecurity, bool $collectionGranted): Document + { + return $this->withJoinIndexed( + $this->withJoinAttributes($this->withJoinAuthorization($collection, $joinDocumentSecurity, $collectionGranted), $joins, $joinedCollections), + $joins, + $joinedCollections, + ); + } + /** * Sum an attribute * - * Sum an attribute for all matching documents. + * Sum an attribute for all matching documents, or with groupBy or distinct, over the rows aggregate() or find() + * returns for them before ordering and paging, where the attribute is one of their groups or selections. * * @param string $collection The collection identifier * @param string $attribute The attribute to sum - * @param array $queries Queries for filtering - * @param int|null $max The most documents to include in the sum, greater than 0, or null for every match + * @param array $queries Queries for filtering, joining, grouping and distinct; order, limit, offset and + * cursor queries are ignored + * @param int|null $max The most documents or rows to include in the sum, greater than 0, or null for every match * @return float|int The sum of the attribute values * * @throws DatabaseException * @throws LimitException When the sum overflows the range of a double - * @throws QueryException When $max is not greater than 0 + * @throws QueryException When $max is not greater than 0, the adapter cannot read the queries, or the attribute is + * not a column of the grouped or distinct rows */ public function sum(string $collection, string $attribute, array $queries = [], ?int $max = null): float|int { @@ -4657,17 +4792,24 @@ public function sum(string $collection, string $attribute, array $queries = [], throw new AuthorizationException($this->authorization->getDescription()); } - $relationships = self::relationshipAttributes($collection); + $parsed = Query::groupByType($queries); + if (self::shapesRows($parsed)) { + if ($this->validation()->get()) { + $this->validateRelationshipSelects($collection, $queries); + } - $prepared = $this->prepareFilterJoinQueries($collection, $queries, $relationships, $collectionGranted, $joinedCollections); - if ($prepared === null) { - return 0; + $sum = $this->readRows( + $collection, + Query::groupByType(self::aliasAggregates($queries)), + $collectionGranted, + $joinedCollections, + fn (Document $adapterCollection, array $adapterQueries): int|float => $this->adapter->sum($adapterCollection, $attribute, $adapterQueries, $max), + 0, + ); + } else { + $sum = $this->sumDocuments($collection, $attribute, $queries, $collectionGranted, $joinedCollections, $max); } - [$collection, $queries, $skipAuth] = $prepared; - - $getSum = fn () => $this->adapter->sum($collection, $attribute, $queries, $max); - $sum = $skipAuth ? $this->authorization->skip($getSum) : $getSum(); self::assertFinite($sum); $listeners = $this->listens(Event::DocumentSum); diff --git a/tests/e2e/Adapter/Scopes/AggregationTests.php b/tests/e2e/Adapter/Scopes/AggregationTests.php index b9cc4578b..6f243f3af 100644 --- a/tests/e2e/Adapter/Scopes/AggregationTests.php +++ b/tests/e2e/Adapter/Scopes/AggregationTests.php @@ -23,6 +23,7 @@ use Utopia\Database\Query; use Utopia\Database\Relationship; use Utopia\Database\Role; +use Utopia\Query\Method; trait AggregationTests { @@ -2823,4 +2824,203 @@ private function pageOneByOne(Database $database, string $collection, Query $sea return $documents; } + + public function testCountMatchesTheRowsOfGroupedDistinctAndAggregateReads(): void + { + $database = static::getDatabase(); + [$products, $reviews] = $this->createCountedRowsCollections($database); + + $join = Query::join($reviews, 'r', [Query::on('$id', 'product')]); + $leftJoin = Query::leftJoin($reviews, 'r', [Query::on('$id', 'product')]); + $fullOuterJoin = Query::fullOuterJoin($reviews, 'r', [Query::on('$id', 'product')]); + + $shapes = [ + 'groupBy' => [[Query::count('*', 'n'), Query::groupBy(['category'])], 4, 3], + 'groupBy with having' => [[Query::count('*', 'n'), Query::groupBy(['category']), Query::having([Query::greaterThan('n', 1)])], 3, 2], + 'groupBy with a filter' => [[Query::lessThan('price', 100), Query::count('*', 'n'), Query::groupBy(['category'])], 3, 2], + 'distinct' => [[Query::distinct(), Query::select(['category'])], 4, 3], + 'plain aggregate' => [[Query::sum('price', 'total')], 1, 1], + 'unaliased aggregate' => [[Query::count('*')], 1, 1], + 'left join with groupBy' => [[$leftJoin, Query::count('*', 'n'), Query::groupBy(['category'])], 4, 3], + 'join with groupBy' => [[$join, Query::count('*', 'n'), Query::groupBy(['category'])], 4, 2], + 'distinct over a join' => [[$join, Query::distinct(), Query::select(['category'])], 4, 2], + 'distinct joined attribute' => [[$join, Query::distinct(), Query::select(['r.score'])], 5, 2], + 'full outer join with groupBy' => [[$fullOuterJoin, Query::count('*', 'n'), Query::groupBy(['r.score'])], null, null], + 'full outer join with distinct' => [[$fullOuterJoin, Query::distinct(), Query::select(['category'])], null, null], + ]; + + $documents = [ + 'plain' => [[], 10, 5], + 'filtered' => [[Query::lessThan('price', 100)], 6, 3], + 'join' => [[$join], 6, 3], + ]; + + $authorization = $database->getAuthorization(); + + try { + foreach (['admin' => 1, 'viewer' => 2] as $role => $column) { + $authorization->cleanRoles(); + $authorization->addRole(Role::user($role)->toString()); + + foreach ($documents as $label => [$queries, $expectedAdmin, $expectedViewer]) { + if ($queries !== [] && $queries[0]->getMethod()->isJoin() && ! $database->getAdapter()->supports(Capability::Joins)) { + continue; + } + + $expected = $column === 1 ? $expectedAdmin : $expectedViewer; + $this->assertSame($expected, $database->count($products, $queries), "{$label} as {$role}"); + $this->assertCount($expected, $database->find($products, [...$queries, Query::limit(100)]), "{$label} as {$role}"); + } + + foreach ($shapes as $label => [$queries, $expectedAdmin, $expectedViewer]) { + $needsJoins = \array_filter($queries, static fn (Query $query): bool => $query->getMethod()->isJoin()) !== []; + if (! $database->getAdapter()->supports(Capability::Aggregations) || ($needsJoins && ! $database->getAdapter()->supports(Capability::Joins))) { + $this->assertCountRefused(fn (): int => $database->count($products, $queries), "{$label} as {$role}"); + + continue; + } + + $isAggregation = \array_filter($queries, static fn (Query $query): bool => $query->getMethod()->isAggregate() || $query->getMethod() === Method::GroupBy) !== []; + $rows = $isAggregation + ? \count($database->aggregate($products, [...$queries, Query::limit(100)])) + : \count($database->find($products, [...$queries, Query::limit(100)])); + + $count = $database->count($products, $queries); + $this->assertSame($rows, $count, "{$label} as {$role}: count() and the rows the read returns"); + $this->assertSame($rows, $database->count($products, [...$queries, Query::limit(1), Query::offset(1)]), "{$label} as {$role}: the page is not counted"); + + $expected = $column === 1 ? $expectedAdmin : $expectedViewer; + if ($expected !== null) { + $this->assertSame($expected, $count, "{$label} as {$role}"); + } else { + $this->assertGreaterThan(0, $count, "{$label} as {$role}"); + } + + $this->assertSame(\min(2, $rows), $database->count($products, $queries, 2), "{$label} as {$role}: capped at max"); + } + } + } finally { + $authorization->cleanRoles(); + $authorization->addRole('any'); + $this->cleanupAggCollections($database, [$products, $reviews]); + } + } + + public function testSumAddsUpTheRowsOfGroupedAndDistinctReads(): void + { + $database = static::getDatabase(); + [$products, $reviews] = $this->createCountedRowsCollections($database); + + $join = Query::join($reviews, 'r', [Query::on('$id', 'product')]); + + $shapes = [ + 'groupBy' => [$reviews, 'score', [Query::count('*', 'n'), Query::groupBy(['score'])], 15, 12], + 'distinct' => [$reviews, 'score', [Query::distinct(), Query::select(['score'])], 15, 12], + 'distinct joined attribute' => [$products, 'r.score', [$join, Query::distinct(), Query::select(['r.score'])], 15, 9], + ]; + + $authorization = $database->getAuthorization(); + + try { + foreach (['admin' => 3, 'viewer' => 4] as $role => $column) { + $authorization->cleanRoles(); + $authorization->addRole(Role::user($role)->toString()); + + $this->assertSame($column === 3 ? 20 : 17, $database->sum($reviews, 'score'), "every review as {$role}"); + + foreach ($shapes as $label => $shape) { + [$collection, $attribute, $queries] = $shape; + $needsJoins = $collection === $products; + if (! $database->getAdapter()->supports(Capability::Aggregations) || ($needsJoins && ! $database->getAdapter()->supports(Capability::Joins))) { + $this->assertCountRefused(fn (): int|float => $database->sum($collection, $attribute, $queries), "{$label} as {$role}"); + + continue; + } + + $this->assertEquals($shape[$column], $database->sum($collection, $attribute, $queries), "{$label} as {$role}"); + $this->assertEquals($shape[$column], $database->sum($collection, $attribute, [...$queries, Query::limit(1), Query::offset(1)]), "{$label} as {$role}: the page is not summed"); + } + + if ($database->getAdapter()->supports(Capability::Aggregations)) { + $this->assertCountRefused(fn (): int|float => $database->sum($reviews, 'score', [Query::count('*', 'n'), Query::groupBy(['product'])]), "an attribute the groups do not hold as {$role}"); + } + } + } finally { + $authorization->cleanRoles(); + $authorization->addRole('any'); + $this->cleanupAggCollections($database, [$products, $reviews]); + } + } + + private function assertCountRefused(callable $count, string $label): void + { + $error = null; + try { + $count(); + } catch (Throwable $caught) { + $error = $caught; + } + + $this->assertInstanceOf(QueryException::class, $error, $label.': '.($error === null ? 'counted' : $error::class.': '.$error->getMessage())); + } + + /** + * Products and their reviews, each readable by `user:admin` through the collection and by `user:viewer` only + * where the document grants it. + * + * @return array{string, string} + */ + private function createCountedRowsCollections(Database $database): array + { + $products = 'counted_rows_products'; + $reviews = 'counted_rows_reviews'; + $this->cleanupAggCollections($database, [$products, $reviews]); + + $permissions = [Permission::create(Role::any()), Permission::read(Role::user('admin'))]; + $viewer = [Permission::read(Role::user('viewer'))]; + + $database->createCollection(Collection::create(id: $products, attributes: [ + Attribute::string(key: 'category', size: 50, required: true), + Attribute::integer(key: 'price', required: true), + ], permissions: $permissions, documentSecurity: true)); + $database->createCollection(Collection::create(id: $reviews, attributes: [ + Attribute::string(key: 'product', size: 36, required: true), + Attribute::integer(key: 'score', required: true), + ], permissions: $permissions, documentSecurity: true)); + + $rows = [ + $products => [ + 'p1' => [['category' => 'electronics', 'price' => 1200], true], + 'p2' => [['category' => 'electronics', 'price' => 800], true], + 'p3' => [['category' => 'electronics', 'price' => 500], false], + 'p4' => [['category' => 'clothing', 'price' => 30], true], + 'p5' => [['category' => 'clothing', 'price' => 50], false], + 'p6' => [['category' => 'clothing', 'price' => 120], false], + 'p7' => [['category' => 'books', 'price' => 15], true], + 'p8' => [['category' => 'books', 'price' => 60], true], + 'p9' => [['category' => 'books', 'price' => 10], false], + 'p10' => [['category' => 'toys', 'price' => 25], false], + ], + $reviews => [ + 'r1' => [['product' => 'p1', 'score' => 5], true], + 'r2' => [['product' => 'p1', 'score' => 4], true], + 'r3' => [['product' => 'p4', 'score' => 3], false], + 'r4' => [['product' => 'p7', 'score' => 5], true], + 'r5' => [['product' => 'p3', 'score' => 2], true], + 'r6' => [['product' => 'p10', 'score' => 1], true], + ], + ]; + + foreach ($rows as $collection => $documents) { + foreach ($documents as $id => [$attributes, $visible]) { + $database->createDocument($collection, new Document([ + '$id' => $id, + ...$attributes, + '$permissions' => $visible ? $viewer : [], + ])); + } + } + + return [$products, $reviews]; + } } diff --git a/tests/unit/RowCountTest.php b/tests/unit/RowCountTest.php new file mode 100644 index 000000000..2499f473d --- /dev/null +++ b/tests/unit/RowCountTest.php @@ -0,0 +1,228 @@ +, bool}>> + */ + private const array ROWS = [ + self::PRODUCTS => [ + 'p1' => [['category' => 'electronics', 'price' => 1200], true], + 'p2' => [['category' => 'electronics', 'price' => 800], true], + 'p3' => [['category' => 'electronics', 'price' => 500], false], + 'p4' => [['category' => 'clothing', 'price' => 30], true], + 'p5' => [['category' => 'clothing', 'price' => 50], false], + 'p6' => [['category' => 'clothing', 'price' => 120], false], + 'p7' => [['category' => 'books', 'price' => 15], true], + 'p8' => [['category' => 'books', 'price' => 60], true], + 'p9' => [['category' => 'books', 'price' => 10], false], + 'p10' => [['category' => 'toys', 'price' => 25], false], + ], + self::REVIEWS => [ + 'r1' => [['product' => 'p1', 'score' => 5], true], + 'r2' => [['product' => 'p1', 'score' => 4], true], + 'r3' => [['product' => 'p4', 'score' => 3], false], + 'r4' => [['product' => 'p7', 'score' => 5], true], + 'r5' => [['product' => 'p3', 'score' => 2], true], + 'r6' => [['product' => 'p10', 'score' => 1], true], + ], + ]; + + /** + * @return array, int, int}> + */ + public static function shapes(): array + { + $join = Query::join(self::REVIEWS, 'r', [Query::on('$id', 'product')]); + $leftJoin = Query::leftJoin(self::REVIEWS, 'r', [Query::on('$id', 'product')]); + $fullOuterJoin = Query::fullOuterJoin(self::REVIEWS, 'r', [Query::on('$id', 'product')]); + + return [ + 'groupBy' => [[Query::count('*', 'n'), Query::groupBy(['category'])], 4, 3], + 'groupBy with having' => [[Query::count('*', 'n'), Query::groupBy(['category']), Query::having([Query::greaterThan('n', 1)])], 3, 2], + 'distinct' => [[Query::distinct(), Query::select(['category'])], 4, 3], + 'plain aggregate' => [[Query::sum('price', 'total')], 1, 1], + 'left join with groupBy' => [[$leftJoin, Query::count('*', 'n'), Query::groupBy(['category'])], 4, 3], + 'join with groupBy' => [[$join, Query::count('*', 'n'), Query::groupBy(['category'])], 4, 2], + 'distinct over a join' => [[$join, Query::distinct(), Query::select(['category'])], 4, 2], + 'distinct joined attribute' => [[$join, Query::distinct(), Query::select(['r.score'])], 5, 2], + 'emulated full outer join with groupBy' => [[$fullOuterJoin, Query::count('*', 'n'), Query::groupBy(['category'])], 4, 4], + 'emulated full outer join with distinct' => [[$fullOuterJoin, Query::distinct(), Query::select(['r.score'])], 6, 5], + ]; + } + + /** + * @param list $queries + */ + #[DataProvider('shapes')] + public function testCountIsTheNumberOfRowsTheReadReturns(array $queries, int $admin, int $viewer): void + { + $database = $this->database(); + + foreach ([self::ADMIN => $admin, self::VIEWER => $viewer] as $role => $expected) { + $this->actAs($database, $role); + + $this->assertSame($expected, $this->rowCount($database, $queries), $role); + $this->assertSame($expected, $database->count(self::PRODUCTS, $queries), $role); + $this->assertSame($expected, $database->count(self::PRODUCTS, [...$queries, Query::limit(1), Query::offset(1)]), $role.': the page is not counted'); + $this->assertSame(\min(2, $expected), $database->count(self::PRODUCTS, $queries, 2), $role.': capped at max'); + } + } + + public function testDocumentCountsAreUnchanged(): void + { + $database = $this->database(); + $join = Query::join(self::REVIEWS, 'r', [Query::on('$id', 'product')]); + + foreach ([self::ADMIN => [10, 6, 6], self::VIEWER => [5, 3, 3]] as $role => [$all, $cheap, $joined]) { + $this->actAs($database, $role); + + $this->assertSame($all, $database->count(self::PRODUCTS), $role); + $this->assertSame($cheap, $database->count(self::PRODUCTS, [Query::lessThan('price', 100)]), $role); + $this->assertSame($joined, $database->count(self::PRODUCTS, [$join]), $role); + } + } + + public function testSumAddsUpTheGroupedOrDistinctRows(): void + { + $database = $this->database(); + $join = Query::join(self::REVIEWS, 'r', [Query::on('$id', 'product')]); + + foreach ([self::ADMIN => [20, 15, 15], self::VIEWER => [17, 12, 9]] as $role => [$every, $distinct, $joined]) { + $this->actAs($database, $role); + + $this->assertSame($every, $database->sum(self::REVIEWS, 'score'), $role); + $this->assertSame($distinct, $database->sum(self::REVIEWS, 'score', [Query::count('*', 'n'), Query::groupBy(['score'])]), $role); + $this->assertSame($distinct, $database->sum(self::REVIEWS, 'score', [Query::distinct(), Query::select(['score'])]), $role); + $this->assertSame($joined, $database->sum(self::PRODUCTS, 'r.score', [$join, Query::distinct(), Query::select(['r.score'])]), $role); + } + } + + public function testSumOfAnAttributeTheGroupsDoNotHoldIsAnInvalidQuery(): void + { + $database = $this->database(); + + $this->expectException(QueryException::class); + $this->expectExceptionMessage('Invalid query: sum() over grouped rows adds up one of their groupBy attributes, and score is not one of them'); + + $database->sum(self::REVIEWS, 'score', [Query::count('*', 'n'), Query::groupBy(['product'])]); + } + + public function testAnAdapterWithoutAggregatesRefusesToCountThemAsItRefusesToReadThem(): void + { + $database = new Database(new Memory(), new Cache(new None())); + $database + ->setAuthorization(new Authorization()) + ->setDatabase('rows') + ->setNamespace('rows_'.\uniqid()) + ->setValidation(false); + $database->create(); + $database->createCollection(Collection::create( + id: self::PRODUCTS, + attributes: [Attribute::string(key: 'category', size: 50, required: true)], + permissions: [Permission::create(Role::any()), Permission::read(Role::any())], + )); + $database->createDocument(self::PRODUCTS, new Document(['category' => 'books'])); + $database->createDocument(self::PRODUCTS, new Document(['category' => 'books'])); + + foreach ([ + 'groupBy' => [Query::count('*', 'n'), Query::groupBy(['category'])], + 'distinct' => [Query::distinct(), Query::select(['category'])], + ] as $label => $queries) { + $error = null; + try { + $database->count(self::PRODUCTS, $queries); + } catch (QueryException $caught) { + $error = $caught; + } + + $this->assertNotNull($error, $label.' was counted as documents'); + } + } + + /** + * @param list $queries + */ + private function rowCount(Database $database, array $queries): int + { + $aggregates = \array_filter($queries, static fn (Query $query): bool => $query->getMethod()->isAggregate() || $query->getMethod() === Method::GroupBy); + + return \count($aggregates === [] + ? $database->find(self::PRODUCTS, [...$queries, Query::limit(100)]) + : $database->aggregate(self::PRODUCTS, [...$queries, Query::limit(100)])); + } + + private function actAs(Database $database, string $role): void + { + $database->getAuthorization()->cleanRoles(); + $database->getAuthorization()->addRole(Role::user($role)->toString()); + } + + private function database(): Database + { + $database = new Database(new SQLite(new PDO('sqlite::memory:')), new Cache(new None())); + $database + ->setAuthorization(new Authorization()) + ->setDatabase('rows') + ->setNamespace('rows_'.\uniqid()); + $database->addHook(new Permissions()); + $database->create(); + + $permissions = [Permission::create(Role::any()), Permission::read(Role::user(self::ADMIN))]; + $database->createCollection(Collection::create(id: self::PRODUCTS, attributes: [ + Attribute::string(key: 'category', size: 50, required: true), + Attribute::integer(key: 'price', required: true), + ], permissions: $permissions, documentSecurity: true)); + $database->createCollection(Collection::create(id: self::REVIEWS, attributes: [ + Attribute::string(key: 'product', size: 36, required: true), + Attribute::integer(key: 'score', required: true), + ], permissions: $permissions, documentSecurity: true)); + + foreach (self::ROWS as $collection => $documents) { + foreach ($documents as $id => [$attributes, $visible]) { + $database->createDocument($collection, new Document([ + Document::ID => $id, + ...$attributes, + Document::PERMISSIONS => $visible ? [Permission::read(Role::user(self::VIEWER))] : [], + ])); + } + } + + return $database; + } +} From 4eda93a017dae57810084dabebea42e01e469f2a Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 15 Oct 2026 23:38:12 +1300 Subject: [PATCH 2/3] fix(sum): range-check a sum of grouped or distinct rows on mariadb and mysql sum() over grouped or distinct rows adds up a column of the derived table as a raw SUM(), which the dialect builder never compiles, so it missed the `+ 0` that #1018 puts on every floating aggregate. MariaDB and MySQL would answer such a sum that overflowed a double with 0 instead of raising 1690. The column can be a dotted alias such as `r.score`, which the builder would split into two identifiers, so the adapter wraps the expression itself through rangeChecked(): unchanged on PostgreSQL and SQLite, which already raise or return INF for Database::sum() to refuse, and `+ 0` on MariaDB and MySQL. Co-Authored-By: Claude Opus 5.5 --- src/Database/Adapter/MariaDB.php | 6 ++ src/Database/Adapter/SQL.php | 11 ++- tests/e2e/Adapter/Scopes/AggregationTests.php | 42 +++++++++ tests/unit/Adapter/RowSumStatementTest.php | 86 +++++++++++++++++++ tests/unit/SumOverflowTest.php | 37 ++++++++ 5 files changed, 181 insertions(+), 1 deletion(-) create mode 100644 tests/unit/Adapter/RowSumStatementTest.php diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index 04f9d826e..140b24864 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -1174,6 +1174,12 @@ protected function dialectBuilder(): SQLBuilder&Scoping return new MariaDBBuilder(); } + #[\Override] + protected function rangeChecked(string $expression): string + { + return $expression.' + 0'; + } + #[\Override] protected function boundsJoinedSort(): bool { diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index a0d3fbc82..704515dd0 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -1952,7 +1952,7 @@ public function sum(Document $collection, string $attribute, array $queries = [] $outerBuilder = $this->dialectBuilder(); $outerBuilder->fromSub($this->rowsBuilder($collectionDoc, $plan, $name, $alias, $roles, $max), 'table_count'); - $outerBuilder->selectRaw('SUM('.$this->quote($column).') AS '.$this->quote('sum')); + $outerBuilder->selectRaw($this->rangeChecked('SUM('.$this->quote($column).')').' AS '.$this->quote('sum')); return $this->sumOf($this->fetchAggregateRow($outerBuilder, Event::DocumentSum, $name)); } @@ -3457,6 +3457,15 @@ final public function builder(): SQLBuilder&Scoping */ abstract protected function dialectBuilder(): SQLBuilder&Scoping; + /** + * A floating expression the engine raises an error for when it overflows a double, rather than answer a wrong + * number. The dialect builder checks the aggregates it compiles the same way. + */ + protected function rangeChecked(string $expression): string + { + return $expression; + } + #[\Override] public function schema(): MySQLSchema|PostgresSchema { diff --git a/tests/e2e/Adapter/Scopes/AggregationTests.php b/tests/e2e/Adapter/Scopes/AggregationTests.php index 6f243f3af..61e58161b 100644 --- a/tests/e2e/Adapter/Scopes/AggregationTests.php +++ b/tests/e2e/Adapter/Scopes/AggregationTests.php @@ -2952,6 +2952,48 @@ public function testSumAddsUpTheRowsOfGroupedAndDistinctReads(): void } } + public function testSumOfGroupedOrDistinctRowsThatOverflowsIsRefused(): void + { + $database = static::getDatabase(); + if (! $database->getAdapter()->supports(Capability::Aggregations)) { + $this->expectNotToPerformAssertions(); + + return; + } + + $collection = 'agg_rows_overflow'.$this->getAggSuffix(); + $database->createCollection(Collection::create( + id: $collection, + attributes: [ + Attribute::string(key: 'series', size: 20, required: true), + Attribute::double(key: 'reading', required: true), + ], + permissions: [Permission::create(Role::any()), Permission::read(Role::any())], + )); + foreach ([['huge', 1e308], ['huge', 1e308], ['huge', 9e307], ['sunk', -1e308], ['sunk', -9e307], ['single', 1e308]] as [$series, $reading]) { + $database->createDocument($collection, new Document([ + 'series' => $series, + 'reading' => $reading, + '$permissions' => [Permission::read(Role::any())], + ])); + } + + foreach (['huge', 'sunk'] as $series) { + foreach ([[Query::count('*', 'n'), Query::groupBy(['reading'])], [Query::distinct(), Query::select(['reading'])]] as $shape) { + try { + $database->sum($collection, 'reading', [Query::equal('series', [$series]), ...$shape]); + $this->fail("A sum of {$series} grouped or distinct rows that overflows a double is refused"); + } catch (LimitException) { + $this->addToAssertionCount(1); + } + } + } + + $this->assertEqualsWithDelta(1e308, $database->sum($collection, 'reading', [Query::equal('series', ['single']), Query::distinct(), Query::select(['reading'])]), 1e293); + + $database->deleteCollection($collection); + } + private function assertCountRefused(callable $count, string $label): void { $error = null; diff --git a/tests/unit/Adapter/RowSumStatementTest.php b/tests/unit/Adapter/RowSumStatementTest.php new file mode 100644 index 000000000..a2bce5e65 --- /dev/null +++ b/tests/unit/Adapter/RowSumStatementTest.php @@ -0,0 +1,86 @@ + + */ + public static function adapters(): iterable + { + yield 'mariadb' => [static fn (PDO $pdo): SQL => new MariaDB($pdo), 'SUM(`reading`) + 0 AS `sum`']; + yield 'mysql' => [static fn (PDO $pdo): SQL => new MySQL($pdo), 'SUM(`reading`) + 0 AS `sum`']; + yield 'postgres' => [static fn (PDO $pdo): SQL => new Postgres($pdo), 'SUM("reading") AS "sum"']; + } + + /** + * @return iterable}> + */ + public static function shapes(): iterable + { + yield 'groupBy' => [[Query::count('*', 'n'), Query::groupBy(['reading'])]]; + yield 'distinct' => [[Query::distinct(), Query::select(['reading'])]]; + } + + /** + * @return iterable}> + */ + public static function sums(): iterable + { + foreach (self::adapters() as $adapter => [$create, $selection]) { + foreach (self::shapes() as $shape => [$queries]) { + yield "{$shape} on {$adapter}" => [$create, $selection, $queries]; + } + } + } + + /** + * @param Closure(PDO): SQL $create + * @param list $queries + */ + #[DataProvider('sums')] + public function testTheRowsAreAddedUpThroughTheRangeCheck(Closure $create, string $selection, array $queries): void + { + $statements = []; + $pdo = $this->createStub(PDO::class); + $pdo->method('prepare')->willReturnCallback(function (string $sql) use (&$statements): PDOStatement { + $statements[] = $sql; + $statement = $this->createStub(PDOStatement::class); + $statement->method('execute')->willReturn(true); + $statement->method('fetchAll')->willReturn([['sum' => '3']]); + $statement->method('closeCursor')->willReturn(true); + + return $statement; + }); + + $adapter = $create($pdo); + $adapter->setDatabase('database'); + $adapter->setNamespace('namespace'); + $adapter->setAuthorization(new Authorization()); + + $collection = Collection::create(id: 'readings', attributes: [Attribute::double(key: 'reading', required: true)]); + + $this->assertSame(3, $adapter->sum($collection, 'reading', $queries)); + $this->assertCount(1, $statements); + $this->assertStringStartsWith('SELECT '.$selection.' FROM (', $statements[0]); + } +} diff --git a/tests/unit/SumOverflowTest.php b/tests/unit/SumOverflowTest.php index d18b74982..04e5e4eaf 100644 --- a/tests/unit/SumOverflowTest.php +++ b/tests/unit/SumOverflowTest.php @@ -94,6 +94,43 @@ public function testASumThatOverflowsUnderFiltersIsRefused(Closure $adapter): vo $database->sum(self::COLLECTION, 'reading', [Query::greaterThan('reading', 0)], 10); } + /** + * @return iterable}> + */ + public static function rows(): iterable + { + foreach (['sqlite' => false, 'sqlite stringified' => true] as $label => $stringify) { + $adapter = static fn (): Adapter => new SQLite(new PDO('sqlite::memory:', null, null, [PDO::ATTR_STRINGIFY_FETCHES => $stringify])); + yield "groupBy on {$label}" => [$adapter, [Query::count('*', 'n'), Query::groupBy(['reading'])]]; + yield "distinct on {$label}" => [$adapter, [Query::distinct(), Query::select(['reading'])]]; + } + } + + /** + * @param Closure(): Adapter $adapter + * @param list $queries + */ + #[DataProvider('rows')] + public function testASumOfGroupedOrDistinctRowsThatOverflowsIsRefused(Closure $adapter, array $queries): void + { + $database = $this->database($adapter, [1e308, 1e308, 9e307]); + + $this->expectException(LimitException::class); + $database->sum(self::COLLECTION, 'reading', $queries); + } + + /** + * @param Closure(): Adapter $adapter + * @param list $queries + */ + #[DataProvider('rows')] + public function testTheLargestFiniteSumOfGroupedOrDistinctRowsIsStillAnswered(Closure $adapter, array $queries): void + { + $database = $this->database($adapter, [1e308, 1e308]); + + $this->assertSame(1e308, $database->sum(self::COLLECTION, 'reading', $queries)); + } + /** * @param Closure(): Adapter $adapter */ From 81d2fc0bf76a38f8e54bbce544583f049f610ee4 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 15 Oct 2026 23:38:12 +1300 Subject: [PATCH 3/3] test(query): count a distinct join without a select MySQL refuses a derived table with two columns of the same name. A distinct join without a select projects the main table's columns next to the joined `r._uid`, so the count holds it to keep those names apart on every adapter. Co-Authored-By: Claude Opus 5.5 --- tests/e2e/Adapter/Scopes/AggregationTests.php | 1 + tests/unit/RowCountTest.php | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/e2e/Adapter/Scopes/AggregationTests.php b/tests/e2e/Adapter/Scopes/AggregationTests.php index 61e58161b..40d457160 100644 --- a/tests/e2e/Adapter/Scopes/AggregationTests.php +++ b/tests/e2e/Adapter/Scopes/AggregationTests.php @@ -2845,6 +2845,7 @@ public function testCountMatchesTheRowsOfGroupedDistinctAndAggregateReads(): voi 'join with groupBy' => [[$join, Query::count('*', 'n'), Query::groupBy(['category'])], 4, 2], 'distinct over a join' => [[$join, Query::distinct(), Query::select(['category'])], 4, 2], 'distinct joined attribute' => [[$join, Query::distinct(), Query::select(['r.score'])], 5, 2], + 'distinct over a join without a select' => [[$join, Query::distinct()], 6, 3], 'full outer join with groupBy' => [[$fullOuterJoin, Query::count('*', 'n'), Query::groupBy(['r.score'])], null, null], 'full outer join with distinct' => [[$fullOuterJoin, Query::distinct(), Query::select(['category'])], null, null], ]; diff --git a/tests/unit/RowCountTest.php b/tests/unit/RowCountTest.php index 2499f473d..341e59e8a 100644 --- a/tests/unit/RowCountTest.php +++ b/tests/unit/RowCountTest.php @@ -81,6 +81,7 @@ public static function shapes(): array 'join with groupBy' => [[$join, Query::count('*', 'n'), Query::groupBy(['category'])], 4, 2], 'distinct over a join' => [[$join, Query::distinct(), Query::select(['category'])], 4, 2], 'distinct joined attribute' => [[$join, Query::distinct(), Query::select(['r.score'])], 5, 2], + 'distinct over a join without a select' => [[$join, Query::distinct()], 6, 3], 'emulated full outer join with groupBy' => [[$fullOuterJoin, Query::count('*', 'n'), Query::groupBy(['category'])], 4, 4], 'emulated full outer join with distinct' => [[$fullOuterJoin, Query::distinct(), Query::select(['r.score'])], 6, 5], ];