Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHPORM-286 Add Query::countByGroup() and other aggregateByGroup() functions #3243

Merged
merged 3 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions src/Query/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
use Override;
use RuntimeException;
use stdClass;
use TypeError;

use function array_fill_keys;
use function array_filter;
Expand Down Expand Up @@ -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);

Expand All @@ -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'])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted that $group['_id'] is only set within the conditional for $this->groups above. I assume this is the code path for aggregateByGroup('count'), but it's not clear to me how aggregate() ends up calling toMql(). Does that happen via the call chain of get() and getFresh()?


Looking a few lines up (beyond the diff, as I couldn't comment there directly), I don't see $column used after it is assigned with implode('.', $splitColumns);. Is that dead code?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, aggregateByGroup calls aggregate, which calls get, which calls getFresh, which calls toMql.


This code is necessary, feature tested by testSubdocumentArrayAggregate;

$options = $this->inheritConnectionOptions($this->options);

return ['countDocuments' => [$wheres, $options]];
Expand Down Expand Up @@ -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
jmikola marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -599,13 +604,32 @@ 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);
jmikola marked this conversation as resolved.
Show resolved Hide resolved

return $result;
});
}

if (isset($results[0])) {
$result = (array) $results[0];

return $result['aggregate'];
}
}

/**
* {@inheritDoc}
*
* @see \Illuminate\Database\Query\Builder::aggregateByGroup()
*/
public function aggregateByGroup(string $function, array $columns = ['*'])
{
return $this->aggregate($function, $columns);
}

/** @inheritdoc */
public function exists()
{
Expand Down
39 changes: 39 additions & 0 deletions tests/QueryBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -32,6 +33,7 @@
use function count;
use function key;
use function md5;
use function method_exists;
use function sort;
use function strlen;

Expand Down Expand Up @@ -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');
jmikola marked this conversation as resolved.
Show resolved Hide resolved
$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+');
GromNaN marked this conversation as resolved.
Show resolved Hide resolved
}

$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')
Expand Down
Loading