From 3e1d8f1b2b6dfe88e1c7945cef1c05697c85c1fd 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/5] Support aggregation by group --- src/Illuminate/Database/Query/Builder.php | 6 ++++++ src/Illuminate/Database/Query/Grammars/Grammar.php | 4 ++++ tests/Database/DatabaseQueryBuilderTest.php | 14 ++++++++++++++ 3 files changed, 24 insertions(+) diff --git a/src/Illuminate/Database/Query/Builder.php b/src/Illuminate/Database/Query/Builder.php index ff5c7be04605..8ec86be462a9 100755 --- a/src/Illuminate/Database/Query/Builder.php +++ b/src/Illuminate/Database/Query/Builder.php @@ -3631,6 +3631,10 @@ public function aggregate($function, $columns = ['*']) ->setAggregate($function, $columns) ->get($columns); + if ($this->groups) { + return $results; + } + if (! $results->isEmpty()) { return array_change_key_case((array) $results[0])['aggregate']; } @@ -3680,6 +3684,8 @@ protected function setAggregate($function, $columns) $this->orders = null; $this->bindings['order'] = []; + } else { + $this->columns = array_merge($this->groups, $columns); } return $this; diff --git a/src/Illuminate/Database/Query/Grammars/Grammar.php b/src/Illuminate/Database/Query/Grammars/Grammar.php index 1d3148f21150..2de1ca329605 100755 --- a/src/Illuminate/Database/Query/Grammars/Grammar.php +++ b/src/Illuminate/Database/Query/Grammars/Grammar.php @@ -154,6 +154,10 @@ protected function compileColumns(Builder $query, $columns) // compiler handle the building of the select clauses, as it will need some // more syntax that is best handled by that function to keep things neat. if (! is_null($query->aggregate)) { + if ($query->groups) { + return ', '.$this->columnize($query->groups); + } + return; } diff --git a/tests/Database/DatabaseQueryBuilderTest.php b/tests/Database/DatabaseQueryBuilderTest.php index d635a7e2efc3..8516ced36064 100755 --- a/tests/Database/DatabaseQueryBuilderTest.php +++ b/tests/Database/DatabaseQueryBuilderTest.php @@ -1804,6 +1804,20 @@ public function testGroupBys() $this->assertEquals(['whereRawBinding', 'groupByRawBinding', 'havingRawBinding'], $builder->getBindings()); } + public function testGroupByAndAggregate() + { + $builder = $this->getBuilder(); + + $queryResults = [(object) ['aggregate' => 2, 'role' => 'admin', 'city' => 'NY'], (object) ['aggregate' => 5, 'role' => 'user', 'city' => 'LA']]; + $builder->getConnection() + ->shouldReceive('select')->once() + ->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->select('role')->from('users')->groupBy('role', 'city')->aggregate('count'); + $this->assertEquals($queryResults, $results->toArray()); + } + public function testOrderBys() { $builder = $this->getBuilder(); From 4e49c8f01c10b1c5b0726048ab55045811e23b86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Fri, 18 Oct 2024 00:31:32 +0200 Subject: [PATCH 2/5] Support group aggregate with union --- src/Illuminate/Database/Query/Builder.php | 2 -- .../Database/Query/Grammars/Grammar.php | 19 ++++++++++++------ tests/Database/DatabaseQueryBuilderTest.php | 20 +++++++++++++++++-- 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/src/Illuminate/Database/Query/Builder.php b/src/Illuminate/Database/Query/Builder.php index 8ec86be462a9..6abe5953fe0d 100755 --- a/src/Illuminate/Database/Query/Builder.php +++ b/src/Illuminate/Database/Query/Builder.php @@ -3684,8 +3684,6 @@ protected function setAggregate($function, $columns) $this->orders = null; $this->bindings['order'] = []; - } else { - $this->columns = array_merge($this->groups, $columns); } return $this; diff --git a/src/Illuminate/Database/Query/Grammars/Grammar.php b/src/Illuminate/Database/Query/Grammars/Grammar.php index 2de1ca329605..02b1caac3c5e 100755 --- a/src/Illuminate/Database/Query/Grammars/Grammar.php +++ b/src/Illuminate/Database/Query/Grammars/Grammar.php @@ -138,7 +138,16 @@ protected function compileAggregate(Builder $query, $aggregate) $column = 'distinct '.$column; } - return 'select '.$aggregate['function'].'('.$column.') as aggregate'; + $sql = 'select '; + + // Select the columns that are used to group the results + if ($query->groups) { + $sql .= $this->columnize($query->groups).', '; + } + + $sql .= $aggregate['function'].'('.$column.') as aggregate'; + + return $sql; } /** @@ -154,10 +163,6 @@ protected function compileColumns(Builder $query, $columns) // compiler handle the building of the select clauses, as it will need some // more syntax that is best handled by that function to keep things neat. if (! is_null($query->aggregate)) { - if ($query->groups) { - return ', '.$this->columnize($query->groups); - } - return; } @@ -1134,10 +1139,12 @@ protected function wrapUnion($sql) protected function compileUnionAggregate(Builder $query) { $sql = $this->compileAggregate($query, $query->aggregate); + $group = $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').$group; } /** diff --git a/tests/Database/DatabaseQueryBuilderTest.php b/tests/Database/DatabaseQueryBuilderTest.php index 8516ced36064..c1a641d2ebf8 100755 --- a/tests/Database/DatabaseQueryBuilderTest.php +++ b/tests/Database/DatabaseQueryBuilderTest.php @@ -1811,10 +1811,26 @@ public function testGroupByAndAggregate() $queryResults = [(object) ['aggregate' => 2, 'role' => 'admin', 'city' => 'NY'], (object) ['aggregate' => 5, 'role' => 'user', 'city' => 'LA']]; $builder->getConnection() ->shouldReceive('select')->once() - ->with('select count(*) as aggregate , "role", "city" from "users" group by "role", "city"', [], true) + ->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->select('role')->from('users')->groupBy('role', 'city')->aggregate('count'); + $results = $builder->from('users')->groupBy('role', 'city')->aggregate('count'); + $this->assertEquals($queryResults, $results->toArray()); + } + + public function testGroupByUnionAndAggregate() + { + $builder = $this->getBuilder(); + + $queryResults = [(object) ['aggregate' => 2, 'role' => 'admin', 'city' => 'NY'], (object) ['aggregate' => 5, 'role' => 'user', 'city' => 'LA']]; + $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')->aggregate('count'); $this->assertEquals($queryResults, $results->toArray()); } From 5453e62f782cddcaea41df39c9ea9e62c7311572 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Fri, 18 Oct 2024 01:14:09 +0200 Subject: [PATCH 3/5] Change return type of all aggregate functions --- src/Illuminate/Database/Query/Builder.php | 18 ++++---- tests/Database/DatabaseQueryBuilderTest.php | 46 +++++++++++++++++++++ 2 files changed, 56 insertions(+), 8 deletions(-) diff --git a/src/Illuminate/Database/Query/Builder.php b/src/Illuminate/Database/Query/Builder.php index 6abe5953fe0d..d861bc9e8385 100755 --- a/src/Illuminate/Database/Query/Builder.php +++ b/src/Illuminate/Database/Query/Builder.php @@ -3553,18 +3553,20 @@ public function doesntExistOr(Closure $callback) * Retrieve the "count" result of the query. * * @param \Illuminate\Contracts\Database\Query\Expression|string $columns - * @return int + * @return int|Collection */ public function count($columns = '*') { - return (int) $this->aggregate(__FUNCTION__, Arr::wrap($columns)); + $results = $this->aggregate(__FUNCTION__, Arr::wrap($columns)); + + return $results instanceof Collection ? $results : (int) $results; } /** * Retrieve the minimum value of a given column. * * @param \Illuminate\Contracts\Database\Query\Expression|string $column - * @return mixed + * @return mixed|Collection */ public function min($column) { @@ -3575,7 +3577,7 @@ public function min($column) * Retrieve the maximum value of a given column. * * @param \Illuminate\Contracts\Database\Query\Expression|string $column - * @return mixed + * @return mixed|Collection */ public function max($column) { @@ -3586,7 +3588,7 @@ public function max($column) * Retrieve the sum of the values of a given column. * * @param \Illuminate\Contracts\Database\Query\Expression|string $column - * @return mixed + * @return mixed|Collection */ public function sum($column) { @@ -3599,7 +3601,7 @@ public function sum($column) * Retrieve the average of the values of a given column. * * @param \Illuminate\Contracts\Database\Query\Expression|string $column - * @return mixed + * @return mixed|Collection */ public function avg($column) { @@ -3610,7 +3612,7 @@ public function avg($column) * Alias for the "avg" method. * * @param \Illuminate\Contracts\Database\Query\Expression|string $column - * @return mixed + * @return mixed|Collection */ public function average($column) { @@ -3622,7 +3624,7 @@ public function average($column) * * @param string $function * @param array $columns - * @return mixed + * @return mixed|Collection */ public function aggregate($function, $columns = ['*']) { diff --git a/tests/Database/DatabaseQueryBuilderTest.php b/tests/Database/DatabaseQueryBuilderTest.php index c1a641d2ebf8..77ac4d27f1d0 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; @@ -3181,6 +3182,51 @@ 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')->count(); + $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')->max('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')->min('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')->sum('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')->avg('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')->average('id'); + $this->assertInstanceOf(Collection::class, $results); + $this->assertEquals([['role' => 'admin', 'aggregate' => 1]], $results->toArray()); + } + public function testSqlServerExists() { $builder = $this->getSqlServerBuilder(); From d7258ce60b006f38bdde2376bae696c9204383f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Fri, 18 Oct 2024 01:17:45 +0200 Subject: [PATCH 4/5] Fix CS --- src/Illuminate/Database/Query/Grammars/Grammar.php | 2 +- tests/Database/DatabaseQueryBuilderTest.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Illuminate/Database/Query/Grammars/Grammar.php b/src/Illuminate/Database/Query/Grammars/Grammar.php index 02b1caac3c5e..6331323c2c04 100755 --- a/src/Illuminate/Database/Query/Grammars/Grammar.php +++ b/src/Illuminate/Database/Query/Grammars/Grammar.php @@ -1139,7 +1139,7 @@ protected function wrapUnion($sql) protected function compileUnionAggregate(Builder $query) { $sql = $this->compileAggregate($query, $query->aggregate); - $group = $query->groups ? ' ' . $this->compileGroups($query, $query->groups) : ''; + $group = $query->groups ? ' '.$this->compileGroups($query, $query->groups) : ''; $query->aggregate = null; $query->groups = null; diff --git a/tests/Database/DatabaseQueryBuilderTest.php b/tests/Database/DatabaseQueryBuilderTest.php index 77ac4d27f1d0..070cf476519f 100755 --- a/tests/Database/DatabaseQueryBuilderTest.php +++ b/tests/Database/DatabaseQueryBuilderTest.php @@ -1809,7 +1809,7 @@ public function testGroupByAndAggregate() { $builder = $this->getBuilder(); - $queryResults = [(object) ['aggregate' => 2, 'role' => 'admin', 'city' => 'NY'], (object) ['aggregate' => 5, 'role' => 'user', 'city' => 'LA']]; + $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) @@ -1823,7 +1823,7 @@ public function testGroupByUnionAndAggregate() { $builder = $this->getBuilder(); - $queryResults = [(object) ['aggregate' => 2, 'role' => 'admin', 'city' => 'NY'], (object) ['aggregate' => 5, 'role' => 'user', 'city' => 'LA']]; + $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) From 61c039b042920c90122da096c5be506b196b6866 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Thu, 31 Oct 2024 15:41:04 -0500 Subject: [PATCH 5/5] formatting --- src/Illuminate/Database/Query/Builder.php | 14 +++++++------- src/Illuminate/Database/Query/Grammars/Grammar.php | 5 ++--- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/Illuminate/Database/Query/Builder.php b/src/Illuminate/Database/Query/Builder.php index d861bc9e8385..a3d8a6c8c9c8 100755 --- a/src/Illuminate/Database/Query/Builder.php +++ b/src/Illuminate/Database/Query/Builder.php @@ -3553,7 +3553,7 @@ public function doesntExistOr(Closure $callback) * Retrieve the "count" result of the query. * * @param \Illuminate\Contracts\Database\Query\Expression|string $columns - * @return int|Collection + * @return \Illuminate\Support\Collection|int */ public function count($columns = '*') { @@ -3566,7 +3566,7 @@ public function count($columns = '*') * Retrieve the minimum value of a given column. * * @param \Illuminate\Contracts\Database\Query\Expression|string $column - * @return mixed|Collection + * @return \Illuminate\Support\Collection|mixed */ public function min($column) { @@ -3577,7 +3577,7 @@ public function min($column) * Retrieve the maximum value of a given column. * * @param \Illuminate\Contracts\Database\Query\Expression|string $column - * @return mixed|Collection + * @return \Illuminate\Support\Collection|mixed */ public function max($column) { @@ -3588,7 +3588,7 @@ public function max($column) * Retrieve the sum of the values of a given column. * * @param \Illuminate\Contracts\Database\Query\Expression|string $column - * @return mixed|Collection + * @return \Illuminate\Support\Collection|mixed */ public function sum($column) { @@ -3601,7 +3601,7 @@ public function sum($column) * Retrieve the average of the values of a given column. * * @param \Illuminate\Contracts\Database\Query\Expression|string $column - * @return mixed|Collection + * @return \Illuminate\Support\Collection|mixed */ public function avg($column) { @@ -3612,7 +3612,7 @@ public function avg($column) * Alias for the "avg" method. * * @param \Illuminate\Contracts\Database\Query\Expression|string $column - * @return mixed|Collection + * @return \Illuminate\Support\Collection|mixed */ public function average($column) { @@ -3624,7 +3624,7 @@ public function average($column) * * @param string $function * @param array $columns - * @return mixed|Collection + * @return \Illuminate\Support\Collection|mixed */ public function aggregate($function, $columns = ['*']) { diff --git a/src/Illuminate/Database/Query/Grammars/Grammar.php b/src/Illuminate/Database/Query/Grammars/Grammar.php index 6331323c2c04..1c82319fc1e9 100755 --- a/src/Illuminate/Database/Query/Grammars/Grammar.php +++ b/src/Illuminate/Database/Query/Grammars/Grammar.php @@ -140,7 +140,6 @@ protected function compileAggregate(Builder $query, $aggregate) $sql = 'select '; - // Select the columns that are used to group the results if ($query->groups) { $sql .= $this->columnize($query->groups).', '; } @@ -1139,12 +1138,12 @@ protected function wrapUnion($sql) protected function compileUnionAggregate(Builder $query) { $sql = $this->compileAggregate($query, $query->aggregate); - $group = $query->groups ? ' '.$this->compileGroups($query, $query->groups) : ''; + $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').$group; + return $sql.' from ('.$this->compileSelect($query).') as '.$this->wrapTable('temp_table').$groups; } /**