Skip to content

Commit

Permalink
Don't use bound params for legacy SQLServer pagination
Browse files Browse the repository at this point in the history
By adding parameters to the query, we make future clones messy. Because
cloned queries need to retain their bound parameters (so they can
execute), cloning a pagination query in old SQLServer after it had
executed would retain the pagination parameters. These additional
parameters would cause PDO to consider the query invalid.

The changes in the PaginatorComponentTest are to cover the very odd
situation where SQLServer's identity column starts at 0 instead of 1 :(

Refs #6871
  • Loading branch information
markstory committed Jun 27, 2015
1 parent e5cf59f commit 8db3292
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 11 deletions.
5 changes: 3 additions & 2 deletions src/Database/Dialect/SqlserverDialectTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,11 @@ protected function _pagingSubquery($original, $limit, $offset)
->from(['_cake_paging_' => $query]);

if ($offset) {
$outer->where(["$field >" => $offset]);
$outer->where(["$field > $offset"]);
}
if ($limit) {
$outer->where(["$field <=" => (int)$offset + (int)$limit]);
$value = (int)$offset + (int)$limit;
$outer->where(["$field <= $value"]);
}

// Decorate the original query as that is what the
Expand Down
22 changes: 16 additions & 6 deletions tests/TestCase/Controller/Component/PaginatorComponentTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -811,22 +811,22 @@ public function testPaginateMaxLimit()
public function testPaginateCustomFind()
{
$this->loadFixtures('Posts');
$idExtractor = function ($result) {
$titleExtractor = function ($result) {
$ids = [];
foreach ($result as $record) {
$ids[] = $record->id;
$ids[] = $record->title;
}
return $ids;
};

$table = TableRegistry::get('PaginatorPosts');
$data = ['author_id' => 3, 'title' => 'Fourth Article', 'body' => 'Article Body, unpublished', 'published' => 'N'];
$data = ['author_id' => 3, 'title' => 'Fourth Post', 'body' => 'Article Body, unpublished', 'published' => 'N'];
$result = $table->save(new \Cake\ORM\Entity($data));
$this->assertNotEmpty($result);

$result = $this->Paginator->paginate($table);
$this->assertCount(4, $result, '4 rows should come back');
$this->assertEquals([1, 2, 3, 4], $idExtractor($result));
$this->assertEquals(['First Post', 'Second Post', 'Third Post', 'Fourth Post'] , $titleExtractor($result));

$result = $this->request->params['paging']['PaginatorPosts'];
$this->assertEquals(4, $result['current']);
Expand All @@ -835,16 +835,26 @@ public function testPaginateCustomFind()
$settings = ['finder' => 'published'];
$result = $this->Paginator->paginate($table, $settings);
$this->assertCount(3, $result, '3 rows should come back');
$this->assertEquals([1, 2, 3], $idExtractor($result));
$this->assertEquals(['First Post', 'Second Post', 'Third Post'], $titleExtractor($result));

$result = $this->request->params['paging']['PaginatorPosts'];
$this->assertEquals(3, $result['current']);
$this->assertEquals(3, $result['count']);

$settings = ['finder' => 'published', 'limit' => 2, 'page' => 2];
$result = $this->Paginator->paginate($table, $settings);
$this->assertCount(1, $result, '1 rows should come back');
$this->assertEquals(['Third Post'], $titleExtractor($result));

$result = $this->request->params['paging']['PaginatorPosts'];
$this->assertEquals(1, $result['current']);
$this->assertEquals(3, $result['count']);
$this->assertEquals(2, $result['pageCount']);

$settings = ['finder' => 'published', 'limit' => 2];
$result = $this->Paginator->paginate($table, $settings);
$this->assertCount(2, $result, '2 rows should come back');
$this->assertEquals([1, 2], $idExtractor($result));
$this->assertEquals(['First Post', 'Second Post'], $titleExtractor($result));

$result = $this->request->params['paging']['PaginatorPosts'];
$this->assertEquals(2, $result['current']);
Expand Down
6 changes: 3 additions & 3 deletions tests/TestCase/Database/Driver/SqlserverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public function testSelectLimitOldServer()
->offset(10);
$expected = 'SELECT * FROM (SELECT id, title, (ROW_NUMBER() OVER (ORDER BY (SELECT NULL))) AS [_cake_page_rownum_] ' .
'FROM articles) _cake_paging_ ' .
'WHERE _cake_paging_._cake_page_rownum_ > :c0';
'WHERE _cake_paging_._cake_page_rownum_ > 10';
$this->assertEquals($expected, $query->sql());

$query = new \Cake\Database\Query($connection);
Expand All @@ -200,7 +200,7 @@ public function testSelectLimitOldServer()
->offset(10);
$expected = 'SELECT * FROM (SELECT id, title, (ROW_NUMBER() OVER (ORDER BY id)) AS [_cake_page_rownum_] ' .
'FROM articles) _cake_paging_ ' .
'WHERE _cake_paging_._cake_page_rownum_ > :c0';
'WHERE _cake_paging_._cake_page_rownum_ > 10';
$this->assertEquals($expected, $query->sql());

$query = new \Cake\Database\Query($connection);
Expand All @@ -212,7 +212,7 @@ public function testSelectLimitOldServer()
->offset(50);
$expected = 'SELECT * FROM (SELECT id, title, (ROW_NUMBER() OVER (ORDER BY id)) AS [_cake_page_rownum_] ' .
'FROM articles WHERE title = :c0) _cake_paging_ ' .
'WHERE (_cake_paging_._cake_page_rownum_ > :c1 AND _cake_paging_._cake_page_rownum_ <= :c2)';
'WHERE (_cake_paging_._cake_page_rownum_ > 50 AND _cake_paging_._cake_page_rownum_ <= 60)';
$this->assertEquals($expected, $query->sql());
}

Expand Down

0 comments on commit 8db3292

Please sign in to comment.