[10.x] Fix expressions in with-functions doing aggregates (#49912)

* support expression in with-functions doing aggregates * apply Laravel docblock rules * typo
laravel · Jan 30, 2024 · b1ebf4f · b1ebf4f
1 parent a4dfd9f
commit b1ebf4f
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 12 deletions.
diff --git a/src/Illuminate/Database/Eloquent/Concerns/QueriesRelationships.php b/src/Illuminate/Database/Eloquent/Concerns/QueriesRelationships.php
@@ -599,7 +599,7 @@ public function orWhereBelongsTo($related, $relationshipName = null)
      * Add subselect queries to include an aggregate value for a relationship.
      *
      * @param  mixed  $relations
-     * @param  string  $column
+     * @param  \Illuminate\Contracts\Database\Query\Expression|string  $column
      * @param  string  $function
      * @return $this
      */
@@ -630,15 +630,19 @@ public function withAggregate($relations, $column, $function = null)
             $relation = $this->getRelationWithoutConstraints($name);
 
             if ($function) {
-                $hashedColumn = $this->getRelationHashedColumn($column, $relation);
+                if ($this->getGrammar()->isExpression($column)) {
+                    $aggregateColumn = $this->getGrammar()->getValue($column);
+                } else {
+                    $hashedColumn = $this->getRelationHashedColumn($column, $relation);
 
-                $wrappedColumn = $this->getQuery()->getGrammar()->wrap(
-                    $column === '*' ? $column : $relation->getRelated()->qualifyColumn($hashedColumn)
-                );
+                    $aggregateColumn = $this->getQuery()->getGrammar()->wrap(
+                        $column === '*' ? $column : $relation->getRelated()->qualifyColumn($hashedColumn)
+                    );
+                }
 
-                $expression = $function === 'exists' ? $wrappedColumn : sprintf('%s(%s)', $function, $wrappedColumn);
+                $expression = $function === 'exists' ? $aggregateColumn : sprintf('%s(%s)', $function, $aggregateColumn);
             } else {
-                $expression = $column;
+                $expression = $this->getGrammar()->getValue($column);
             }
 
             // Here, we will grab the relationship sub-query and prepare to add it to the main query
@@ -667,7 +671,7 @@ public function withAggregate($relations, $column, $function = null)
             // the query builder. Then, we will return the builder instance back to the developer
             // for further constraint chaining that needs to take place on the query as needed.
             $alias ??= Str::snake(
-                preg_replace('/[^[:alnum:][:space:]_]/u', '', "$name $function $column")
+                preg_replace('/[^[:alnum:][:space:]_]/u', '', "$name $function {$this->getGrammar()->getValue($column)}")
             );
 
             if ($function === 'exists') {
@@ -719,7 +723,7 @@ public function withCount($relations)
      * Add subselect queries to include the max of the relation's column.
      *
      * @param  string|array  $relation
-     * @param  string  $column
+     * @param  \Illuminate\Contracts\Database\Query\Expression|string  $column
      * @return $this
      */
     public function withMax($relation, $column)
@@ -731,7 +735,7 @@ public function withMax($relation, $column)
      * Add subselect queries to include the min of the relation's column.
      *
      * @param  string|array  $relation
-     * @param  string  $column
+     * @param  \Illuminate\Contracts\Database\Query\Expression|string  $column
      * @return $this
      */
     public function withMin($relation, $column)
@@ -743,7 +747,7 @@ public function withMin($relation, $column)
      * Add subselect queries to include the sum of the relation's column.
      *
      * @param  string|array  $relation
-     * @param  string  $column
+     * @param  \Illuminate\Contracts\Database\Query\Expression|string  $column
      * @return $this
      */
     public function withSum($relation, $column)
@@ -755,7 +759,7 @@ public function withSum($relation, $column)
      * Add subselect queries to include the average of the relation's column.
      *
      * @param  string|array  $relation
-     * @param  string  $column
+     * @param  \Illuminate\Contracts\Database\Query\Expression|string  $column
      * @return $this
      */
     public function withAvg($relation, $column)

diff --git a/tests/Database/DatabaseEloquentBelongsToManyAggregateTest.php b/tests/Database/DatabaseEloquentBelongsToManyAggregateTest.php
@@ -4,6 +4,7 @@
 
 use Illuminate\Database\Capsule\Manager as DB;
 use Illuminate\Database\Eloquent\Model as Eloquent;
+use Illuminate\Database\Query\Expression;
 use PHPUnit\Framework\TestCase;
 
 class DatabaseEloquentBelongsToManyAggregateTest extends TestCase
@@ -45,6 +46,17 @@ public function testWithSumSameTable()
         $this->assertEquals(1200, $order->total_allocated);
     }
 
+    public function testWithSumExpression()
+    {
+        $this->seedData();
+
+        $order = BelongsToManyAggregateTestTestTransaction::query()
+            ->withSum('allocatedTo as total_allocated', new Expression('allocations.amount * 2'))
+            ->first();
+
+        $this->assertEquals(2400, $order->total_allocated);
+    }
+
     /**
      * Setup the database schema.
      *

diff --git a/tests/Database/DatabaseEloquentBuilderTest.php b/tests/Database/DatabaseEloquentBuilderTest.php
@@ -14,6 +14,7 @@
 use Illuminate\Database\Eloquent\Relations\Relation;
 use Illuminate\Database\Eloquent\SoftDeletes;
 use Illuminate\Database\Query\Builder as BaseBuilder;
+use Illuminate\Database\Query\Expression;
 use Illuminate\Database\Query\Grammars\Grammar;
 use Illuminate\Database\Query\Processors\Processor;
 use Illuminate\Support\Carbon;
@@ -1278,6 +1279,15 @@ public function testWithMin()
         $this->assertSame('select "eloquent_builder_test_model_parent_stubs".*, (select min("eloquent_builder_test_model_close_related_stubs"."price") from "eloquent_builder_test_model_close_related_stubs" where "eloquent_builder_test_model_parent_stubs"."foo_id" = "eloquent_builder_test_model_close_related_stubs"."id") as "foo_min_price" from "eloquent_builder_test_model_parent_stubs"', $builder->toSql());
     }
 
+    public function testWithMinExpression()
+    {
+        $model = new EloquentBuilderTestModelParentStub;
+
+        $builder = $model->withMin('foo', new Expression('price - discount'));
+
+        $this->assertSame('select "eloquent_builder_test_model_parent_stubs".*, (select min(price - discount) from "eloquent_builder_test_model_close_related_stubs" where "eloquent_builder_test_model_parent_stubs"."foo_id" = "eloquent_builder_test_model_close_related_stubs"."id") as "foo_min_price_discount" from "eloquent_builder_test_model_parent_stubs"', $builder->toSql());
+    }
+
     public function testWithMinOnBelongsToMany()
     {
         $model = new EloquentBuilderTestModelParentStub;
@@ -1302,6 +1312,42 @@ public function testWithMinOnSelfRelated()
         $this->assertSame('select "self_related_stubs".*, (select min("self_alias_hash"."created_at") from "self_related_stubs" as "self_alias_hash" where "self_related_stubs"."id" = "self_alias_hash"."parent_id") as "child_foos_min_created_at" from "self_related_stubs"', $sql);
     }
 
+    public function testWithMax()
+    {
+        $model = new EloquentBuilderTestModelParentStub;
+
+        $builder = $model->withMax('foo', 'price');
+
+        $this->assertSame('select "eloquent_builder_test_model_parent_stubs".*, (select max("eloquent_builder_test_model_close_related_stubs"."price") from "eloquent_builder_test_model_close_related_stubs" where "eloquent_builder_test_model_parent_stubs"."foo_id" = "eloquent_builder_test_model_close_related_stubs"."id") as "foo_max_price" from "eloquent_builder_test_model_parent_stubs"', $builder->toSql());
+    }
+
+    public function testWithMaxExpression()
+    {
+        $model = new EloquentBuilderTestModelParentStub;
+
+        $builder = $model->withMax('foo', new Expression('price - discount'));
+
+        $this->assertSame('select "eloquent_builder_test_model_parent_stubs".*, (select max(price - discount) from "eloquent_builder_test_model_close_related_stubs" where "eloquent_builder_test_model_parent_stubs"."foo_id" = "eloquent_builder_test_model_close_related_stubs"."id") as "foo_max_price_discount" from "eloquent_builder_test_model_parent_stubs"', $builder->toSql());
+    }
+
+    public function testWithAvg()
+    {
+        $model = new EloquentBuilderTestModelParentStub;
+
+        $builder = $model->withAvg('foo', 'price');
+
+        $this->assertSame('select "eloquent_builder_test_model_parent_stubs".*, (select avg("eloquent_builder_test_model_close_related_stubs"."price") from "eloquent_builder_test_model_close_related_stubs" where "eloquent_builder_test_model_parent_stubs"."foo_id" = "eloquent_builder_test_model_close_related_stubs"."id") as "foo_avg_price" from "eloquent_builder_test_model_parent_stubs"', $builder->toSql());
+    }
+
+    public function testWitAvgExpression()
+    {
+        $model = new EloquentBuilderTestModelParentStub;
+
+        $builder = $model->withAvg('foo', new Expression('price - discount'));
+
+        $this->assertSame('select "eloquent_builder_test_model_parent_stubs".*, (select avg(price - discount) from "eloquent_builder_test_model_close_related_stubs" where "eloquent_builder_test_model_parent_stubs"."foo_id" = "eloquent_builder_test_model_close_related_stubs"."id") as "foo_avg_price_discount" from "eloquent_builder_test_model_parent_stubs"', $builder->toSql());
+    }
+
     public function testWithCountAndConstraintsAndHaving()
     {
         $model = new EloquentBuilderTestModelParentStub;