From 7140bb1834b23498f6a1d05294d7e773dd8597cc Mon Sep 17 00:00:00 2001 From: joelharkes Date: Wed, 1 Dec 2021 22:51:17 +0100 Subject: [PATCH 1/3] Add proper paging offset when possible to sql server --- .../Database/Query/Grammars/SqlServerGrammar.php | 14 ++++++++++---- tests/Database/DatabaseQueryBuilderTest.php | 10 +++++----- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php b/src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php index 0f5a27a66145..31942e22bead 100755 --- a/src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php +++ b/src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php @@ -31,15 +31,21 @@ public function compileSelect(Builder $query) return parent::compileSelect($query); } - // If an offset is present on the query, we will need to wrap the query in - // a big "ANSI" offset syntax block. This is very nasty compared to the - // other database systems but is necessary for implementing features. if (is_null($query->columns)) { $query->columns = ['*']; } + // For order by queries we can paginate, to avoid sorting issues. + $components = $this->compileComponents($query); + if (!empty($components['orders'])) { + return parent::compileSelect($query) . " OFFSET {$query->offset} ROWS FETCH NEXT {$query->limit} ROWS ONLY"; + } + + // If an offset is present on the query, we will need to wrap the query in + // a big "ANSI" offset syntax block. This is very nasty compared to the + // other database systems but is necessary for implementing features. return $this->compileAnsiOffset( - $query, $this->compileComponents($query) + $query, $components ); } diff --git a/tests/Database/DatabaseQueryBuilderTest.php b/tests/Database/DatabaseQueryBuilderTest.php index 98a42ff88f09..d0b77c0f3093 100755 --- a/tests/Database/DatabaseQueryBuilderTest.php +++ b/tests/Database/DatabaseQueryBuilderTest.php @@ -1221,7 +1221,7 @@ public function testOrderBysSqlServer() $builder = $this->getSqlServerBuilder(); $builder->select('*')->from('users')->skip(25)->take(10)->orderByRaw('[email] desc'); - $this->assertSame('select * from (select *, row_number() over (order by [email] desc) as row_num from [users]) as temp_table where row_num between 26 and 35 order by row_num', $builder->toSql()); + $this->assertSame('select * from [users] order by [email] desc OFFSET 25 ROWS FETCH NEXT 10 ROWS ONLY', $builder->toSql()); } public function testReorder() @@ -3109,8 +3109,8 @@ public function testSqlServerLimitsAndOffsets() $this->assertSame('select * from (select *, row_number() over (order by (select 0)) as row_num from [users]) as temp_table where row_num between 11 and 20 order by row_num', $builder->toSql()); $builder = $this->getSqlServerBuilder(); - $builder->select('*')->from('users')->skip(10)->take(10)->orderBy('email', 'desc'); - $this->assertSame('select * from (select *, row_number() over (order by [email] desc) as row_num from [users]) as temp_table where row_num between 11 and 20 order by row_num', $builder->toSql()); + $builder->select('*')->from('users')->skip(11)->take(10)->orderBy('email', 'desc'); + $this->assertSame('select * from [users] order by [email] desc OFFSET 11 ROWS FETCH NEXT 10 ROWS ONLY', $builder->toSql()); $builder = $this->getSqlServerBuilder(); $subQueryBuilder = $this->getSqlServerBuilder(); @@ -3118,8 +3118,8 @@ public function testSqlServerLimitsAndOffsets() return $query->select('created_at')->from('logins')->where('users.name', 'nameBinding')->whereColumn('user_id', 'users.id')->limit(1); }; $builder->select('*')->from('users')->where('email', 'emailBinding')->orderBy($subQuery)->skip(10)->take(10); - $this->assertSame('select * from (select *, row_number() over (order by (select top 1 [created_at] from [logins] where [users].[name] = ? and [user_id] = [users].[id]) asc) as row_num from [users] where [email] = ?) as temp_table where row_num between 11 and 20 order by row_num', $builder->toSql()); - $this->assertEquals(['nameBinding', 'emailBinding'], $builder->getBindings()); + $this->assertSame('select * from [users] where [email] = ? order by (select top 1 [created_at] from [logins] where [users].[name] = ? and [user_id] = [users].[id]) asc OFFSET 10 ROWS FETCH NEXT 10 ROWS ONLY', $builder->toSql()); + $this->assertEquals(['emailBinding', 'nameBinding'], $builder->getBindings()); $builder = $this->getSqlServerBuilder(); $builder->select('*')->from('users')->take('foo'); From d76d5f5dc2f542bbda2452b292277c0794674b07 Mon Sep 17 00:00:00 2001 From: joelharkes Date: Thu, 2 Dec 2021 09:09:37 +0100 Subject: [PATCH 2/3] update styling --- src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php b/src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php index 31942e22bead..f195ded3ca8f 100755 --- a/src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php +++ b/src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php @@ -37,8 +37,8 @@ public function compileSelect(Builder $query) // For order by queries we can paginate, to avoid sorting issues. $components = $this->compileComponents($query); - if (!empty($components['orders'])) { - return parent::compileSelect($query) . " OFFSET {$query->offset} ROWS FETCH NEXT {$query->limit} ROWS ONLY"; + if (! empty($components['orders'])) { + return parent::compileSelect($query)." OFFSET {$query->offset} ROWS FETCH NEXT {$query->limit} ROWS ONLY"; } // If an offset is present on the query, we will need to wrap the query in From 6142fc8b070d6d01bbdb71a607656358e0ba7d19 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Thu, 2 Dec 2021 14:50:14 -0600 Subject: [PATCH 3/3] formatting --- src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php | 4 ++-- tests/Database/DatabaseQueryBuilderTest.php | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php b/src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php index f195ded3ca8f..3fce201bd28c 100755 --- a/src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php +++ b/src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php @@ -35,10 +35,10 @@ public function compileSelect(Builder $query) $query->columns = ['*']; } - // For order by queries we can paginate, to avoid sorting issues. $components = $this->compileComponents($query); + if (! empty($components['orders'])) { - return parent::compileSelect($query)." OFFSET {$query->offset} ROWS FETCH NEXT {$query->limit} ROWS ONLY"; + return parent::compileSelect($query)." offset {$query->offset} rows fetch next {$query->limit} rows only"; } // If an offset is present on the query, we will need to wrap the query in diff --git a/tests/Database/DatabaseQueryBuilderTest.php b/tests/Database/DatabaseQueryBuilderTest.php index d0b77c0f3093..4064664fa4b7 100755 --- a/tests/Database/DatabaseQueryBuilderTest.php +++ b/tests/Database/DatabaseQueryBuilderTest.php @@ -1221,7 +1221,7 @@ public function testOrderBysSqlServer() $builder = $this->getSqlServerBuilder(); $builder->select('*')->from('users')->skip(25)->take(10)->orderByRaw('[email] desc'); - $this->assertSame('select * from [users] order by [email] desc OFFSET 25 ROWS FETCH NEXT 10 ROWS ONLY', $builder->toSql()); + $this->assertSame('select * from [users] order by [email] desc offset 25 rows fetch next 10 rows only', $builder->toSql()); } public function testReorder() @@ -3110,7 +3110,7 @@ public function testSqlServerLimitsAndOffsets() $builder = $this->getSqlServerBuilder(); $builder->select('*')->from('users')->skip(11)->take(10)->orderBy('email', 'desc'); - $this->assertSame('select * from [users] order by [email] desc OFFSET 11 ROWS FETCH NEXT 10 ROWS ONLY', $builder->toSql()); + $this->assertSame('select * from [users] order by [email] desc offset 11 rows fetch next 10 rows only', $builder->toSql()); $builder = $this->getSqlServerBuilder(); $subQueryBuilder = $this->getSqlServerBuilder(); @@ -3118,7 +3118,7 @@ public function testSqlServerLimitsAndOffsets() return $query->select('created_at')->from('logins')->where('users.name', 'nameBinding')->whereColumn('user_id', 'users.id')->limit(1); }; $builder->select('*')->from('users')->where('email', 'emailBinding')->orderBy($subQuery)->skip(10)->take(10); - $this->assertSame('select * from [users] where [email] = ? order by (select top 1 [created_at] from [logins] where [users].[name] = ? and [user_id] = [users].[id]) asc OFFSET 10 ROWS FETCH NEXT 10 ROWS ONLY', $builder->toSql()); + $this->assertSame('select * from [users] where [email] = ? order by (select top 1 [created_at] from [logins] where [users].[name] = ? and [user_id] = [users].[id]) asc offset 10 rows fetch next 10 rows only', $builder->toSql()); $this->assertEquals(['emailBinding', 'nameBinding'], $builder->getBindings()); $builder = $this->getSqlServerBuilder();