Skip to content

Commit 0cb8b7a

Browse files
marcovtwoutmarcovtwout
authored
Merge pull request #4501 from wtommyw/master
Fix limit and offset not working correctly with MSSQL
2 parents 8530c30 + 4253a93 commit 0cb8b7a

File tree

4 files changed

+182
-28
lines changed

4 files changed

+182
-28
lines changed

‎CHANGELOG‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
Version 1.1.28 under development
55
--------------------------------
66

7-
- No changes yet
7+
- Bug #4491: Fixed limit and Offset not working correctly with MSSQL version 11 (2012) and newer (shnoulle, wtommyw)
88

99
Version 1.1.27 November 21, 2022
1010
--------------------------------

‎framework/db/schema/mssql/CMssqlCommandBuilder.php‎

Lines changed: 57 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,32 @@ public function applyJoin($sql,$join)
149149
}
150150

151151
/**
152+
* Apply limit and offset to sql query
153+
* @param string $sql SQL query string.
154+
* @param integer $limit maximum number of rows, -1 to ignore limit.
155+
* @param integer $offset row offset, -1 to ignore offset.
156+
* @return string SQL with limit and offset.
157+
* @see https://github.com/yiisoft/yii/issues/4491
158+
*/
159+
public function applyLimit($sql, $limit, $offset)
160+
{
161+
$limit = $limit!==null ? (int)$limit : -1;
162+
$offset = $offset!==null ? (int)$offset : -1;
163+
164+
if($limit <= 0 && $offset <=0) // no limit, no offset
165+
return $sql;
166+
if($limit > 0 && $offset <= 0) // only limit
167+
return preg_replace('/^([\s(])*SELECT( DISTINCT)?(?!\s*TOP\s*\()/i',"\\1SELECT\\2 TOP $limit", $sql);
168+
169+
if(version_compare($this->dbConnection->getServerVersion(), '11', '<'))
170+
return $this->oldRewriteLimitOffsetSql($sql, $limit, $offset);
171+
else
172+
return $this->newRewriteLimitOffsetSql($sql, $limit, $offset);
173+
}
174+
175+
/**
176+
* Rewrite sql to apply $limit and $offset for MSSQL database version 10 (2008) and lower.
177+
*
152178
* This is a port from Prado Framework.
153179
*
154180
* Overrides parent implementation. Alters the sql to apply $limit and $offset.
@@ -185,36 +211,18 @@ public function applyJoin($sql,$join)
185211
* </li>
186212
* </ul>
187213
*
188-
* @param string $sql SQL query string.
189-
* @param integer $limit maximum number of rows, -1 to ignore limit.
190-
* @param integer $offset row offset, -1 to ignore offset.
191-
* @return string SQL with limit and offset.
192-
*
193-
* @author Wei Zhuo <weizhuo[at]gmail[dot]com>
194-
*/
195-
public function applyLimit($sql, $limit, $offset)
196-
{
197-
$limit = $limit!==null ? (int)$limit : -1;
198-
$offset = $offset!==null ? (int)$offset : -1;
199-
if ($limit > 0 && $offset <= 0) //just limit
200-
$sql = preg_replace('/^([\s(])*SELECT( DISTINCT)?(?!\s*TOP\s*\()/i',"\\1SELECT\\2 TOP $limit", $sql);
201-
elseif($limit > 0 && $offset > 0)
202-
$sql = $this->rewriteLimitOffsetSql($sql, $limit,$offset);
203-
return $sql;
204-
}
205-
206-
/**
207-
* Rewrite sql to apply $limit > and $offset > 0 for MSSQL database.
208-
* See https://troels.arvin.dk/db/rdbms/#select-limit-offset
209214
* @param string $sql sql query
210-
* @param integer $limit $limit > 0
211-
* @param integer $offset $offset > 0
215+
* @param integer $limit $limit
216+
* @param integer $offset $offset
212217
* @return string modified sql query applied with limit and offset.
213-
*
214-
* @author Wei Zhuo <weizhuo[at]gmail[dot]com>
218+
* @see https://troels.arvin.dk/db/rdbms/#select-limit-offset
219+
* @see https://github.com/yiisoft/yii/issues/4491
215220
*/
216-
protected function rewriteLimitOffsetSql($sql, $limit, $offset)
221+
protected function oldRewriteLimitOffsetSql($sql, $limit, $offset)
217222
{
223+
if ($limit <= 0) // Offset without limit has never worked for MSSQL 10 and older, see https://github.com/yiisoft/yii/pull/4501
224+
return $sql;
225+
218226
$fetch = $limit+$offset;
219227
$sql = preg_replace('/^([\s(])*SELECT( DISTINCT)?(?!\s*TOP\s*\()/i',"\\1SELECT\\2 TOP $fetch", $sql);
220228
$ordering = $this->findOrdering($sql);
@@ -224,6 +232,29 @@ protected function rewriteLimitOffsetSql($sql, $limit, $offset)
224232
return $sql;
225233
}
226234

235+
/**
236+
* Rewrite SQL to apply $limit and $offset for MSSQL database version 11 (2012) and newer.
237+
* @see https://learn.microsoft.com/en-us/sql/t-sql/queries/select-order-by-clause-transact-sql?view=sql-server-ver15#using-offset-and-fetch-to-limit-the-rows-returned
238+
* @see https://github.com/yiisoft/yii/issues/4491
239+
* @param string $sql sql query
240+
* @param integer $limit $limit
241+
* @param integer $offset $offset
242+
* @return string modified sql query applied w th limit and offset.
243+
*/
244+
protected function newRewriteLimitOffsetSql($sql, $limit, $offset)
245+
{
246+
// ORDER BY is required when using OFFSET and FETCH
247+
if(count($this->findOrdering($sql)) === 0)
248+
$sql .= " ORDER BY (SELECT NULL)";
249+
250+
$sql .= sprintf(" OFFSET %d ROWS", $offset);
251+
252+
if($limit > 0)
253+
$sql .= sprintf(' FETCH NEXT %d ROWS ONLY', $limit);
254+
255+
return $sql;
256+
}
257+
227258
/**
228259
* Base on simplified syntax https://msdn2.microsoft.com/en-us/library/aa259187(SQL.80).aspx
229260
*

‎tests/framework/db/schema/CMssqlCommandBuilderTest.php‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
<?php
2+
3+
class CMssqlCommandBuilderTest extends CTestCase
4+
{
5+
/**
6+
* @var CDbConnection
7+
*/
8+
private $db;
9+
10+
/**
11+
* Mock a CDbConection, with a CMssqlSchema and CMssqlTableSchema
12+
*/
13+
public function setUp()
14+
{
15+
/*
16+
* Disable the constructor and mock `open` so that CDbConnection does not
17+
* try to make a connection
18+
*/
19+
$this->db = $this->getMockBuilder('CDbConnection')
20+
->disableOriginalConstructor()
21+
->setMethods(array('open', 'getServerVersion', 'getSchema'))
22+
->getMock();
23+
24+
$schema = $this->getMockBuilder('CMssqlSchema')
25+
->setConstructorArgs(array($this->db))
26+
->setMethods(array('getTable'))
27+
->getMock();
28+
29+
$tableMetaData = new CMssqlTableSchema();
30+
$tableMetaData->schemaName = 'posts';
31+
$tableMetaData->rawName = '[dbo].[posts]';
32+
$tableMetaData->primaryKey = 'id';
33+
34+
$schema->method('getTable')->willReturn($tableMetaData);
35+
36+
$this->db->method('getSchema')->willReturn($schema);
37+
}
38+
39+
/**
40+
* Verify generated SQL for MSSQL 10 (2008) and older hasn't changed
41+
*/
42+
public function testCommandBuilderOldMssql()
43+
{
44+
$this->db->method('getServerVersion')->willReturn('10');
45+
46+
$command = $this->createFindCommand(array(
47+
'limit'=>3,
48+
));
49+
$this->assertEquals('SELECT TOP 3 * FROM [dbo].[posts] [t]', $command->text);
50+
51+
$command = $this->createFindCommand(array(
52+
'select'=>'id, title',
53+
'order'=>'title',
54+
'limit'=>2,
55+
'offset'=>3
56+
));
57+
$this->assertEquals('SELECT * FROM (SELECT TOP 2 * FROM (SELECT TOP 5 id, title FROM [dbo].[posts] [t] ORDER BY title) as [__inner__] ORDER BY title DESC) as [__outer__] ORDER BY title ASC', $command->text);
58+
59+
$command = $this->createFindCommand(array(
60+
'limit'=>2,
61+
'offset'=>3
62+
));
63+
$this->assertEquals('SELECT * FROM (SELECT TOP 2 * FROM (SELECT TOP 5 * FROM [dbo].[posts] [t] ORDER BY id) as [__inner__] ORDER BY id DESC) as [__outer__] ORDER BY id ASC', $command->text);
64+
65+
$command = $this->createFindCommand(array(
66+
'select'=>'title',
67+
));
68+
$this->assertEquals('SELECT title FROM [dbo].[posts] [t]', $command->text);
69+
}
70+
71+
public function testCommandBuilderNewMssql()
72+
{
73+
$this->db->method('getServerVersion')->willReturn('11');
74+
75+
$command = $this->createFindCommand(array(
76+
'limit'=>3,
77+
));
78+
$this->assertEquals('SELECT TOP 3 * FROM [dbo].[posts] [t]', $command->text);
79+
80+
$command = $this->createFindCommand(array(
81+
'select'=>'id, title',
82+
'order'=>'title',
83+
'limit'=>2,
84+
'offset'=>3,
85+
));
86+
$this->assertEquals('SELECT id, title FROM [dbo].[posts] [t] ORDER BY title OFFSET 3 ROWS FETCH NEXT 2 ROWS ONLY', $command->text);
87+
88+
$command = $this->createFindCommand(array(
89+
'limit'=>2,
90+
'offset'=>3,
91+
));
92+
$this->assertEquals('SELECT * FROM [dbo].[posts] [t] ORDER BY id OFFSET 3 ROWS FETCH NEXT 2 ROWS ONLY', $command->text);
93+
94+
95+
$command = $this->createFindCommand(array(
96+
'select'=>'title',
97+
'offset'=>3,
98+
));
99+
$this->assertEquals('SELECT title FROM [dbo].[posts] [t] ORDER BY id OFFSET 3 ROWS', $command->text);
100+
101+
$command = $this->createFindCommand(array(
102+
'offset'=>3,
103+
));
104+
$this->assertEquals('SELECT * FROM [dbo].[posts] [t] ORDER BY id OFFSET 3 ROWS', $command->text);
105+
106+
$command = $this->createFindCommand(array(
107+
'select'=>'title',
108+
));
109+
$this->assertEquals('SELECT title FROM [dbo].[posts] [t]', $command->text);
110+
}
111+
112+
/**
113+
* @param $criteria array
114+
* @return CDbCommand
115+
* @throws CDbException
116+
*/
117+
private function createFindCommand($criteria)
118+
{
119+
$schema = $this->db->getSchema();
120+
$table = $schema->getTable('posts');
121+
return $schema->commandBuilder->createFindCommand($table, new CDbCriteria($criteria));
122+
}
123+
}

‎tests/framework/db/schema/CMssqlTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ public function testCommandBuilder()
224224
'order'=>'title',
225225
'limit'=>2,
226226
'offset'=>3)));
227-
$this->assertEquals('SELECT * FROM (SELECT TOP 2 * FROM (SELECT TOP 5 id, title FROM [dbo].[posts] [t] ORDER BY title) as [__inner__] ORDER BY title DESC) as [__outer__] ORDER BY title ASC',$c->text);
227+
228228
$rows=$c->query()->readAll();
229229
$this->assertEquals(2,count($rows));
230230
$this->assertEquals('post 4',$rows[0]['title']);

0 commit comments

Comments
 (0)