From 7b4f800d184a510a95d798417aa000095cb39e1c Mon Sep 17 00:00:00 2001 From: Jonas Staudenmeir Date: Mon, 15 Jan 2024 19:20:05 +0100 Subject: [PATCH 1/3] Support eager loading with limit --- .../Eloquent/Relations/BelongsToMany.php | 37 ++++ .../Eloquent/Relations/HasManyThrough.php | 37 ++++ .../Eloquent/Relations/HasOneOrMany.php | 28 +++ src/Illuminate/Database/Query/Builder.php | 46 ++++- .../Database/Query/Grammars/Grammar.php | 70 +++++++ .../Database/Query/Grammars/MySqlGrammar.php | 77 ++++++++ .../Database/Query/Grammars/SQLiteGrammar.php | 20 ++ .../Query/Grammars/SqlServerGrammar.php | 16 ++ ...EloquentBelongsToManyCreateOrFirstTest.php | 4 + ...loquentHasManyThroughCreateOrFirstTest.php | 7 + .../EloquentEagerLoadingLimitTest.php | 183 ++++++++++++++++++ 11 files changed, 524 insertions(+), 1 deletion(-) create mode 100644 tests/Integration/Database/EloquentEagerLoadingLimitTest.php diff --git a/src/Illuminate/Database/Eloquent/Relations/BelongsToMany.php b/src/Illuminate/Database/Eloquent/Relations/BelongsToMany.php index 37c698f3d80f..0a229e830398 100755 --- a/src/Illuminate/Database/Eloquent/Relations/BelongsToMany.php +++ b/src/Illuminate/Database/Eloquent/Relations/BelongsToMany.php @@ -11,6 +11,7 @@ use Illuminate\Database\Eloquent\Relations\Concerns\AsPivot; use Illuminate\Database\Eloquent\Relations\Concerns\InteractsWithDictionary; use Illuminate\Database\Eloquent\Relations\Concerns\InteractsWithPivotTable; +use Illuminate\Database\Query\Grammars\MySqlGrammar; use Illuminate\Database\UniqueConstraintViolationException; use Illuminate\Support\Str; use InvalidArgumentException; @@ -1347,6 +1348,42 @@ public function getRelationExistenceQueryForSelfJoin(Builder $query, Builder $pa return parent::getRelationExistenceQuery($query, $parentQuery, $columns); } + /** + * Alias to set the "limit" value of the query. + * + * @param int $value + * @return $this + */ + public function take($value) + { + return $this->limit($value); + } + + /** + * Set the "limit" value of the query. + * + * @param int $value + * @return $this + */ + public function limit($value) + { + if ($this->parent->exists) { + $this->query->limit($value); + } else { + $column = $this->getExistenceCompareKey(); + + $grammar = $this->query->getQuery()->getGrammar(); + + if ($grammar instanceof MySqlGrammar && $grammar->useLegacyGroupLimit($this->query->getQuery())) { + $column = 'pivot_'.last(explode('.', $column)); + } + + $this->query->groupLimit($value, $column); + } + + return $this; + } + /** * Get the key for comparing against the parent key in "has" query. * diff --git a/src/Illuminate/Database/Eloquent/Relations/HasManyThrough.php b/src/Illuminate/Database/Eloquent/Relations/HasManyThrough.php index b0b4b1fdebe1..55f9aacd1e2c 100644 --- a/src/Illuminate/Database/Eloquent/Relations/HasManyThrough.php +++ b/src/Illuminate/Database/Eloquent/Relations/HasManyThrough.php @@ -10,6 +10,7 @@ use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Database\Eloquent\Relations\Concerns\InteractsWithDictionary; use Illuminate\Database\Eloquent\SoftDeletes; +use Illuminate\Database\Query\Grammars\MySqlGrammar; use Illuminate\Database\UniqueConstraintViolationException; class HasManyThrough extends Relation @@ -762,6 +763,42 @@ public function getRelationExistenceQueryForThroughSelfRelation(Builder $query, ); } + /** + * Alias to set the "limit" value of the query. + * + * @param int $value + * @return $this + */ + public function take($value) + { + return $this->limit($value); + } + + /** + * Set the "limit" value of the query. + * + * @param int $value + * @return $this + */ + public function limit($value) + { + if ($this->farParent->exists) { + $this->query->limit($value); + } else { + $column = $this->getQualifiedFirstKeyName(); + + $grammar = $this->query->getQuery()->getGrammar(); + + if ($grammar instanceof MySqlGrammar && $grammar->useLegacyGroupLimit($this->query->getQuery())) { + $column = 'laravel_through_key'; + } + + $this->query->groupLimit($value, $column); + } + + return $this; + } + /** * Get the qualified foreign key on the related model. * diff --git a/src/Illuminate/Database/Eloquent/Relations/HasOneOrMany.php b/src/Illuminate/Database/Eloquent/Relations/HasOneOrMany.php index af263baf854f..e1d295d86be4 100755 --- a/src/Illuminate/Database/Eloquent/Relations/HasOneOrMany.php +++ b/src/Illuminate/Database/Eloquent/Relations/HasOneOrMany.php @@ -453,6 +453,34 @@ public function getRelationExistenceQueryForSelfRelation(Builder $query, Builder ); } + /** + * Alias to set the "limit" value of the query. + * + * @param int $value + * @return $this + */ + public function take($value) + { + return $this->limit($value); + } + + /** + * Set the "limit" value of the query. + * + * @param int $value + * @return $this + */ + public function limit($value) + { + if ($this->parent->exists) { + $this->query->limit($value); + } else { + $this->query->groupLimit($value, $this->getExistenceCompareKey()); + } + + return $this; + } + /** * Get the key for comparing against the parent key in "has" query. * diff --git a/src/Illuminate/Database/Query/Builder.php b/src/Illuminate/Database/Query/Builder.php index 694c8c720db0..d857b62c3836 100755 --- a/src/Illuminate/Database/Query/Builder.php +++ b/src/Illuminate/Database/Query/Builder.php @@ -152,6 +152,13 @@ class Builder implements BuilderContract */ public $limit; + /** + * The maximum number of records to return per group. + * + * @var array + */ + public $groupLimit; + /** * The number of records to skip. * @@ -2444,6 +2451,22 @@ public function limit($value) return $this; } + /** + * Add a "group limit" clause to the query. + * + * @param int $value + * @param string $column + * @return $this + */ + public function groupLimit($value, $column) + { + if ($value >= 0) { + $this->groupLimit = compact('value', 'column'); + } + + return $this; + } + /** * Set the limit and offset for a given page. * @@ -2737,9 +2760,30 @@ public function soleValue($column) */ public function get($columns = ['*']) { - return collect($this->onceWithColumns(Arr::wrap($columns), function () { + $items = collect($this->onceWithColumns(Arr::wrap($columns), function () { return $this->processor->processSelect($this, $this->runSelect()); })); + + if (!isset($this->groupLimit)) { + return $items; + } + + $keysToRemove = ['laravel_row']; + + if (is_string($this->groupLimit['column'])) { + $column = last(explode('.', $this->groupLimit['column'])); + + $keysToRemove[] = '@laravel_group := '.$this->grammar->wrap($column); + $keysToRemove[] = '@laravel_group := '.$this->grammar->wrap('pivot_'.$column); + } + + $items->each(function ($item) use ($keysToRemove) { + foreach ($keysToRemove as $key) { + unset($item->$key); + } + }); + + return $items; } /** diff --git a/src/Illuminate/Database/Query/Grammars/Grammar.php b/src/Illuminate/Database/Query/Grammars/Grammar.php index 3b4f117693f6..fbf48220ec5b 100755 --- a/src/Illuminate/Database/Query/Grammars/Grammar.php +++ b/src/Illuminate/Database/Query/Grammars/Grammar.php @@ -60,6 +60,14 @@ public function compileSelect(Builder $query) return $this->compileUnionAggregate($query); } + if (isset($query->groupLimit)) { + if (is_null($query->columns)) { + $query->columns = ['*']; + } + + return $this->compileGroupLimit($query); + } + // If the query does not have any columns set, we'll set the columns to the // * character to just get all of the columns from the database. Then we // can build the query and concatenate all the pieces together as one. @@ -917,6 +925,68 @@ protected function compileLimit(Builder $query, $limit) return 'limit '.(int) $limit; } + /** + * Compile a group limit clause. + * + * @param \Illuminate\Database\Query\Builder $query + * @return string + */ + protected function compileGroupLimit(Builder $query) + { + $selectBindings = array_merge($query->getRawBindings()['select'], $query->getRawBindings()['order']); + + $query->setBindings($selectBindings, 'select'); + $query->setBindings([], 'order'); + + $limit = (int) $query->groupLimit['value']; + + $offset = $query->offset; + + if (isset($offset)) { + $offset = (int) $offset; + $limit += $offset; + + $query->offset = null; + } + + $components = $this->compileComponents($query); + + $components['columns'] .= $this->compileRowNumber($query->groupLimit['column'], $components['orders'] ?? ''); + + unset($components['orders']); + + $table = $this->wrap('laravel_table'); + $row = $this->wrap('laravel_row'); + + $sql = $this->concatenate($components); + + $sql = 'select * from ('.$sql.') as '.$table.' where '.$row.' <= '.$limit; + + if (isset($offset)) { + $sql .= ' and '.$row.' > '.$offset; + } + + return $sql.' order by '.$row; + } + + /** + * Compile a row number clause. + * + * @param string $partition + * @param string $orders + * @return string + */ + protected function compileRowNumber($partition, $orders) + { + $partition = 'partition by '.$this->wrap($partition); + + $over = trim($partition.' '.$orders); + + $row = $this->wrap('laravel_row'); + + return ', row_number() over ('.$over.') as '.$row; + } + /** * Compile the "offset" portions of the query. * diff --git a/src/Illuminate/Database/Query/Grammars/MySqlGrammar.php b/src/Illuminate/Database/Query/Grammars/MySqlGrammar.php index b9d2a624bd9a..c4cccccdfc98 100755 --- a/src/Illuminate/Database/Query/Grammars/MySqlGrammar.php +++ b/src/Illuminate/Database/Query/Grammars/MySqlGrammar.php @@ -4,6 +4,7 @@ use Illuminate\Database\Query\Builder; use Illuminate\Support\Str; +use PDO; class MySqlGrammar extends Grammar { @@ -94,6 +95,82 @@ protected function compileIndexHint(Builder $query, $indexHint) }; } + /** + * Compile a group limit clause. + * + * @param \Illuminate\Database\Query\Builder $query + * @return string + */ + protected function compileGroupLimit(Builder $query) + { + return $this->useLegacyGroupLimit($query) + ? $this->compileLegacyGroupLimit($query) + : parent::compileGroupLimit($query); + } + + /** + * Determine whether to use a legacy group limit clause for MySQL < 8.0. + * + * @param \Illuminate\Database\Query\Builder $query + * @return bool + */ + public function useLegacyGroupLimit(Builder $query) + { + $version = $query->getConnection()->getReadPdo()->getAttribute(PDO::ATTR_SERVER_VERSION); + + return ! $query->getConnection()->isMaria() && version_compare($version, '8.0.11') < 0; + } + + /** + * Compile a group limit clause for MySQL < 8.0. + * + * Derived from https://softonsofa.com/tweaking-eloquent-relations-how-to-get-n-related-models-per-parent/. + * + * @param \Illuminate\Database\Query\Builder $query + * @return string + */ + protected function compileLegacyGroupLimit(Builder $query) + { + $limit = (int) $query->groupLimit['value']; + + $offset = $query->offset; + + if (isset($offset)) { + $offset = (int) $offset; + $limit += $offset; + + $query->offset = null; + } + + $column = last(explode('.', $query->groupLimit['column'])); + + $column = $this->wrap($column); + + $partition = ', @laravel_row := if(@laravel_group = '.$column.', @laravel_row + 1, 1) as `laravel_row`'; + + $partition .= ', @laravel_group := '.$column; + + $orders = (array) $query->orders; + + array_unshift($orders, ['column' => $query->groupLimit['column'], 'direction' => 'asc']); + + $query->orders = $orders; + + $components = $this->compileComponents($query); + + $sql = $this->concatenate($components); + + $from = '(select @laravel_row := 0, @laravel_group := 0) as `laravel_vars`, ('.$sql.') as `laravel_table`'; + + $sql = 'select `laravel_table`.*'.$partition.' from '.$from.' having `laravel_row` <= '.$limit; + + if (isset($offset)) { + $sql .= ' and `laravel_row` > '.$offset; + } + + return $sql.' order by `laravel_row`'; + } + /** * Compile an insert ignore statement into SQL. * diff --git a/src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php b/src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php index b628d70d2b02..72b591ea5715 100755 --- a/src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php +++ b/src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php @@ -5,6 +5,7 @@ use Illuminate\Database\Query\Builder; use Illuminate\Support\Arr; use Illuminate\Support\Str; +use PDO; class SQLiteGrammar extends Grammar { @@ -184,6 +185,25 @@ protected function compileJsonContainsKey($column) return 'json_type('.$field.$path.') is not null'; } + /** + * Compile a group limit clause. + * + * @param \Illuminate\Database\Query\Builder $query + * @return string + */ + protected function compileGroupLimit(Builder $query) + { + $version = $query->getConnection()->getReadPdo()->getAttribute(PDO::ATTR_SERVER_VERSION); + + if (version_compare($version, '3.25.0') >= 0) { + return parent::compileGroupLimit($query); + } + + $query->groupLimit = null; + + return $this->compileSelect($query); + } + /** * Compile an update statement into SQL. * diff --git a/src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php b/src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php index f68722a64bce..0085f758d431 100755 --- a/src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php +++ b/src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php @@ -312,6 +312,22 @@ protected function compileLimit(Builder $query, $limit) return ''; } + /** + * Compile a row number clause. + * + * @param string $partition + * @param string $orders + * @return string + */ + protected function compileRowNumber($partition, $orders) + { + if (empty($orders)) { + $orders = 'order by (select 0)'; + } + + return parent::compileRowNumber($partition, $orders); + } + /** * Compile the "offset" portions of the query. * diff --git a/tests/Database/DatabaseEloquentBelongsToManyCreateOrFirstTest.php b/tests/Database/DatabaseEloquentBelongsToManyCreateOrFirstTest.php index 047dc138a5e9..3ea77e45aedc 100644 --- a/tests/Database/DatabaseEloquentBelongsToManyCreateOrFirstTest.php +++ b/tests/Database/DatabaseEloquentBelongsToManyCreateOrFirstTest.php @@ -114,6 +114,7 @@ public function testFirstOrCreateMethodRetrievesExistingRelatedAlreadyAssociated { $source = new BelongsToManyCreateOrFirstTestSourceModel(); $source->id = 123; + $source->exists = true; $this->mockConnectionForModels( [$source, new BelongsToManyCreateOrFirstTestRelatedModel()], 'SQLite', @@ -157,6 +158,7 @@ public function testCreateOrFirstMethodRetrievesExistingRelatedAssociatedJustNow { $source = new BelongsToManyCreateOrFirstTestSourceModel(); $source->id = 123; + $source->exists = true; $this->mockConnectionForModels( [$source, new BelongsToManyCreateOrFirstTestRelatedModel()], 'SQLite', @@ -227,6 +229,7 @@ public function testFirstOrCreateMethodRetrievesExistingRelatedAndAssociatesIt() { $source = new BelongsToManyCreateOrFirstTestSourceModel(); $source->id = 123; + $source->exists = true; $this->mockConnectionForModels( [$source, new BelongsToManyCreateOrFirstTestRelatedModel()], 'SQLite', @@ -309,6 +312,7 @@ protected function newBelongsToMany(Builder $query, Model $parent, $table, $fore } }; $source->id = 123; + $source->exists = true; $this->mockConnectionForModels( [$source, new BelongsToManyCreateOrFirstTestRelatedModel()], 'SQLite', diff --git a/tests/Database/DatabaseEloquentHasManyThroughCreateOrFirstTest.php b/tests/Database/DatabaseEloquentHasManyThroughCreateOrFirstTest.php index bb1142027c5a..1b364e57b9f7 100644 --- a/tests/Database/DatabaseEloquentHasManyThroughCreateOrFirstTest.php +++ b/tests/Database/DatabaseEloquentHasManyThroughCreateOrFirstTest.php @@ -56,6 +56,7 @@ public function testCreateOrFirstMethodRetrievesExistingRecord(): void { $parent = new HasManyThroughCreateOrFirstTestParentModel(); $parent->id = 123; + $parent->exists = true; $this->mockConnectionForModel($parent, 'SQLite'); $parent->getConnection()->shouldReceive('transactionLevel')->andReturn(0); $parent->getConnection()->shouldReceive('getName')->andReturn('sqlite'); @@ -102,6 +103,7 @@ public function testFirstOrCreateMethodCreatesNewRecord(): void { $parent = new HasManyThroughCreateOrFirstTestParentModel(); $parent->id = 123; + $parent->exists = true; $this->mockConnectionForModel($parent, 'SQLite', [789]); $parent->getConnection()->shouldReceive('transactionLevel')->andReturn(0); $parent->getConnection()->shouldReceive('getName')->andReturn('sqlite'); @@ -135,6 +137,7 @@ public function testFirstOrCreateMethodRetrievesExistingRecord(): void { $parent = new HasManyThroughCreateOrFirstTestParentModel(); $parent->id = 123; + $parent->exists = true; $this->mockConnectionForModel($parent, 'SQLite'); $parent->getConnection()->shouldReceive('transactionLevel')->andReturn(0); $parent->getConnection()->shouldReceive('getName')->andReturn('sqlite'); @@ -173,6 +176,7 @@ public function testFirstOrCreateMethodRetrievesRecordCreatedJustNow(): void { $parent = new HasManyThroughCreateOrFirstTestParentModel(); $parent->id = 123; + $parent->exists = true; $this->mockConnectionForModel($parent, 'SQLite'); $parent->getConnection()->shouldReceive('transactionLevel')->andReturn(0); $parent->getConnection()->shouldReceive('getName')->andReturn('sqlite'); @@ -228,6 +232,7 @@ public function testUpdateOrCreateMethodCreatesNewRecord(): void { $parent = new HasManyThroughCreateOrFirstTestParentModel(); $parent->id = 123; + $parent->exists = true; $this->mockConnectionForModel($parent, 'SQLite', [789]); $parent->getConnection()->shouldReceive('transactionLevel')->andReturn(0); $parent->getConnection()->shouldReceive('getName')->andReturn('sqlite'); @@ -264,6 +269,7 @@ public function testUpdateOrCreateMethodUpdatesExistingRecord(): void { $parent = new HasManyThroughCreateOrFirstTestParentModel(); $parent->id = 123; + $parent->exists = true; $this->mockConnectionForModel($parent, 'SQLite'); $parent->getConnection()->shouldReceive('transactionLevel')->andReturn(0); $parent->getConnection()->shouldReceive('getName')->andReturn('sqlite'); @@ -310,6 +316,7 @@ public function testUpdateOrCreateMethodUpdatesRecordCreatedJustNow(): void { $parent = new HasManyThroughCreateOrFirstTestParentModel(); $parent->id = 123; + $parent->exists = true; $this->mockConnectionForModel($parent, 'SQLite'); $parent->getConnection()->shouldReceive('transactionLevel')->andReturn(0); $parent->getConnection()->shouldReceive('getName')->andReturn('sqlite'); diff --git a/tests/Integration/Database/EloquentEagerLoadingLimitTest.php b/tests/Integration/Database/EloquentEagerLoadingLimitTest.php new file mode 100644 index 000000000000..feb220ed382e --- /dev/null +++ b/tests/Integration/Database/EloquentEagerLoadingLimitTest.php @@ -0,0 +1,183 @@ +id(); + }); + + Schema::create('posts', function (Blueprint $table) { + $table->id(); + $table->unsignedBigInteger('user_id'); + $table->timestamps(); + }); + + Schema::create('comments', function (Blueprint $table) { + $table->id(); + $table->unsignedBigInteger('post_id'); + $table->timestamps(); + }); + + Schema::create('roles', function (Blueprint $table) { + $table->id(); + $table->timestamps(); + }); + + Schema::create('role_user', function (Blueprint $table) { + $table->unsignedBigInteger('role_id'); + $table->unsignedBigInteger('user_id'); + }); + + User::create(); + User::create(); + + Post::create(['user_id' => 1, 'created_at' => new Carbon('2024-01-01 00:00:01')]); + Post::create(['user_id' => 1, 'created_at' => new Carbon('2024-01-01 00:00:02')]); + Post::create(['user_id' => 1, 'created_at' => new Carbon('2024-01-01 00:00:03')]); + Post::create(['user_id' => 2, 'created_at' => new Carbon('2024-01-01 00:00:04')]); + Post::create(['user_id' => 2, 'created_at' => new Carbon('2024-01-01 00:00:05')]); + Post::create(['user_id' => 2, 'created_at' => new Carbon('2024-01-01 00:00:06')]); + + Comment::create(['post_id' => 1, 'created_at' => new Carbon('2024-01-01 00:00:01')]); + Comment::create(['post_id' => 2, 'created_at' => new Carbon('2024-01-01 00:00:02')]); + Comment::create(['post_id' => 3, 'created_at' => new Carbon('2024-01-01 00:00:03')]); + Comment::create(['post_id' => 4, 'created_at' => new Carbon('2024-01-01 00:00:04')]); + Comment::create(['post_id' => 5, 'created_at' => new Carbon('2024-01-01 00:00:05')]); + Comment::create(['post_id' => 6, 'created_at' => new Carbon('2024-01-01 00:00:06')]); + + Role::create(['created_at' => new Carbon('2024-01-01 00:00:01')]); + Role::create(['created_at' => new Carbon('2024-01-01 00:00:02')]); + Role::create(['created_at' => new Carbon('2024-01-01 00:00:03')]); + Role::create(['created_at' => new Carbon('2024-01-01 00:00:04')]); + Role::create(['created_at' => new Carbon('2024-01-01 00:00:05')]); + Role::create(['created_at' => new Carbon('2024-01-01 00:00:06')]); + + DB::table('role_user')->insert([ + ['role_id' => 1, 'user_id' => 1], + ['role_id' => 2, 'user_id' => 1], + ['role_id' => 3, 'user_id' => 1], + ['role_id' => 4, 'user_id' => 2], + ['role_id' => 5, 'user_id' => 2], + ['role_id' => 6, 'user_id' => 2], + ]); + } + + public function testBelongsToMany(): void + { + $users = User::with(['roles' => fn ($query) => $query->latest()->limit(2)]) + ->orderBy('id') + ->get(); + + $this->assertEquals([3, 2], $users[0]->roles->pluck('id')->all()); + $this->assertEquals([6, 5], $users[1]->roles->pluck('id')->all()); + $this->assertArrayNotHasKey('laravel_row', $users[0]->roles[0]); + $this->assertArrayNotHasKey('@laravel_group := `user_id`', $users[0]->roles[0]); + } + + public function testBelongsToManyWithOffset(): void + { + $users = User::with(['roles' => fn ($query) => $query->latest()->limit(2)->offset(1)]) + ->orderBy('id') + ->get(); + + $this->assertEquals([2, 1], $users[0]->roles->pluck('id')->all()); + $this->assertEquals([5, 4], $users[1]->roles->pluck('id')->all()); + } + + public function testHasMany(): void + { + $users = User::with(['posts' => fn ($query) => $query->latest()->limit(2)]) + ->orderBy('id') + ->get(); + + $this->assertEquals([3, 2], $users[0]->posts->pluck('id')->all()); + $this->assertEquals([6, 5], $users[1]->posts->pluck('id')->all()); + $this->assertArrayNotHasKey('laravel_row', $users[0]->posts[0]); + $this->assertArrayNotHasKey('@laravel_group := `user_id`', $users[0]->posts[0]); + } + + public function testHasManyWithOffset(): void + { + $users = User::with(['posts' => fn ($query) => $query->latest()->limit(2)->offset(1)]) + ->orderBy('id') + ->get(); + + $this->assertEquals([2, 1], $users[0]->posts->pluck('id')->all()); + $this->assertEquals([5, 4], $users[1]->posts->pluck('id')->all()); + } + + public function testHasManyThrough(): void + { + $users = User::with(['comments' => fn ($query) => $query->latest('comments.created_at')->limit(2)]) + ->orderBy('id') + ->get(); + + $this->assertEquals([3, 2], $users[0]->comments->pluck('id')->all()); + $this->assertEquals([6, 5], $users[1]->comments->pluck('id')->all()); + $this->assertArrayNotHasKey('laravel_row', $users[0]->comments[0]); + $this->assertArrayNotHasKey('@laravel_group := `user_id`', $users[0]->comments[0]); + } + + public function testHasManyThroughWithOffset(): void + { + $users = User::with(['comments' => fn ($query) => $query->latest('comments.created_at')->limit(2)->offset(1)]) + ->orderBy('id') + ->get(); + + $this->assertEquals([2, 1], $users[0]->comments->pluck('id')->all()); + $this->assertEquals([5, 4], $users[1]->comments->pluck('id')->all()); + } +} + +class Comment extends Model +{ + public $timestamps = false; + + protected $guarded = []; +} + +class Post extends Model +{ + protected $guarded = []; +} + +class Role extends Model +{ + protected $guarded = []; +} + +class User extends Model +{ + public $timestamps = false; + + protected $guarded = []; + + public function comments(): HasManyThrough + { + return $this->hasManyThrough(Comment::class, Post::class); + } + + public function posts(): HasMany + { + return $this->hasMany(Post::class); + } + + public function roles(): BelongsToMany + { + return $this->belongsToMany(Role::class); + } +} From 0d4f4d323093f74c8c0a6385f73b88383548efe5 Mon Sep 17 00:00:00 2001 From: Jonas Staudenmeir Date: Mon, 15 Jan 2024 20:01:24 +0100 Subject: [PATCH 2/3] Fix style --- src/Illuminate/Database/Query/Builder.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Database/Query/Builder.php b/src/Illuminate/Database/Query/Builder.php index d857b62c3836..e47073324785 100755 --- a/src/Illuminate/Database/Query/Builder.php +++ b/src/Illuminate/Database/Query/Builder.php @@ -2764,7 +2764,7 @@ public function get($columns = ['*']) return $this->processor->processSelect($this, $this->runSelect()); })); - if (!isset($this->groupLimit)) { + if (! isset($this->groupLimit)) { return $items; } From 3384acc0dd12497d50c7a9fe0acf25e4f846520b Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Mon, 15 Jan 2024 16:38:22 -0600 Subject: [PATCH 3/3] formatting --- src/Illuminate/Database/Query/Builder.php | 39 ++++++++++++------- .../Database/Query/Grammars/Grammar.php | 17 ++++---- .../Database/Query/Grammars/MySqlGrammar.php | 8 ++-- 3 files changed, 37 insertions(+), 27 deletions(-) diff --git a/src/Illuminate/Database/Query/Builder.php b/src/Illuminate/Database/Query/Builder.php index e47073324785..35337871f142 100755 --- a/src/Illuminate/Database/Query/Builder.php +++ b/src/Illuminate/Database/Query/Builder.php @@ -2764,10 +2764,31 @@ public function get($columns = ['*']) return $this->processor->processSelect($this, $this->runSelect()); })); - if (! isset($this->groupLimit)) { - return $items; - } + return isset($this->groupLimit) + ? $this->withoutGroupLimitKeys($items) + : $items; + } + + /** + * Run the query as a "select" statement against the connection. + * + * @return array + */ + protected function runSelect() + { + return $this->connection->select( + $this->toSql(), $this->getBindings(), ! $this->useWritePdo + ); + } + /** + * Remove the group limit keys from the results in the collection. + * + * @param \Illuminate\Support\Collection $items + * @return \Illuminate\Support\Collection + */ + protected function withoutGroupLimitKeys($items) + { $keysToRemove = ['laravel_row']; if (is_string($this->groupLimit['column'])) { @@ -2786,18 +2807,6 @@ public function get($columns = ['*']) return $items; } - /** - * Run the query as a "select" statement against the connection. - * - * @return array - */ - protected function runSelect() - { - return $this->connection->select( - $this->toSql(), $this->getBindings(), ! $this->useWritePdo - ); - } - /** * Paginate the given query into a simple paginator. * diff --git a/src/Illuminate/Database/Query/Grammars/Grammar.php b/src/Illuminate/Database/Query/Grammars/Grammar.php index fbf48220ec5b..7e52c10c57fa 100755 --- a/src/Illuminate/Database/Query/Grammars/Grammar.php +++ b/src/Illuminate/Database/Query/Grammars/Grammar.php @@ -60,6 +60,9 @@ public function compileSelect(Builder $query) return $this->compileUnionAggregate($query); } + // If a "group limit" is in place, we will need to compile the SQL to use a + // different syntax. This primarily supports limits on eager loads using + // Eloquent. We'll also set the columns if they have not been defined. if (isset($query->groupLimit)) { if (is_null($query->columns)) { $query->columns = ['*']; @@ -939,7 +942,6 @@ protected function compileGroupLimit(Builder $query) $query->setBindings([], 'order'); $limit = (int) $query->groupLimit['value']; - $offset = $query->offset; if (isset($offset)) { @@ -951,7 +953,10 @@ protected function compileGroupLimit(Builder $query) $components = $this->compileComponents($query); - $components['columns'] .= $this->compileRowNumber($query->groupLimit['column'], $components['orders'] ?? ''); + $components['columns'] .= $this->compileRowNumber( + $query->groupLimit['column'], + $components['orders'] ?? '' + ); unset($components['orders']); @@ -978,13 +983,9 @@ protected function compileGroupLimit(Builder $query) */ protected function compileRowNumber($partition, $orders) { - $partition = 'partition by '.$this->wrap($partition); - - $over = trim($partition.' '.$orders); - - $row = $this->wrap('laravel_row'); + $over = trim('partition by '.$this->wrap($partition).' '.$orders); - return ', row_number() over ('.$over.') as '.$row; + return ', row_number() over ('.$over.') as '.$this->wrap('laravel_row'); } /** diff --git a/src/Illuminate/Database/Query/Grammars/MySqlGrammar.php b/src/Illuminate/Database/Query/Grammars/MySqlGrammar.php index c4cccccdfc98..296d02bf066d 100755 --- a/src/Illuminate/Database/Query/Grammars/MySqlGrammar.php +++ b/src/Illuminate/Database/Query/Grammars/MySqlGrammar.php @@ -132,7 +132,6 @@ public function useLegacyGroupLimit(Builder $query) protected function compileLegacyGroupLimit(Builder $query) { $limit = (int) $query->groupLimit['value']; - $offset = $query->offset; if (isset($offset)) { @@ -143,16 +142,17 @@ protected function compileLegacyGroupLimit(Builder $query) } $column = last(explode('.', $query->groupLimit['column'])); - $column = $this->wrap($column); $partition = ', @laravel_row := if(@laravel_group = '.$column.', @laravel_row + 1, 1) as `laravel_row`'; - $partition .= ', @laravel_group := '.$column; $orders = (array) $query->orders; - array_unshift($orders, ['column' => $query->groupLimit['column'], 'direction' => 'asc']); + array_unshift($orders, [ + 'column' => $query->groupLimit['column'], + 'direction' => 'asc' + ]); $query->orders = $orders;