From 5b7eab1c313fd8dbc3feba4e726dbe41bb8208e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Thu, 17 Oct 2024 22:22:46 +0200 Subject: [PATCH 1/4] Support aggregation by group --- src/Illuminate/Database/Query/Builder.php | 72 +++++++++++++++++++ .../Database/Query/Grammars/Grammar.php | 14 +++- tests/Database/DatabaseQueryBuilderTest.php | 69 ++++++++++++++++++ 3 files changed, 153 insertions(+), 2 deletions(-) diff --git a/src/Illuminate/Database/Query/Builder.php b/src/Illuminate/Database/Query/Builder.php index f166b28bbbfe..98fdc758a4fd 100755 --- a/src/Illuminate/Database/Query/Builder.php +++ b/src/Illuminate/Database/Query/Builder.php @@ -3561,6 +3561,18 @@ public function count($columns = '*') return (int) $this->aggregate(__FUNCTION__, Arr::wrap($columns)); } + /** + * Retrieve the "count" of the distinct results of a given column + * for each group. + * + * @param \Illuminate\Contracts\Database\Query\Expression|string $columns + * @return \Illuminate\Support\Collection + */ + public function countByGroup($columns = '*'): Collection + { + return $this->aggregateByGroup('count', Arr::wrap($columns)); + } + /** * Retrieve the minimum value of a given column. * @@ -3572,6 +3584,17 @@ public function min($column) return $this->aggregate(__FUNCTION__, [$column]); } + /** + * Retrieve the minimum value of a given column by group. + * + * @param \Illuminate\Contracts\Database\Query\Expression|string $column + * @return \Illuminate\Support\Collection + */ + public function minByGroup($column): Collection + { + return $this->aggregateByGroup('min', [$column]); + } + /** * Retrieve the maximum value of a given column. * @@ -3583,6 +3606,17 @@ public function max($column) return $this->aggregate(__FUNCTION__, [$column]); } + /** + * Retrieve the maximum value of a given column by group. + * + * @param \Illuminate\Contracts\Database\Query\Expression|string $column + * @return \Illuminate\Support\Collection + */ + public function maxByGroup($column): Collection + { + return $this->aggregateByGroup('max', [$column]); + } + /** * Retrieve the sum of the values of a given column. * @@ -3596,6 +3630,17 @@ public function sum($column) return $result ?: 0; } + /** + * Retrieve the sum of the values of a given column by group. + * + * @param \Illuminate\Contracts\Database\Query\Expression|string $column + * @return \Illuminate\Support\Collection + */ + public function sumByGroup($column): Collection + { + return $this->aggregateByGroup('sum', [$column]); + } + /** * Retrieve the average of the values of a given column. * @@ -3607,6 +3652,17 @@ public function avg($column) return $this->aggregate(__FUNCTION__, [$column]); } + /** + * Retrieve the average of the values of a given column by group. + * + * @param \Illuminate\Contracts\Database\Query\Expression|string $column + * @return \Illuminate\Support\Collection + */ + public function avgByGroup($column): Collection + { + return $this->aggregateByGroup('avg', [$column]); + } + /** * Alias for the "avg" method. * @@ -3637,6 +3693,22 @@ public function aggregate($function, $columns = ['*']) } } + /** + * Execute an aggregate function for each group and return the results + * in a collection of objects with the group columns and aggregate value. + * + * @param string $function The aggregate function to call + * @param array $columns The columns to perform the aggregate function on + * @return \Illuminate\Support\Collection + */ + public function aggregateByGroup(string $function, array $columns = ['*']): Collection + { + return $this->cloneWithout($this->unions || $this->havings ? [] : ['columns']) + ->cloneWithoutBindings($this->unions || $this->havings ? [] : ['select']) + ->setAggregate($function, $columns) + ->get($columns); + } + /** * Execute a numeric aggregate function on the database. * diff --git a/src/Illuminate/Database/Query/Grammars/Grammar.php b/src/Illuminate/Database/Query/Grammars/Grammar.php index 64e8f916d45c..cdc6d2c2e5d8 100755 --- a/src/Illuminate/Database/Query/Grammars/Grammar.php +++ b/src/Illuminate/Database/Query/Grammars/Grammar.php @@ -139,7 +139,15 @@ protected function compileAggregate(Builder $query, $aggregate) $column = 'distinct '.$column; } - return 'select '.$aggregate['function'].'('.$column.') as aggregate'; + $sql = 'select '; + + if ($query->groups) { + $sql .= $this->columnize($query->groups).', '; + } + + $sql .= $aggregate['function'].'('.$column.') as aggregate'; + + return $sql; } /** @@ -1131,10 +1139,12 @@ protected function wrapUnion($sql) protected function compileUnionAggregate(Builder $query) { $sql = $this->compileAggregate($query, $query->aggregate); + $groups = $query->groups ? ' '.$this->compileGroups($query, $query->groups) : ''; $query->aggregate = null; + $query->groups = null; - return $sql.' from ('.$this->compileSelect($query).') as '.$this->wrapTable('temp_table'); + return $sql.' from ('.$this->compileSelect($query).') as '.$this->wrapTable('temp_table').$groups; } /** diff --git a/tests/Database/DatabaseQueryBuilderTest.php b/tests/Database/DatabaseQueryBuilderTest.php index 096b82450d25..552fc60a047d 100755 --- a/tests/Database/DatabaseQueryBuilderTest.php +++ b/tests/Database/DatabaseQueryBuilderTest.php @@ -25,6 +25,7 @@ use Illuminate\Pagination\Cursor; use Illuminate\Pagination\CursorPaginator; use Illuminate\Pagination\LengthAwarePaginator; +use Illuminate\Support\Collection; use Illuminate\Tests\Database\Fixtures\Enums\Bar; use InvalidArgumentException; use Mockery as m; @@ -1804,6 +1805,36 @@ public function testGroupBys() $this->assertEquals(['whereRawBinding', 'groupByRawBinding', 'havingRawBinding'], $builder->getBindings()); } + public function testAggregateByGroup() + { + $builder = $this->getBuilder(); + + $queryResults = [['aggregate' => 2, 'role' => 'admin', 'city' => 'NY'], ['aggregate' => 5, 'role' => 'user', 'city' => 'LA']]; + $builder->getConnection() + ->shouldReceive('select')->once() + ->with('select "role", "city", count(*) as aggregate from "users" group by "role", "city"', [], true) + ->andReturn($queryResults); + $builder->getProcessor()->shouldReceive('processSelect')->once()->andReturnUsing(fn ($builder, $results) => $results); + $results = $builder->from('users')->groupBy('role', 'city')->aggregateByGroup('count'); + $this->assertEquals($queryResults, $results->toArray()); + } + + public function testUnionAndAggregateByGroup() + { + $builder = $this->getBuilder(); + + $queryResults = [['aggregate' => 2, 'role' => 'admin'], ['aggregate' => 5, 'role' => 'user']]; + $builder->getConnection() + ->shouldReceive('select')->once() + ->with('select "role", count(*) as aggregate from ((select * from "users") union (select * from "members")) as "temp_table" group by "role"', [], true) + ->andReturn($queryResults); + $builder->getProcessor()->shouldReceive('processSelect')->once()->andReturnUsing(fn ($builder, $results) => $results); + $results = $builder->from('users') + ->union($this->getBuilder()->select('*')->from('members')) + ->groupBy('role')->aggregateByGroup('count'); + $this->assertEquals($queryResults, $results->toArray()); + } + public function testOrderBys() { $builder = $this->getBuilder(); @@ -3464,6 +3495,44 @@ public function testAggregateFunctions() $this->assertEquals(1, $results); } + public function testAggregateFunctionsWithGroupBy() + { + $builder = $this->getBuilder(); + $builder->getConnection()->shouldReceive('select')->once()->with('select "role", count(*) as aggregate from "users" group by "role"', [], true)->andReturn([['role' => 'admin', 'aggregate' => 1]]); + $builder->getProcessor()->shouldReceive('processSelect')->once()->andReturnUsing(fn ($builder, $results) => $results); + $results = $builder->from('users')->groupBy('role')->countByGroup(); + $this->assertInstanceOf(Collection::class, $results); + $this->assertEquals([['role' => 'admin', 'aggregate' => 1]], $results->toArray()); + + $builder = $this->getBuilder(); + $builder->getConnection()->shouldReceive('select')->once()->with('select "role", max("id") as aggregate from "users" group by "role"', [], true)->andReturn([['role' => 'admin', 'aggregate' => 1]]); + $builder->getProcessor()->shouldReceive('processSelect')->once()->andReturnUsing(fn ($builder, $results) => $results); + $results = $builder->from('users')->groupBy('role')->maxByGroup('id'); + $this->assertInstanceOf(Collection::class, $results); + $this->assertEquals([['role' => 'admin', 'aggregate' => 1]], $results->toArray()); + + $builder = $this->getBuilder(); + $builder->getConnection()->shouldReceive('select')->once()->with('select "role", min("id") as aggregate from "users" group by "role"', [], true)->andReturn([['role' => 'admin', 'aggregate' => 1]]); + $builder->getProcessor()->shouldReceive('processSelect')->once()->andReturnUsing(fn ($builder, $results) => $results); + $results = $builder->from('users')->groupBy('role')->minByGroup('id'); + $this->assertInstanceOf(Collection::class, $results); + $this->assertEquals([['role' => 'admin', 'aggregate' => 1]], $results->toArray()); + + $builder = $this->getBuilder(); + $builder->getConnection()->shouldReceive('select')->once()->with('select "role", sum("id") as aggregate from "users" group by "role"', [], true)->andReturn([['role' => 'admin', 'aggregate' => 1]]); + $builder->getProcessor()->shouldReceive('processSelect')->once()->andReturnUsing(fn ($builder, $results) => $results); + $results = $builder->from('users')->groupBy('role')->sumByGroup('id'); + $this->assertInstanceOf(Collection::class, $results); + $this->assertEquals([['role' => 'admin', 'aggregate' => 1]], $results->toArray()); + + $builder = $this->getBuilder(); + $builder->getConnection()->shouldReceive('select')->once()->with('select "role", avg("id") as aggregate from "users" group by "role"', [], true)->andReturn([['role' => 'admin', 'aggregate' => 1]]); + $builder->getProcessor()->shouldReceive('processSelect')->once()->andReturnUsing(fn ($builder, $results) => $results); + $results = $builder->from('users')->groupBy('role')->avgByGroup('id'); + $this->assertInstanceOf(Collection::class, $results); + $this->assertEquals([['role' => 'admin', 'aggregate' => 1]], $results->toArray()); + } + public function testSqlServerExists() { $builder = $this->getSqlServerBuilder(); From bd778115214157861a0ad651e57f779dbb83abdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Wed, 18 Dec 2024 23:24:42 +0100 Subject: [PATCH 2/4] Move group columns after aggregate result for safe backward compatibility --- .../Database/Query/Grammars/Grammar.php | 6 +++--- tests/Database/DatabaseQueryBuilderTest.php | 18 ++++++++++-------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/Illuminate/Database/Query/Grammars/Grammar.php b/src/Illuminate/Database/Query/Grammars/Grammar.php index cdc6d2c2e5d8..a4cd649858ac 100755 --- a/src/Illuminate/Database/Query/Grammars/Grammar.php +++ b/src/Illuminate/Database/Query/Grammars/Grammar.php @@ -141,12 +141,12 @@ protected function compileAggregate(Builder $query, $aggregate) $sql = 'select '; + $sql .= $aggregate['function'].'('.$column.') as aggregate'; + if ($query->groups) { - $sql .= $this->columnize($query->groups).', '; + $sql .= ', '.$this->columnize($query->groups); } - $sql .= $aggregate['function'].'('.$column.') as aggregate'; - return $sql; } diff --git a/tests/Database/DatabaseQueryBuilderTest.php b/tests/Database/DatabaseQueryBuilderTest.php index 552fc60a047d..5f07e97ae10d 100755 --- a/tests/Database/DatabaseQueryBuilderTest.php +++ b/tests/Database/DatabaseQueryBuilderTest.php @@ -1812,10 +1812,12 @@ public function testAggregateByGroup() $queryResults = [['aggregate' => 2, 'role' => 'admin', 'city' => 'NY'], ['aggregate' => 5, 'role' => 'user', 'city' => 'LA']]; $builder->getConnection() ->shouldReceive('select')->once() - ->with('select "role", "city", count(*) as aggregate from "users" group by "role", "city"', [], true) + ->with('select count(*) as aggregate, "role", "city" from "users" group by "role", "city"', [], true) ->andReturn($queryResults); $builder->getProcessor()->shouldReceive('processSelect')->once()->andReturnUsing(fn ($builder, $results) => $results); - $results = $builder->from('users')->groupBy('role', 'city')->aggregateByGroup('count'); + $builder->from('users')->groupBy('role', 'city'); + $builder->aggregate = ['function' => 'count', 'columns' => ['*']]; + $results = $builder->get(); $this->assertEquals($queryResults, $results->toArray()); } @@ -1826,7 +1828,7 @@ public function testUnionAndAggregateByGroup() $queryResults = [['aggregate' => 2, 'role' => 'admin'], ['aggregate' => 5, 'role' => 'user']]; $builder->getConnection() ->shouldReceive('select')->once() - ->with('select "role", count(*) as aggregate from ((select * from "users") union (select * from "members")) as "temp_table" group by "role"', [], true) + ->with('select count(*) as aggregate, "role" from ((select * from "users") union (select * from "members")) as "temp_table" group by "role"', [], true) ->andReturn($queryResults); $builder->getProcessor()->shouldReceive('processSelect')->once()->andReturnUsing(fn ($builder, $results) => $results); $results = $builder->from('users') @@ -3498,35 +3500,35 @@ public function testAggregateFunctions() public function testAggregateFunctionsWithGroupBy() { $builder = $this->getBuilder(); - $builder->getConnection()->shouldReceive('select')->once()->with('select "role", count(*) as aggregate from "users" group by "role"', [], true)->andReturn([['role' => 'admin', 'aggregate' => 1]]); + $builder->getConnection()->shouldReceive('select')->once()->with('select count(*) as aggregate, "role" from "users" group by "role"', [], true)->andReturn([['role' => 'admin', 'aggregate' => 1]]); $builder->getProcessor()->shouldReceive('processSelect')->once()->andReturnUsing(fn ($builder, $results) => $results); $results = $builder->from('users')->groupBy('role')->countByGroup(); $this->assertInstanceOf(Collection::class, $results); $this->assertEquals([['role' => 'admin', 'aggregate' => 1]], $results->toArray()); $builder = $this->getBuilder(); - $builder->getConnection()->shouldReceive('select')->once()->with('select "role", max("id") as aggregate from "users" group by "role"', [], true)->andReturn([['role' => 'admin', 'aggregate' => 1]]); + $builder->getConnection()->shouldReceive('select')->once()->with('select max("id") as aggregate, "role" from "users" group by "role"', [], true)->andReturn([['role' => 'admin', 'aggregate' => 1]]); $builder->getProcessor()->shouldReceive('processSelect')->once()->andReturnUsing(fn ($builder, $results) => $results); $results = $builder->from('users')->groupBy('role')->maxByGroup('id'); $this->assertInstanceOf(Collection::class, $results); $this->assertEquals([['role' => 'admin', 'aggregate' => 1]], $results->toArray()); $builder = $this->getBuilder(); - $builder->getConnection()->shouldReceive('select')->once()->with('select "role", min("id") as aggregate from "users" group by "role"', [], true)->andReturn([['role' => 'admin', 'aggregate' => 1]]); + $builder->getConnection()->shouldReceive('select')->once()->with('select min("id") as aggregate, "role" from "users" group by "role"', [], true)->andReturn([['role' => 'admin', 'aggregate' => 1]]); $builder->getProcessor()->shouldReceive('processSelect')->once()->andReturnUsing(fn ($builder, $results) => $results); $results = $builder->from('users')->groupBy('role')->minByGroup('id'); $this->assertInstanceOf(Collection::class, $results); $this->assertEquals([['role' => 'admin', 'aggregate' => 1]], $results->toArray()); $builder = $this->getBuilder(); - $builder->getConnection()->shouldReceive('select')->once()->with('select "role", sum("id") as aggregate from "users" group by "role"', [], true)->andReturn([['role' => 'admin', 'aggregate' => 1]]); + $builder->getConnection()->shouldReceive('select')->once()->with('select sum("id") as aggregate, "role" from "users" group by "role"', [], true)->andReturn([['role' => 'admin', 'aggregate' => 1]]); $builder->getProcessor()->shouldReceive('processSelect')->once()->andReturnUsing(fn ($builder, $results) => $results); $results = $builder->from('users')->groupBy('role')->sumByGroup('id'); $this->assertInstanceOf(Collection::class, $results); $this->assertEquals([['role' => 'admin', 'aggregate' => 1]], $results->toArray()); $builder = $this->getBuilder(); - $builder->getConnection()->shouldReceive('select')->once()->with('select "role", avg("id") as aggregate from "users" group by "role"', [], true)->andReturn([['role' => 'admin', 'aggregate' => 1]]); + $builder->getConnection()->shouldReceive('select')->once()->with('select avg("id") as aggregate, "role" from "users" group by "role"', [], true)->andReturn([['role' => 'admin', 'aggregate' => 1]]); $builder->getProcessor()->shouldReceive('processSelect')->once()->andReturnUsing(fn ($builder, $results) => $results); $results = $builder->from('users')->groupBy('role')->avgByGroup('id'); $this->assertInstanceOf(Collection::class, $results); From 2cb920cb91a5c535cb268cb9fd64c1ff9083a951 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Thu, 9 Jan 2025 14:51:36 -0800 Subject: [PATCH 3/4] formatting --- src/Illuminate/Database/Query/Builder.php | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/Illuminate/Database/Query/Builder.php b/src/Illuminate/Database/Query/Builder.php index 98fdc758a4fd..adf1a81c8a60 100755 --- a/src/Illuminate/Database/Query/Builder.php +++ b/src/Illuminate/Database/Query/Builder.php @@ -3568,7 +3568,7 @@ public function count($columns = '*') * @param \Illuminate\Contracts\Database\Query\Expression|string $columns * @return \Illuminate\Support\Collection */ - public function countByGroup($columns = '*'): Collection + public function countByGroup($columns = '*') { return $this->aggregateByGroup('count', Arr::wrap($columns)); } @@ -3590,7 +3590,7 @@ public function min($column) * @param \Illuminate\Contracts\Database\Query\Expression|string $column * @return \Illuminate\Support\Collection */ - public function minByGroup($column): Collection + public function minByGroup($column) { return $this->aggregateByGroup('min', [$column]); } @@ -3612,7 +3612,7 @@ public function max($column) * @param \Illuminate\Contracts\Database\Query\Expression|string $column * @return \Illuminate\Support\Collection */ - public function maxByGroup($column): Collection + public function maxByGroup($column) { return $this->aggregateByGroup('max', [$column]); } @@ -3636,7 +3636,7 @@ public function sum($column) * @param \Illuminate\Contracts\Database\Query\Expression|string $column * @return \Illuminate\Support\Collection */ - public function sumByGroup($column): Collection + public function sumByGroup($column) { return $this->aggregateByGroup('sum', [$column]); } @@ -3658,7 +3658,7 @@ public function avg($column) * @param \Illuminate\Contracts\Database\Query\Expression|string $column * @return \Illuminate\Support\Collection */ - public function avgByGroup($column): Collection + public function avgByGroup($column) { return $this->aggregateByGroup('avg', [$column]); } @@ -3694,14 +3694,13 @@ public function aggregate($function, $columns = ['*']) } /** - * Execute an aggregate function for each group and return the results - * in a collection of objects with the group columns and aggregate value. + * Execute an aggregate function for each group. * - * @param string $function The aggregate function to call - * @param array $columns The columns to perform the aggregate function on + * @param string $function + * @param array $columns * @return \Illuminate\Support\Collection */ - public function aggregateByGroup(string $function, array $columns = ['*']): Collection + public function aggregateByGroup(string $function, array $columns = ['*']) { return $this->cloneWithout($this->unions || $this->havings ? [] : ['columns']) ->cloneWithoutBindings($this->unions || $this->havings ? [] : ['select']) From 54628636236b8f8ea742a174a66d47591fd84b0e Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Thu, 9 Jan 2025 14:52:44 -0800 Subject: [PATCH 4/4] formatting --- src/Illuminate/Database/Query/Builder.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Illuminate/Database/Query/Builder.php b/src/Illuminate/Database/Query/Builder.php index adf1a81c8a60..f2f171b4bc93 100755 --- a/src/Illuminate/Database/Query/Builder.php +++ b/src/Illuminate/Database/Query/Builder.php @@ -3562,8 +3562,7 @@ public function count($columns = '*') } /** - * Retrieve the "count" of the distinct results of a given column - * for each group. + * Retrieve the "count" of the distinct results of a given column for each group. * * @param \Illuminate\Contracts\Database\Query\Expression|string $columns * @return \Illuminate\Support\Collection