Skip to content

Commit 818ebee

Browse files
calebdwcalebdw
committed
fix: one of many subquery constraints
1 parent 46ac782 commit 818ebee

14 files changed

+59
-65
lines changed

Diff for: ‎src/Illuminate/Database/Eloquent/Relations/BelongsTo.php

+5-7
Original file line numberDiff line numberDiff line change
@@ -84,20 +84,18 @@ public function getResults()
8484
return $this->query->first() ?: $this->getDefaultFor($this->parent);
8585
}
8686

87-
/**
88-
* Set the base constraints on the relation query.
89-
*
90-
* @return void
91-
*/
92-
public function addConstraints()
87+
/** @inheritDoc */
88+
public function addConstraints(?Builder $query = null)
9389
{
90+
$query ??= $this->query;
91+
9492
if (static::$constraints) {
9593
// For belongs to relationships, which are essentially the inverse of has one
9694
// or has many relationships, we need to actually query on the primary key
9795
// of the related models matching on the foreign key that's on a parent.
9896
$key = $this->getQualifiedOwnerKeyName();
9997

100-
$this->query->where($key, '=', $this->getForeignKeyFrom($this->child));
98+
$query->where($key, '=', $this->getForeignKeyFrom($this->child));
10199
}
102100
}
103101

Diff for: ‎src/Illuminate/Database/Eloquent/Relations/BelongsToMany.php

+10-11
Original file line numberDiff line numberDiff line change
@@ -200,17 +200,13 @@ protected function resolveTableName($table)
200200
return $model->getTable();
201201
}
202202

203-
/**
204-
* Set the base constraints on the relation query.
205-
*
206-
* @return void
207-
*/
208-
public function addConstraints()
203+
/** @inheritDoc */
204+
public function addConstraints(?Builder $query = null)
209205
{
210-
$this->performJoin();
206+
$this->performJoin($query);
211207

212208
if (static::$constraints) {
213-
$this->addWhereConstraints();
209+
$this->addWhereConstraints($query);
214210
}
215211
}
216212

@@ -222,7 +218,7 @@ public function addConstraints()
222218
*/
223219
protected function performJoin($query = null)
224220
{
225-
$query = $query ?: $this->query;
221+
$query ??= $this->query;
226222

227223
// We need to join to the intermediate table on the related model's primary
228224
// key column with the intermediate table's foreign key for the related
@@ -240,11 +236,14 @@ protected function performJoin($query = null)
240236
/**
241237
* Set the where clause for the relation query.
242238
*
239+
* @param \Illuminate\Database\Eloquent\Builder<TRelatedModel>|null $query
243240
* @return $this
244241
*/
245-
protected function addWhereConstraints()
242+
protected function addWhereConstraints(?Builder $query = null)
246243
{
247-
$this->query->where(
244+
$query ??= $this->query;
245+
246+
$query->where(
248247
$this->getQualifiedForeignPivotKeyName(), '=', $this->parent->{$this->parentKey}
249248
);
250249

Diff for: ‎src/Illuminate/Database/Eloquent/Relations/Concerns/CanBeOneOfMany.php

+2
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,8 @@ protected function newOneOfManySubQuery($groupBy, $columns = null, $aggregate =
213213
}
214214
}
215215

216+
$this->addConstraints($subQuery);
217+
216218
$this->addOneOfManySubQueryConstraints($subQuery, column: null, aggregate: $aggregate);
217219

218220
return $subQuery;

Diff for: ‎src/Illuminate/Database/Eloquent/Relations/HasOneOrMany.php

+3-7
Original file line numberDiff line numberDiff line change
@@ -81,15 +81,11 @@ public function makeMany($records)
8181
return $instances;
8282
}
8383

84-
/**
85-
* Set the base constraints on the relation query.
86-
*
87-
* @return void
88-
*/
89-
public function addConstraints()
84+
/** @inheritDoc */
85+
public function addConstraints(?Builder $query = null)
9086
{
9187
if (static::$constraints) {
92-
$query = $this->getRelationQuery();
88+
$query ??= $this->getRelationQuery();
9389

9490
$query->where($this->foreignKey, '=', $this->getParentKey());
9591

Diff for: ‎src/Illuminate/Database/Eloquent/Relations/HasOneOrManyThrough.php

+6-9
Original file line numberDiff line numberDiff line change
@@ -90,19 +90,16 @@ public function __construct(Builder $query, Model $farParent, Model $throughPare
9090
parent::__construct($query, $throughParent);
9191
}
9292

93-
/**
94-
* Set the base constraints on the relation query.
95-
*
96-
* @return void
97-
*/
98-
public function addConstraints()
93+
/** @inheritDoc */
94+
public function addConstraints(?Builder $query = null)
9995
{
96+
$query ??= $this->getRelationQuery();
10097
$localValue = $this->farParent[$this->localKey];
10198

102-
$this->performJoin();
99+
$this->performJoin($query);
103100

104101
if (static::$constraints) {
105-
$this->query->where($this->getQualifiedFirstKeyName(), '=', $localValue);
102+
$query->where($this->getQualifiedFirstKeyName(), '=', $localValue);
106103
}
107104
}
108105

@@ -114,7 +111,7 @@ public function addConstraints()
114111
*/
115112
protected function performJoin(?Builder $query = null)
116113
{
117-
$query = $query ?: $this->query;
114+
$query ??= $this->query;
118115

119116
$farKey = $this->getQualifiedFarKeyName();
120117

Diff for: ‎src/Illuminate/Database/Eloquent/Relations/HasOneThrough.php

-2
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,6 @@ public function getRelationExistenceQuery(Builder $query, Builder $parentQuery,
7777
public function addOneOfManySubQueryConstraints(Builder $query, $column = null, $aggregate = null)
7878
{
7979
$query->addSelect([$this->getQualifiedFirstKeyName()]);
80-
81-
$this->performJoin($query);
8280
}
8381

8482
/** @inheritDoc */

Diff for: ‎src/Illuminate/Database/Eloquent/Relations/MorphOneOrMany.php

+6-8
Original file line numberDiff line numberDiff line change
@@ -47,17 +47,15 @@ public function __construct(Builder $query, Model $parent, $type, $id, $localKey
4747
parent::__construct($query, $parent, $id, $localKey);
4848
}
4949

50-
/**
51-
* Set the base constraints on the relation query.
52-
*
53-
* @return void
54-
*/
55-
public function addConstraints()
50+
/** @inheritDoc */
51+
public function addConstraints(?Builder $query = null)
5652
{
53+
$query ??= $this->getRelationQuery();
54+
5755
if (static::$constraints) {
58-
$this->getRelationQuery()->where($this->morphType, $this->morphClass);
56+
$query->where($this->morphType, $this->morphClass);
5957

60-
parent::addConstraints();
58+
parent::addConstraints($query);
6159
}
6260
}
6361

Diff for: ‎src/Illuminate/Database/Eloquent/Relations/MorphToMany.php

+6-8
Original file line numberDiff line numberDiff line change
@@ -76,16 +76,14 @@ public function __construct(
7676
);
7777
}
7878

79-
/**
80-
* Set the where clause for the relation query.
81-
*
82-
* @return $this
83-
*/
84-
protected function addWhereConstraints()
79+
/** @inheritDoc */
80+
protected function addWhereConstraints(?Builder $query = null)
8581
{
86-
parent::addWhereConstraints();
82+
$query ??= $this->query;
8783

88-
$this->query->where($this->qualifyPivotColumn($this->morphType), $this->morphClass);
84+
parent::addWhereConstraints($query);
85+
86+
$query->where($this->qualifyPivotColumn($this->morphType), $this->morphClass);
8987

9088
return $this;
9189
}

Diff for: ‎src/Illuminate/Database/Eloquent/Relations/Relation.php

+2-1
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,10 @@ public static function noConstraints(Closure $callback)
125125
/**
126126
* Set the base constraints on the relation query.
127127
*
128+
* @param \Illuminate\Database\Eloquent\Builder<TRelatedModel>|null $query
128129
* @return void
129130
*/
130-
abstract public function addConstraints();
131+
abstract public function addConstraints(?Builder $query = null);
131132

132133
/**
133134
* Set the constraints for an eager load of the relation.

Diff for: ‎tests/Database/DatabaseEloquentHasOneOfManyTest.php

+11-4
Original file line numberDiff line numberDiff line change
@@ -99,12 +99,19 @@ public function testRelationNameCanBeSet()
9999
$this->assertSame('baz', $relation->getRelationName());
100100
}
101101

102+
public function testCorrectLatestOfManyQuery(): void
103+
{
104+
$user = HasOneOfManyTestUser::create();
105+
$relation = $user->latest_login();
106+
$this->assertSame('select "logins".* from "logins" inner join (select MAX("logins"."id") as "id_aggregate", "logins"."user_id" from "logins" where "logins"."user_id" = ? and "logins"."user_id" is not null group by "logins"."user_id") as "latest_login" on "latest_login"."id_aggregate" = "logins"."id" and "latest_login"."user_id" = "logins"."user_id" where "logins"."user_id" = ? and "logins"."user_id" is not null', $relation->getQuery()->toSql());
107+
}
108+
102109
public function testEagerLoadingAppliesConstraintsToInnerJoinSubQuery()
103110
{
104111
$user = HasOneOfManyTestUser::create();
105112
$relation = $user->latest_login();
106113
$relation->addEagerConstraints([$user]);
107-
$this->assertSame('select MAX("logins"."id") as "id_aggregate", "logins"."user_id" from "logins" where "logins"."user_id" in (1) group by "logins"."user_id"', $relation->getOneOfManySubQuery()->toSql());
114+
$this->assertSame('select MAX("logins"."id") as "id_aggregate", "logins"."user_id" from "logins" where "logins"."user_id" = ? and "logins"."user_id" is not null and "logins"."user_id" in (1) group by "logins"."user_id"', $relation->getOneOfManySubQuery()->toSql());
108115
}
109116

110117
public function testGlobalScopeIsNotAppliedWhenRelationIsDefinedWithoutGlobalScope()
@@ -116,7 +123,7 @@ public function testGlobalScopeIsNotAppliedWhenRelationIsDefinedWithoutGlobalSco
116123
$user = HasOneOfManyTestUser::create();
117124
$relation = $user->latest_login_without_global_scope();
118125
$relation->addEagerConstraints([$user]);
119-
$this->assertSame('select "logins".* from "logins" inner join (select MAX("logins"."id") as "id_aggregate", "logins"."user_id" from "logins" where "logins"."user_id" in (1) group by "logins"."user_id") as "latestOfMany" on "latestOfMany"."id_aggregate" = "logins"."id" and "latestOfMany"."user_id" = "logins"."user_id" where "logins"."user_id" = ? and "logins"."user_id" is not null', $relation->getQuery()->toSql());
126+
$this->assertSame('select "logins".* from "logins" inner join (select MAX("logins"."id") as "id_aggregate", "logins"."user_id" from "logins" where "logins"."user_id" = ? and "logins"."user_id" is not null and "logins"."user_id" in (1) group by "logins"."user_id") as "latestOfMany" on "latestOfMany"."id_aggregate" = "logins"."id" and "latestOfMany"."user_id" = "logins"."user_id" where "logins"."user_id" = ? and "logins"."user_id" is not null', $relation->getQuery()->toSql());
120127

121128
HasOneOfManyTestLogin::addGlobalScope('test', function ($query) {
122129
});
@@ -130,7 +137,7 @@ public function testGlobalScopeIsNotAppliedWhenRelationIsDefinedWithoutGlobalSco
130137

131138
$user = HasOneOfManyTestUser::create();
132139
$relation = $user->price_without_global_scope();
133-
$this->assertSame('select "prices".* from "prices" inner join (select max("prices"."id") as "id_aggregate", min("prices"."published_at") as "published_at_aggregate", "prices"."user_id" from "prices" inner join (select max("prices"."published_at") as "published_at_aggregate", "prices"."user_id" from "prices" where "published_at" < ? group by "prices"."user_id") as "price_without_global_scope" on "price_without_global_scope"."published_at_aggregate" = "prices"."published_at" and "price_without_global_scope"."user_id" = "prices"."user_id" where "published_at" < ? group by "prices"."user_id") as "price_without_global_scope" on "price_without_global_scope"."id_aggregate" = "prices"."id" and "price_without_global_scope"."published_at_aggregate" = "prices"."published_at" and "price_without_global_scope"."user_id" = "prices"."user_id" where "prices"."user_id" = ? and "prices"."user_id" is not null', $relation->getQuery()->toSql());
140+
$this->assertSame('select "prices".* from "prices" inner join (select max("prices"."id") as "id_aggregate", min("prices"."published_at") as "published_at_aggregate", "prices"."user_id" from "prices" inner join (select max("prices"."published_at") as "published_at_aggregate", "prices"."user_id" from "prices" where "prices"."user_id" = ? and "prices"."user_id" is not null and "published_at" < ? group by "prices"."user_id") as "price_without_global_scope" on "price_without_global_scope"."published_at_aggregate" = "prices"."published_at" and "price_without_global_scope"."user_id" = "prices"."user_id" where "prices"."user_id" = ? and "prices"."user_id" is not null and "published_at" < ? group by "prices"."user_id") as "price_without_global_scope" on "price_without_global_scope"."id_aggregate" = "prices"."id" and "price_without_global_scope"."published_at_aggregate" = "prices"."published_at" and "price_without_global_scope"."user_id" = "prices"."user_id" where "prices"."user_id" = ? and "prices"."user_id" is not null', $relation->getQuery()->toSql());
134141

135142
HasOneOfManyTestPrice::addGlobalScope('test', function ($query) {
136143
});
@@ -591,7 +598,7 @@ public function states()
591598
public function foo_state()
592599
{
593600
return $this->hasOne(HasOneOfManyTestState::class, 'user_id')->ofMany(
594-
['id' => 'max'],
601+
[], // should automatically add 'id' => 'max'
595602
function ($q) {
596603
$q->where('type', 'foo');
597604
}

Diff for: ‎tests/Database/DatabaseEloquentHasOneThroughOfManyTest.php

+5-5
Original file line numberDiff line numberDiff line change
@@ -98,23 +98,23 @@ public function testCorrectLatestOfManyQuery(): void
9898
{
9999
$user = HasOneThroughOfManyTestUser::create();
100100
$relation = $user->latest_login();
101-
$this->assertSame('select "logins".* from "logins" inner join "intermediates" on "intermediates"."id" = "logins"."intermediate_id" inner join (select MAX("logins"."id") as "id_aggregate", "intermediates"."user_id" from "logins" inner join "intermediates" on "intermediates"."id" = "logins"."intermediate_id" group by "intermediates"."user_id") as "latest_login" on "latest_login"."id_aggregate" = "logins"."id" and "latest_login"."user_id" = "intermediates"."user_id" where "intermediates"."user_id" = ?', $relation->getQuery()->toSql());
101+
$this->assertSame('select "logins".* from "logins" inner join "intermediates" on "intermediates"."id" = "logins"."intermediate_id" inner join (select MAX("logins"."id") as "id_aggregate", "intermediates"."user_id" from "logins" inner join "intermediates" on "intermediates"."id" = "logins"."intermediate_id" where "intermediates"."user_id" = ? group by "intermediates"."user_id") as "latest_login" on "latest_login"."id_aggregate" = "logins"."id" and "latest_login"."user_id" = "intermediates"."user_id" where "intermediates"."user_id" = ?', $relation->getQuery()->toSql());
102102
}
103103

104104
public function testEagerLoadingAppliesConstraintsToInnerJoinSubQuery(): void
105105
{
106106
$user = HasOneThroughOfManyTestUser::create();
107107
$relation = $user->latest_login();
108108
$relation->addEagerConstraints([$user]);
109-
$this->assertSame('select MAX("logins"."id") as "id_aggregate", "intermediates"."user_id" from "logins" inner join "intermediates" on "intermediates"."id" = "logins"."intermediate_id" where "intermediates"."user_id" in (1) group by "intermediates"."user_id"', $relation->getOneOfManySubQuery()->toSql());
109+
$this->assertSame('select MAX("logins"."id") as "id_aggregate", "intermediates"."user_id" from "logins" inner join "intermediates" on "intermediates"."id" = "logins"."intermediate_id" where "intermediates"."user_id" = ? and "intermediates"."user_id" in (1) group by "intermediates"."user_id"', $relation->getOneOfManySubQuery()->toSql());
110110
}
111111

112112
public function testEagerLoadingAppliesConstraintsToQuery(): void
113113
{
114114
$user = HasOneThroughOfManyTestUser::create();
115115
$relation = $user->latest_login();
116116
$relation->addEagerConstraints([$user]);
117-
$this->assertSame('select "logins".* from "logins" inner join "intermediates" on "intermediates"."id" = "logins"."intermediate_id" inner join (select MAX("logins"."id") as "id_aggregate", "intermediates"."user_id" from "logins" inner join "intermediates" on "intermediates"."id" = "logins"."intermediate_id" where "intermediates"."user_id" in (1) group by "intermediates"."user_id") as "latest_login" on "latest_login"."id_aggregate" = "logins"."id" and "latest_login"."user_id" = "intermediates"."user_id" where "intermediates"."user_id" = ?', $relation->getQuery()->toSql());
117+
$this->assertSame('select "logins".* from "logins" inner join "intermediates" on "intermediates"."id" = "logins"."intermediate_id" inner join (select MAX("logins"."id") as "id_aggregate", "intermediates"."user_id" from "logins" inner join "intermediates" on "intermediates"."id" = "logins"."intermediate_id" where "intermediates"."user_id" = ? and "intermediates"."user_id" in (1) group by "intermediates"."user_id") as "latest_login" on "latest_login"."id_aggregate" = "logins"."id" and "latest_login"."user_id" = "intermediates"."user_id" where "intermediates"."user_id" = ?', $relation->getQuery()->toSql());
118118
}
119119

120120
public function testGlobalScopeIsNotAppliedWhenRelationIsDefinedWithoutGlobalScope(): void
@@ -126,7 +126,7 @@ public function testGlobalScopeIsNotAppliedWhenRelationIsDefinedWithoutGlobalSco
126126
$user = HasOneThroughOfManyTestUser::create();
127127
$relation = $user->latest_login_without_global_scope();
128128
$relation->addEagerConstraints([$user]);
129-
$this->assertSame('select "logins".* from "logins" inner join "intermediates" on "intermediates"."id" = "logins"."intermediate_id" inner join (select MAX("logins"."id") as "id_aggregate", "intermediates"."user_id" from "logins" inner join "intermediates" on "intermediates"."id" = "logins"."intermediate_id" where "intermediates"."user_id" in (1) group by "intermediates"."user_id") as "latestOfMany" on "latestOfMany"."id_aggregate" = "logins"."id" and "latestOfMany"."user_id" = "intermediates"."user_id" where "intermediates"."user_id" = ?', $relation->getQuery()->toSql());
129+
$this->assertSame('select "logins".* from "logins" inner join "intermediates" on "intermediates"."id" = "logins"."intermediate_id" inner join (select MAX("logins"."id") as "id_aggregate", "intermediates"."user_id" from "logins" inner join "intermediates" on "intermediates"."id" = "logins"."intermediate_id" where "intermediates"."user_id" = ? and "intermediates"."user_id" in (1) group by "intermediates"."user_id") as "latestOfMany" on "latestOfMany"."id_aggregate" = "logins"."id" and "latestOfMany"."user_id" = "intermediates"."user_id" where "intermediates"."user_id" = ?', $relation->getQuery()->toSql());
130130

131131
HasOneThroughOfManyTestLogin::addGlobalScope('test', function ($query) {
132132
});
@@ -140,7 +140,7 @@ public function testGlobalScopeIsNotAppliedWhenRelationIsDefinedWithoutGlobalSco
140140

141141
$user = HasOneThroughOfManyTestUser::create();
142142
$relation = $user->price_without_global_scope();
143-
$this->assertSame('select "prices".* from "prices" inner join "intermediates" on "intermediates"."id" = "prices"."intermediate_id" inner join (select max("prices"."id") as "id_aggregate", min("prices"."published_at") as "published_at_aggregate", "intermediates"."user_id" from "prices" inner join "intermediates" on "intermediates"."id" = "prices"."intermediate_id" inner join (select max("prices"."published_at") as "published_at_aggregate", "intermediates"."user_id" from "prices" inner join "intermediates" on "intermediates"."id" = "prices"."intermediate_id" where "published_at" < ? group by "intermediates"."user_id") as "price_without_global_scope" on "price_without_global_scope"."published_at_aggregate" = "prices"."published_at" and "price_without_global_scope"."user_id" = "intermediates"."user_id" where "published_at" < ? group by "intermediates"."user_id") as "price_without_global_scope" on "price_without_global_scope"."id_aggregate" = "prices"."id" and "price_without_global_scope"."published_at_aggregate" = "prices"."published_at" and "price_without_global_scope"."user_id" = "intermediates"."user_id" where "intermediates"."user_id" = ?', $relation->getQuery()->toSql());
143+
$this->assertSame('select "prices".* from "prices" inner join "intermediates" on "intermediates"."id" = "prices"."intermediate_id" inner join (select max("prices"."id") as "id_aggregate", min("prices"."published_at") as "published_at_aggregate", "intermediates"."user_id" from "prices" inner join "intermediates" on "intermediates"."id" = "prices"."intermediate_id" inner join (select max("prices"."published_at") as "published_at_aggregate", "intermediates"."user_id" from "prices" inner join "intermediates" on "intermediates"."id" = "prices"."intermediate_id" where "intermediates"."user_id" = ? and "published_at" < ? group by "intermediates"."user_id") as "price_without_global_scope" on "price_without_global_scope"."published_at_aggregate" = "prices"."published_at" and "price_without_global_scope"."user_id" = "intermediates"."user_id" where "intermediates"."user_id" = ? and "published_at" < ? group by "intermediates"."user_id") as "price_without_global_scope" on "price_without_global_scope"."id_aggregate" = "prices"."id" and "price_without_global_scope"."published_at_aggregate" = "prices"."published_at" and "price_without_global_scope"."user_id" = "intermediates"."user_id" where "intermediates"."user_id" = ?', $relation->getQuery()->toSql());
144144

145145
HasOneThroughOfManyTestPrice::addGlobalScope('test', function ($query) {
146146
});

Diff for: ‎tests/Database/DatabaseEloquentInverseRelationTest.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ public function getResults()
363363
return $this->query->get();
364364
}
365365

366-
public function addConstraints()
366+
public function addConstraints(?Builder $query = null)
367367
{
368368
//
369369
}

0 commit comments

Comments
 (0)