Skip to content

Commit d8eb542

Browse files
kayrunmkayrunm
authored
[9.x] Fix issue with aggregates (withSum, etc.) for pivot columns on self-referencing many-to-many relations (#44286)
* Add failing test for current state. * Apply potential fix. * Potential fix? * Use better name
1 parent 1fa2669 commit d8eb542

File tree

2 files changed

+203
-3
lines changed

2 files changed

+203
-3
lines changed

‎src/Illuminate/Database/Eloquent/Concerns/QueriesRelationships.php

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -622,9 +622,7 @@ public function withAggregate($relations, $column, $function = null)
622622
$relation = $this->getRelationWithoutConstraints($name);
623623

624624
if ($function) {
625-
$hashedColumn = $this->getQuery()->from === $relation->getQuery()->getQuery()->from
626-
? "{$relation->getRelationCountHash(false)}.$column"
627-
: $column;
625+
$hashedColumn = $this->getRelationHashedColumn($column, $relation);
628626

629627
$wrappedColumn = $this->getQuery()->getGrammar()->wrap(
630628
$column === '*' ? $column : $relation->getRelated()->qualifyColumn($hashedColumn)
@@ -680,6 +678,24 @@ public function withAggregate($relations, $column, $function = null)
680678
return $this;
681679
}
682680

681+
/**
682+
* Get the relation hashed column name for the given column and relation.
683+
*
684+
* @param string $column
685+
* @param \Illuminate\Database\Eloquent\Relations\Relationship $relation
686+
* @return string
687+
*/
688+
protected function getRelationHashedColumn($column, $relation)
689+
{
690+
if (str_contains($column, '.')) {
691+
return $column;
692+
}
693+
694+
return $this->getQuery()->from === $relation->getQuery()->getQuery()->from
695+
? "{$relation->getRelationCountHash(false)}.$column"
696+
: $column;
697+
}
698+
683699
/**
684700
* Add subselect queries to count the relations.
685701
*

‎tests/Database/DatabaseEloquentBelongsToManyAggregateTest.php

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
<?php
2+
3+
namespace Illuminate\Tests\Database;
4+
5+
use Illuminate\Database\Capsule\Manager as DB;
6+
use Illuminate\Database\Eloquent\Model as Eloquent;
7+
use PHPUnit\Framework\TestCase;
8+
9+
class DatabaseEloquentBelongsToManyAggregateTest extends TestCase
10+
{
11+
protected function setUp(): void
12+
{
13+
$db = new DB;
14+
15+
$db->addConnection([
16+
'driver' => 'sqlite',
17+
'database' => ':memory:',
18+
]);
19+
20+
$db->bootEloquent();
21+
$db->setAsGlobal();
22+
23+
$this->createSchema();
24+
}
25+
26+
public function testWithSumDifferentTables()
27+
{
28+
$this->seedData();
29+
30+
$order = BelongsToManyAggregateTestTestOrder::query()
31+
->withSum('products as total_products', 'order_product.quantity')
32+
->first();
33+
34+
$this->assertEquals(12, $order->total_products);
35+
}
36+
37+
public function testWithSumSameTable()
38+
{
39+
$this->seedData();
40+
41+
$order = BelongsToManyAggregateTestTestTransaction::query()
42+
->withSum('allocatedTo as total_allocated', 'allocations.amount')
43+
->first();
44+
45+
$this->assertEquals(1200, $order->total_allocated);
46+
}
47+
48+
/**
49+
* Setup the database schema.
50+
*
51+
* @return void
52+
*/
53+
public function createSchema()
54+
{
55+
$this->schema()->create('orders', function ($table) {
56+
$table->increments('id');
57+
});
58+
59+
$this->schema()->create('products', function ($table) {
60+
$table->increments('id');
61+
});
62+
63+
$this->schema()->create('order_product', function ($table) {
64+
$table->integer('order_id')->unsigned();
65+
$table->foreign('order_id')->references('id')->on('orders');
66+
$table->integer('product_id')->unsigned();
67+
$table->foreign('product_id')->references('id')->on('products');
68+
$table->integer('quantity')->unsigned();
69+
});
70+
71+
$this->schema()->create('transactions', function ($table) {
72+
$table->increments('id');
73+
$table->integer('value')->unsigned();
74+
});
75+
76+
$this->schema()->create('allocations', function ($table) {
77+
$table->integer('from_id')->unsigned();
78+
$table->foreign('from_id')->references('id')->on('transactions');
79+
$table->integer('to_id')->unsigned();
80+
$table->foreign('to_id')->references('id')->on('transactions');
81+
$table->integer('amount')->unsigned();
82+
});
83+
}
84+
85+
/**
86+
* Tear down the database schema.
87+
*
88+
* @return void
89+
*/
90+
protected function tearDown(): void
91+
{
92+
$this->schema()->drop('orders');
93+
$this->schema()->drop('products');
94+
}
95+
96+
/**
97+
* Helpers...
98+
*/
99+
protected function seedData()
100+
{
101+
$order = BelongsToManyAggregateTestTestOrder::create(['id' => 1]);
102+
103+
BelongsToManyAggregateTestTestProduct::query()->insert([
104+
['id' => 1],
105+
['id' => 2],
106+
['id' => 3],
107+
]);
108+
109+
$order->products()->sync([
110+
1 => ['quantity' => 3],
111+
2 => ['quantity' => 4],
112+
3 => ['quantity' => 5],
113+
]);
114+
115+
$transaction = BelongsToManyAggregateTestTestTransaction::create(['id' => 1, 'value' => 1200]);
116+
117+
BelongsToManyAggregateTestTestTransaction::query()->insert([
118+
['id' => 2, 'value' => -300],
119+
['id' => 3, 'value' => -400],
120+
['id' => 4, 'value' => -500],
121+
]);
122+
123+
$transaction->allocatedTo()->sync([
124+
2 => ['amount' => 300],
125+
3 => ['amount' => 400],
126+
4 => ['amount' => 500],
127+
]);
128+
}
129+
130+
/**
131+
* Get a database connection instance.
132+
*
133+
* @return \Illuminate\Database\ConnectionInterface
134+
*/
135+
protected function connection()
136+
{
137+
return Eloquent::getConnectionResolver()->connection();
138+
}
139+
140+
/**
141+
* Get a schema builder instance.
142+
*
143+
* @return \Illuminate\Database\Schema\Builder
144+
*/
145+
protected function schema()
146+
{
147+
return $this->connection()->getSchemaBuilder();
148+
}
149+
}
150+
151+
class BelongsToManyAggregateTestTestOrder extends Eloquent
152+
{
153+
protected $table = 'orders';
154+
protected $fillable = ['id'];
155+
public $timestamps = false;
156+
157+
public function products()
158+
{
159+
return $this
160+
->belongsToMany(BelongsToManyAggregateTestTestProduct::class, 'order_product', 'order_id', 'product_id')
161+
->withPivot('quantity');
162+
}
163+
}
164+
165+
class BelongsToManyAggregateTestTestProduct extends Eloquent
166+
{
167+
protected $table = 'products';
168+
protected $fillable = ['id'];
169+
public $timestamps = false;
170+
}
171+
172+
class BelongsToManyAggregateTestTestTransaction extends Eloquent
173+
{
174+
protected $table = 'transactions';
175+
protected $fillable = ['id', 'value'];
176+
public $timestamps = false;
177+
178+
public function allocatedTo()
179+
{
180+
return $this
181+
->belongsToMany(BelongsToManyAggregateTestTestTransaction::class, 'allocations', 'from_id', 'to_id')
182+
->withPivot('quantity');
183+
}
184+
}

0 commit comments

Comments
 (0)