Browse Source

Don't use bound params for legacy SQLServer pagination

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
Mark Story 10 years ago
parent
commit
8db3292b22

+ 3 - 2
src/Database/Dialect/SqlserverDialectTrait.php

@@ -113,10 +113,11 @@ trait SqlserverDialectTrait
             ->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

+ 16 - 6
tests/TestCase/Controller/Component/PaginatorComponentTest.php

@@ -811,22 +811,22 @@ class PaginatorComponentTest extends TestCase
     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']);
@@ -835,16 +835,26 @@ class PaginatorComponentTest extends TestCase
         $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']);

+ 3 - 3
tests/TestCase/Database/Driver/SqlserverTest.php

@@ -190,7 +190,7 @@ class SqlserverTest extends TestCase
             ->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);
@@ -200,7 +200,7 @@ class SqlserverTest extends TestCase
             ->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);
@@ -212,7 +212,7 @@ class SqlserverTest extends TestCase
             ->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());
     }