From 3e1d8f1b2b6dfe88e1c7945cef1c05697c85c1fd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= <jerome@tamarelle.net>
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?= <jerome@tamarelle.net>
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?= <jerome@tamarelle.net>
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?= <jerome@tamarelle.net>
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 <taylor@laravel.com>
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;
}
/**