From 3692165414c23945bbc66f2a6ea9b69f9a326007 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Fri, 10 Jan 2025 13:09:36 +0100 Subject: [PATCH 1/3] PHPORM-286 Add Query::countByGroup and other aggregateByGroup functions --- src/Query/Builder.php | 28 +++++++++++++++++++++++++-- tests/QueryBuilderTest.php | 39 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/src/Query/Builder.php b/src/Query/Builder.php index 0e9e028bb..8e981b378 100644 --- a/src/Query/Builder.php +++ b/src/Query/Builder.php @@ -30,6 +30,7 @@ use Override; use RuntimeException; use stdClass; +use TypeError; use function array_fill_keys; use function array_filter; @@ -324,8 +325,10 @@ public function toMql(): array // this mimics SQL's behaviour a bit. $group[$column] = ['$last' => '$' . $column]; } + } - // Do the same for other columns that are selected. + // Add the last value of each column when there is no aggregate function. + if ($this->groups && ! $this->aggregate) { foreach ($columns as $column) { $key = str_replace('.', '_', $column); @@ -349,7 +352,7 @@ public function toMql(): array $aggregations = blank($this->aggregate['columns']) ? [] : $this->aggregate['columns']; - if (in_array('*', $aggregations) && $function === 'count') { + if (in_array('*', $aggregations) && $function === 'count' && empty($group['_id'])) { $options = $this->inheritConnectionOptions($this->options); return ['countDocuments' => [$wheres, $options]]; @@ -559,6 +562,8 @@ public function generateCacheKey() /** @return ($function is null ? AggregationBuilder : mixed) */ public function aggregate($function = null, $columns = ['*']) { + assert(is_array($columns), new TypeError(sprintf('Argument #2 ($columns) must be of type array, %s given', get_debug_type($columns)))); + if ($function === null) { if (! trait_exists(FluentFactoryTrait::class)) { // This error will be unreachable when the mongodb/builder package will be merged into mongodb/mongodb @@ -599,6 +604,15 @@ public function aggregate($function = null, $columns = ['*']) $this->columns = $previousColumns; $this->bindings['select'] = $previousSelectBindings; + // When the aggregation is per group, we return the results as is. + if ($this->groups) { + return $results->map(function (object $result) { + unset($result->id); + + return $result; + }); + } + if (isset($results[0])) { $result = (array) $results[0]; @@ -606,6 +620,16 @@ public function aggregate($function = null, $columns = ['*']) } } + /** + * {@inheritDoc} + * + * @see \Illuminate\Database\Query\Builder::aggregateByGroup() + */ + public function aggregateByGroup(string $function, array $columns = ['*']) + { + return $this->aggregate($function, $columns); + } + /** @inheritdoc */ public function exists() { diff --git a/tests/QueryBuilderTest.php b/tests/QueryBuilderTest.php index 136b1cf72..09d2bd1be 100644 --- a/tests/QueryBuilderTest.php +++ b/tests/QueryBuilderTest.php @@ -7,6 +7,7 @@ use Carbon\Carbon; use DateTime; use DateTimeImmutable; +use Illuminate\Support\Collection as LaravelCollection; use Illuminate\Support\Facades\Date; use Illuminate\Support\Facades\DB; use Illuminate\Support\LazyCollection; @@ -32,6 +33,7 @@ use function count; use function key; use function md5; +use function method_exists; use function sort; use function strlen; @@ -617,6 +619,43 @@ public function testSubdocumentArrayAggregate() $this->assertEquals(12, DB::table('items')->avg('amount.*.hidden')); } + public function testAggregateGroupBy() + { + DB::table('users')->insert([ + ['name' => 'John Doe', 'role' => 'admin', 'score' => 1], + ['name' => 'Jane Doe', 'role' => 'admin', 'score' => 2], + ['name' => 'Robert Roe', 'role' => 'user', 'score' => 4], + ]); + + $results = DB::table('users')->groupBy('role')->orderBy('role')->aggregateByGroup('count'); + $this->assertInstanceOf(LaravelCollection::class, $results); + $this->assertEquals([(object) ['role' => 'admin', 'aggregate' => 2], (object) ['role' => 'user', 'aggregate' => 1]], $results->toArray()); + + if (! method_exists(Builder::class, 'countByGroup')) { + $this->markTestSkipped('countBy* function require Laravel v11.38+'); + } + + $results = DB::table('users')->groupBy('role')->orderBy('role')->countByGroup(); + $this->assertInstanceOf(LaravelCollection::class, $results); + $this->assertEquals([(object) ['role' => 'admin', 'aggregate' => 2], (object) ['role' => 'user', 'aggregate' => 1]], $results->toArray()); + + $results = DB::table('users')->groupBy('role')->orderBy('role')->maxByGroup('score'); + $this->assertInstanceOf(LaravelCollection::class, $results); + $this->assertEquals([(object) ['role' => 'admin', 'aggregate' => 2], (object) ['role' => 'user', 'aggregate' => 4]], $results->toArray()); + + $results = DB::table('users')->groupBy('role')->orderBy('role')->minByGroup('score'); + $this->assertInstanceOf(LaravelCollection::class, $results); + $this->assertEquals([(object) ['role' => 'admin', 'aggregate' => 1], (object) ['role' => 'user', 'aggregate' => 4]], $results->toArray()); + + $results = DB::table('users')->groupBy('role')->orderBy('role')->sumByGroup('score'); + $this->assertInstanceOf(LaravelCollection::class, $results); + $this->assertEquals([(object) ['role' => 'admin', 'aggregate' => 3], (object) ['role' => 'user', 'aggregate' => 4]], $results->toArray()); + + $results = DB::table('users')->groupBy('role')->orderBy('role')->avgByGroup('score'); + $this->assertInstanceOf(LaravelCollection::class, $results); + $this->assertEquals([(object) ['role' => 'admin', 'aggregate' => 1.5], (object) ['role' => 'user', 'aggregate' => 4]], $results->toArray()); + } + public function testUpdateWithUpsert() { DB::table('items')->where('name', 'knife') From 694e76d0d0faa2850611633fc3201bab3fdd5f75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Mon, 13 Jan 2025 11:08:08 +0100 Subject: [PATCH 2/3] Support counting distinct values with aggregate by group --- src/Query/Builder.php | 22 +++++++++++++++++++--- tests/QueryBuilderTest.php | 22 +++++++++++++++++++--- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/Query/Builder.php b/src/Query/Builder.php index 8e981b378..b70b843a5 100644 --- a/src/Query/Builder.php +++ b/src/Query/Builder.php @@ -315,6 +315,7 @@ public function toMql(): array if ($this->groups || $this->aggregate) { $group = []; $unwinds = []; + $set = []; // Add grouping columns to the $group part of the aggregation pipeline. if ($this->groups) { @@ -352,15 +353,22 @@ public function toMql(): array $aggregations = blank($this->aggregate['columns']) ? [] : $this->aggregate['columns']; - if (in_array('*', $aggregations) && $function === 'count' && empty($group['_id'])) { + if ($column === '*' && $function === 'count' && ! $this->groups) { $options = $this->inheritConnectionOptions($this->options); return ['countDocuments' => [$wheres, $options]]; } + // "aggregate" is the name of the field that will hold the aggregated value. if ($function === 'count') { - // Translate count into sum. - $group['aggregate'] = ['$sum' => 1]; + if ($column === '*' || $aggregations === []) { + // Translate count into sum. + $group['aggregate'] = ['$sum' => 1]; + } else { + // Count the number of distinct values. + $group['aggregate'] = ['$addToSet' => '$' . $column]; + $set['aggregate'] = ['$size' => '$aggregate']; + } } else { $group['aggregate'] = ['$' . $function => '$' . $column]; } @@ -387,6 +395,10 @@ public function toMql(): array $pipeline[] = ['$group' => $group]; } + if ($set) { + $pipeline[] = ['$set' => $set]; + } + // Apply order and limit if ($this->orders) { $pipeline[] = ['$sort' => $this->aliasIdForQuery($this->orders)]; @@ -627,6 +639,10 @@ public function aggregate($function = null, $columns = ['*']) */ public function aggregateByGroup(string $function, array $columns = ['*']) { + if (count($columns) > 1) { + throw new InvalidArgumentException('Aggregating by group requires zero or one columns.'); + } + return $this->aggregate($function, $columns); } diff --git a/tests/QueryBuilderTest.php b/tests/QueryBuilderTest.php index 09d2bd1be..01f937915 100644 --- a/tests/QueryBuilderTest.php +++ b/tests/QueryBuilderTest.php @@ -622,8 +622,8 @@ public function testSubdocumentArrayAggregate() public function testAggregateGroupBy() { DB::table('users')->insert([ - ['name' => 'John Doe', 'role' => 'admin', 'score' => 1], - ['name' => 'Jane Doe', 'role' => 'admin', 'score' => 2], + ['name' => 'John Doe', 'role' => 'admin', 'score' => 1, 'active' => true], + ['name' => 'Jane Doe', 'role' => 'admin', 'score' => 2, 'active' => true], ['name' => 'Robert Roe', 'role' => 'user', 'score' => 4], ]); @@ -631,8 +631,16 @@ public function testAggregateGroupBy() $this->assertInstanceOf(LaravelCollection::class, $results); $this->assertEquals([(object) ['role' => 'admin', 'aggregate' => 2], (object) ['role' => 'user', 'aggregate' => 1]], $results->toArray()); + $results = DB::table('users')->groupBy('role')->orderBy('role')->aggregateByGroup('count', ['active']); + $this->assertInstanceOf(LaravelCollection::class, $results); + $this->assertEquals([(object) ['role' => 'admin', 'aggregate' => 1], (object) ['role' => 'user', 'aggregate' => 0]], $results->toArray()); + + $results = DB::table('users')->groupBy('role')->orderBy('role')->aggregateByGroup('max', ['score']); + $this->assertInstanceOf(LaravelCollection::class, $results); + $this->assertEquals([(object) ['role' => 'admin', 'aggregate' => 2], (object) ['role' => 'user', 'aggregate' => 4]], $results->toArray()); + if (! method_exists(Builder::class, 'countByGroup')) { - $this->markTestSkipped('countBy* function require Laravel v11.38+'); + $this->markTestSkipped('*byGroup functions require Laravel v11.38+'); } $results = DB::table('users')->groupBy('role')->orderBy('role')->countByGroup(); @@ -656,6 +664,14 @@ public function testAggregateGroupBy() $this->assertEquals([(object) ['role' => 'admin', 'aggregate' => 1.5], (object) ['role' => 'user', 'aggregate' => 4]], $results->toArray()); } + public function testAggregateByGroupException(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Aggregating by group requires zero or one columns.'); + + DB::table('users')->aggregateByGroup('max', ['foo', 'bar']); + } + public function testUpdateWithUpsert() { DB::table('items')->where('name', 'knife') From 7b22c62b7ef00adf7a0ab2c815535fd3fc1a5d86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Mon, 13 Jan 2025 11:26:25 +0100 Subject: [PATCH 3/3] Disable fail-fast due to Atlas issues --- .github/workflows/build-ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/build-ci.yml b/.github/workflows/build-ci.yml index 7a987d251..4fea1b84d 100644 --- a/.github/workflows/build-ci.yml +++ b/.github/workflows/build-ci.yml @@ -11,6 +11,8 @@ jobs: name: "PHP ${{ matrix.php }} Laravel ${{ matrix.laravel }} MongoDB ${{ matrix.mongodb }} ${{ matrix.mode }}" strategy: + # Tests with Atlas fail randomly + fail-fast: false matrix: os: - "ubuntu-latest"