From 6b27582ad76b0bc1b2f418340d5ef280b675b8b7 Mon Sep 17 00:00:00 2001 From: Kit Senior Date: Tue, 9 Jul 2024 16:01:27 +0100 Subject: [PATCH 1/3] make groupLimit support multiple columns --- src/Illuminate/Database/Query/Builder.php | 12 ++-- .../Database/Query/Grammars/Grammar.php | 4 +- .../Database/Query/Grammars/MySqlGrammar.php | 32 +++++++--- .../Query/Grammars/SqlServerGrammar.php | 2 +- tests/Database/DatabaseQueryBuilderTest.php | 62 +++++++++++++++++++ 5 files changed, 94 insertions(+), 18 deletions(-) diff --git a/src/Illuminate/Database/Query/Builder.php b/src/Illuminate/Database/Query/Builder.php index 84e7eca0ae85..78b8b10e746a 100755 --- a/src/Illuminate/Database/Query/Builder.php +++ b/src/Illuminate/Database/Query/Builder.php @@ -2642,7 +2642,7 @@ public function limit($value) * Add a "group limit" clause to the query. * * @param int $value - * @param string $column + * @param string|array $column * @return $this */ public function groupLimit($value, $column) @@ -3008,11 +3008,11 @@ protected function withoutGroupLimitKeys($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); + if (isset($this->groupLimit['column'])) { + foreach ((array) $this->groupLimit['column'] as $i => $column) { + $keysToRemove[] = '@laravel_group'.$i.' := '.$this->grammar->wrap($column); + $keysToRemove[] = '@laravel_group'.$i.' := '.$this->grammar->wrap('pivot_'.$column); + } } $items->each(function ($item) use ($keysToRemove) { diff --git a/src/Illuminate/Database/Query/Grammars/Grammar.php b/src/Illuminate/Database/Query/Grammars/Grammar.php index 42c9102b4c62..08cf0d40abba 100755 --- a/src/Illuminate/Database/Query/Grammars/Grammar.php +++ b/src/Illuminate/Database/Query/Grammars/Grammar.php @@ -1027,13 +1027,13 @@ protected function compileGroupLimit(Builder $query) /** * Compile a row number clause. * - * @param string $partition + * @param string|array $partition * @param string $orders * @return string */ protected function compileRowNumber($partition, $orders) { - $over = trim('partition by '.$this->wrap($partition).' '.$orders); + $over = trim('partition by '.$this->columnize((array) $partition).' '.$orders); 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 8c1521f60d31..bc6cc53aec54 100755 --- a/src/Illuminate/Database/Query/Grammars/MySqlGrammar.php +++ b/src/Illuminate/Database/Query/Grammars/MySqlGrammar.php @@ -141,18 +141,32 @@ protected function compileLegacyGroupLimit(Builder $query) $query->offset = null; } - $column = last(explode('.', $query->groupLimit['column'])); - $column = $this->wrap($column); + $groupColumns = (array) $query->groupLimit['column']; - $partition = ', @laravel_row := if(@laravel_group = '.$column.', @laravel_row + 1, 1) as `laravel_row`'; - $partition .= ', @laravel_group := '.$column; + $groupPartitions = []; + $groupSelects = []; + $groupInitialValues = []; + $groupOrders = []; + + foreach ($groupColumns as $i => $column) { + $groupPartitions[] = '@laravel_group'.$i.' = '.$this->wrap($column); + + $groupSelects[] = '@laravel_group'.$i.' := '.$this->wrap($column); + + $groupInitialValues[] = '@laravel_group'.$i.' := 0'; + + $groupOrders[] = [ + 'column' => $column, + 'direction' => 'asc', + ]; + } + + $partition = ', @laravel_row := if('.implode(' and ', $groupPartitions).', @laravel_row + 1, 1) as `laravel_row`'; + $partition .= ', '.implode(', ', $groupSelects); $orders = (array) $query->orders; - array_unshift($orders, [ - 'column' => $query->groupLimit['column'], - 'direction' => 'asc', - ]); + array_unshift($orders, ...$groupOrders); $query->orders = $orders; @@ -160,7 +174,7 @@ protected function compileLegacyGroupLimit(Builder $query) $sql = $this->concatenate($components); - $from = '(select @laravel_row := 0, @laravel_group := 0) as `laravel_vars`, ('.$sql.') as `laravel_table`'; + $from = '(select @laravel_row := 0, '.implode(', ', $groupInitialValues).') as `laravel_vars`, ('.$sql.') as `laravel_table`'; $sql = 'select `laravel_table`.*'.$partition.' from '.$from.' having `laravel_row` <= '.$limit; diff --git a/src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php b/src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php index c084308b74ba..1aceb29f4114 100755 --- a/src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php +++ b/src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php @@ -316,7 +316,7 @@ protected function compileLimit(Builder $query, $limit) /** * Compile a row number clause. * - * @param string $partition + * @param string|array $partition * @param string $orders * @return string */ diff --git a/tests/Database/DatabaseQueryBuilderTest.php b/tests/Database/DatabaseQueryBuilderTest.php index a045f41548c6..e4eb36c2f399 100755 --- a/tests/Database/DatabaseQueryBuilderTest.php +++ b/tests/Database/DatabaseQueryBuilderTest.php @@ -6243,6 +6243,68 @@ public function testToRawSql() $this->assertSame('select * from "users" where "email" = \'foo\'', $builder->toRawSql()); } + public function testGroupLimit() + { + $builder = $this->getBuilder(); + $builder->select('*')->from('users')->orderBy('foo')->groupLimit(1, 'bar'); + $this->assertSame('select * from (select *, row_number() over (partition by "bar" order by "foo" asc) as "laravel_row" from "users") as "laravel_table" where "laravel_row" <= 1 order by "laravel_row"', $builder->toSql()); + + $builder = $this->getBuilder(); + $builder->select('*')->from('users')->orderBy('foo')->groupLimit(1, ['bar', 'baz']); + $this->assertSame('select * from (select *, row_number() over (partition by "bar", "baz" order by "foo" asc) as "laravel_row" from "users") as "laravel_table" where "laravel_row" <= 1 order by "laravel_row"', $builder->toSql()); + } + + public function testGroupLimitMySql() + { + $connection = m::mock(ConnectionInterface::class); + $connection->shouldReceive('getServerVersion')->andReturn('8.0.11'); + $connection->shouldReceive('isMaria')->andReturn(false); + + $grammar = new MySqlGrammar; + $processor = m::mock(Processor::class); + + $builder = new Builder($connection, $grammar, $processor); + + $builder->select('*')->from('users')->orderBy('foo')->groupLimit(1, 'bar'); + $this->assertSame('select * from (select *, row_number() over (partition by `bar` order by `foo` asc) as `laravel_row` from `users`) as `laravel_table` where `laravel_row` <= 1 order by `laravel_row`', $builder->toSql()); + + $builder = new Builder($connection, $grammar, $processor); + + $builder->select('*')->from('users')->orderBy('foo')->groupLimit(1, ['bar', 'baz']); + $this->assertSame('select * from (select *, row_number() over (partition by `bar`, `baz` order by `foo` asc) as `laravel_row` from `users`) as `laravel_table` where `laravel_row` <= 1 order by `laravel_row`', $builder->toSql()); + } + + public function testGroupLimitMySqlLegacy() + { + $connection = m::mock(ConnectionInterface::class); + $connection->shouldReceive('getServerVersion')->andReturn('8.0.10'); + $connection->shouldReceive('isMaria')->andReturn(false); + + $grammar = new MySqlGrammar; + $processor = m::mock(Processor::class); + + $builder = new Builder($connection, $grammar, $processor); + + $builder->select('*')->from('users')->orderBy('foo')->groupLimit(1, 'bar'); + $this->assertSame('select `laravel_table`.*, @laravel_row := if(@laravel_group0 = `bar`, @laravel_row + 1, 1) as `laravel_row`, @laravel_group0 := `bar` from (select @laravel_row := 0, @laravel_group0 := 0) as `laravel_vars`, (select * from `users` order by `bar` asc, `foo` asc) as `laravel_table` having `laravel_row` <= 1 order by `laravel_row`', $builder->toSql()); + + $builder = new Builder($connection, $grammar, $processor); + + $builder->select('*')->from('users')->orderBy('foo')->groupLimit(1, ['bar', 'baz']); + $this->assertSame('select `laravel_table`.*, @laravel_row := if(@laravel_group0 = `bar` and @laravel_group1 = `baz`, @laravel_row + 1, 1) as `laravel_row`, @laravel_group0 := `bar`, @laravel_group1 := `baz` from (select @laravel_row := 0, @laravel_group0 := 0, @laravel_group1 := 0) as `laravel_vars`, (select * from `users` order by `bar` asc, `baz` asc, `foo` asc) as `laravel_table` having `laravel_row` <= 1 order by `laravel_row`', $builder->toSql()); + } + + public function testGroupLimitSqlServer() + { + $builder = $this->getSqlServerBuilder(); + $builder->select('*')->from('users')->orderBy('foo')->groupLimit(1, 'bar'); + $this->assertSame('select * from (select *, row_number() over (partition by [bar] order by [foo] asc) as [laravel_row] from [users]) as [laravel_table] where [laravel_row] <= 1 order by [laravel_row]', $builder->toSql()); + + $builder = $this->getSqlServerBuilder(); + $builder->select('*')->from('users')->groupLimit(1, 'bar'); + $this->assertSame('select * from (select *, row_number() over (partition by [bar] order by (select 0)) as [laravel_row] from [users]) as [laravel_table] where [laravel_row] <= 1 order by [laravel_row]', $builder->toSql()); + } + protected function getConnection() { $connection = m::mock(ConnectionInterface::class); From 19c909f585275b3298df79355871b007e063a073 Mon Sep 17 00:00:00 2001 From: Kit Senior Date: Tue, 9 Jul 2024 17:47:31 +0100 Subject: [PATCH 2/3] mysql 5.7 test fix --- .../Database/Query/Grammars/MySqlGrammar.php | 10 ++++++---- .../Database/EloquentEagerLoadingLimitTest.php | 6 +++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Illuminate/Database/Query/Grammars/MySqlGrammar.php b/src/Illuminate/Database/Query/Grammars/MySqlGrammar.php index bc6cc53aec54..b3b55b0b7d92 100755 --- a/src/Illuminate/Database/Query/Grammars/MySqlGrammar.php +++ b/src/Illuminate/Database/Query/Grammars/MySqlGrammar.php @@ -149,16 +149,18 @@ protected function compileLegacyGroupLimit(Builder $query) $groupOrders = []; foreach ($groupColumns as $i => $column) { - $groupPartitions[] = '@laravel_group'.$i.' = '.$this->wrap($column); - - $groupSelects[] = '@laravel_group'.$i.' := '.$this->wrap($column); - $groupInitialValues[] = '@laravel_group'.$i.' := 0'; $groupOrders[] = [ 'column' => $column, 'direction' => 'asc', ]; + + $column = $this->wrap(last(explode('.', $column))); + + $groupPartitions[] = '@laravel_group'.$i.' = '.$column; + + $groupSelects[] = '@laravel_group'.$i.' := '.$column; } $partition = ', @laravel_row := if('.implode(' and ', $groupPartitions).', @laravel_row + 1, 1) as `laravel_row`'; diff --git a/tests/Integration/Database/EloquentEagerLoadingLimitTest.php b/tests/Integration/Database/EloquentEagerLoadingLimitTest.php index feb220ed382e..68255baf5c8b 100644 --- a/tests/Integration/Database/EloquentEagerLoadingLimitTest.php +++ b/tests/Integration/Database/EloquentEagerLoadingLimitTest.php @@ -85,7 +85,7 @@ public function testBelongsToMany(): void $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]); + $this->assertArrayNotHasKey('@laravel_group0 := `user_id`', $users[0]->roles[0]); } public function testBelongsToManyWithOffset(): void @@ -107,7 +107,7 @@ public function testHasMany(): void $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]); + $this->assertArrayNotHasKey('@laravel_group0 := `user_id`', $users[0]->posts[0]); } public function testHasManyWithOffset(): void @@ -129,7 +129,7 @@ public function testHasManyThrough(): void $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]); + $this->assertArrayNotHasKey('@laravel_group0 := `user_id`', $users[0]->comments[0]); } public function testHasManyThroughWithOffset(): void From 4e91bcd707ba2da7545d9acb9b72bc9f3a131aa4 Mon Sep 17 00:00:00 2001 From: Kit Senior Date: Tue, 9 Jul 2024 17:51:47 +0100 Subject: [PATCH 3/3] withoutGroupLimitKeys fix --- src/Illuminate/Database/Query/Builder.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Illuminate/Database/Query/Builder.php b/src/Illuminate/Database/Query/Builder.php index 78b8b10e746a..503ecfc72b4b 100755 --- a/src/Illuminate/Database/Query/Builder.php +++ b/src/Illuminate/Database/Query/Builder.php @@ -3010,6 +3010,7 @@ protected function withoutGroupLimitKeys($items) if (isset($this->groupLimit['column'])) { foreach ((array) $this->groupLimit['column'] as $i => $column) { + $column = last(explode('.', $column)); $keysToRemove[] = '@laravel_group'.$i.' := '.$this->grammar->wrap($column); $keysToRemove[] = '@laravel_group'.$i.' := '.$this->grammar->wrap('pivot_'.$column); }