Merge pull request #348 from kicken/remove-order-by

garak · web-flow · commit eabf39263fff · 2025-03-20T08:35:37.000+01:00
Remove order by clause from the item count query.
diff --git a/src/Knp/Component/Pager/Event/Subscriber/Paginate/Doctrine/DBALQueryBuilderSubscriber.php b/src/Knp/Component/Pager/Event/Subscriber/Paginate/Doctrine/DBALQueryBuilderSubscriber.php
@@ -27,7 +27,7 @@ public function items(ItemsEvent $event): void
                 ->connection
                 ->createQueryBuilder()
                 ->select('COUNT(*)')
-                ->from('(' . $target->getSQL() . ')', 'tmp')
+                ->from('(' . (clone $target)->resetOrderBy()->getSQL() . ')', 'tmp')
                 ->setParameters($target->getParameters(), $target->getParameterTypes())
             ;
 
diff --git a/tests/Test/Pager/Subscriber/Paginate/Doctrine/DBALQueryBuilderTest.php b/tests/Test/Pager/Subscriber/Paginate/Doctrine/DBALQueryBuilderTest.php
@@ -3,8 +3,14 @@
 namespace Test\Pager\Subscriber\Paginate\Doctrine;
 
 use Doctrine\DBAL\Query\QueryBuilder;
+use Doctrine\ORM\EntityManager;
+use Knp\Component\Pager\ArgumentAccess\ArgumentAccessInterface;
+use Knp\Component\Pager\Event\ItemsEvent;
+use Knp\Component\Pager\Event\Subscriber\Paginate\Doctrine\DBALQueryBuilderSubscriber;
 use PHPUnit\Framework\Attributes\Test;
 use Test\Fixture\Entity\Article;
+use Test\Fixture\Entity\Shop\Product;
+use Test\Fixture\Entity\Shop\Tag;
 use Test\Tool\BaseTestCaseORM;
 
 final class DBALQueryBuilderTest extends BaseTestCaseORM
@@ -31,9 +37,61 @@ public function shouldPaginateSimpleDoctrineQuery(): void
         $this->assertEquals('winter', $items[1]['title']);
     }
 
+    #[Test]
+    public function shouldStripOrderByForCount(): void
+    {
+        $this->populate();
+        $qb = new QueryBuilder($this->em->getConnection());
+        $qb
+            ->select('*')
+            ->from('Article', 'a')
+            ->orderBy('a.title');
+
+        $event = new ItemsEvent(0, 2, $this->mockArgumentAccess());
+        $event->target = $qb;
+
+        $this->queryAnalyzer->enable();
+        $service = new DBALQueryBuilderSubscriber($this->em->getConnection());
+        $service->items($event);
+        $this->queryAnalyzer->disable();
+        $countQuery = null;
+        foreach ($this->queryAnalyzer->getExecutedQueries() as $query) {
+            $query = strtolower($query);
+            if (str_contains($query, 'count(*)')) {
+                $countQuery = $query;
+                break;
+            }
+        }
+
+        $this->assertNotNull($countQuery);
+        $this->assertFalse(str_contains($countQuery, 'order by'));
+        //Ensure original query builder is not affected by the order by removal.
+        $this->assertTrue(str_contains(strtolower($qb->getSQL()), 'order by'));
+    }
+
+    #[Test]
+    public function shouldWorkWithGroupBy(): void
+    {
+        $this->populateProducts();
+        $qb = new QueryBuilder($this->em->getConnection());
+        $qb
+            ->select('p.title, count(pt.tag_id) as totalTags')
+            ->from('Product', 'p')
+            ->join('p', 'product_tag', 'pt', 'pt.product_id = p.id')
+            ->groupBy('p.id');
+
+        $event = new ItemsEvent(0, 2, $this->mockArgumentAccess());
+        $event->target = $qb;
+
+        $service = new DBALQueryBuilderSubscriber($this->em->getConnection());
+        $service->items($event);
+        $this->assertCount(2, $event->items);
+        $this->assertEquals(4, $event->count);
+    }
+
     protected function getUsedEntityFixtures(): array
     {
-        return [Article::class];
+        return [Article::class, Product::class, Tag::class];
     }
 
     private function populate(): void
@@ -57,4 +115,61 @@ private function populate(): void
         $em->persist($spring);
         $em->flush();
     }
+
+    private function populateProducts(): void
+    {
+        $em = $this->getMockSqliteEntityManager();
+        $product = new Product();
+        $product->setTitle('Item 1');
+        $product->addTag($this->createTag($em, 'A'));
+        $product->addTag($this->createTag($em, 'B'));
+        $product->addTag($this->createTag($em, 'C'));
+        $em->persist($product);
+
+        $product = new Product();
+        $product->setTitle('Item 2');
+        $product->addTag($this->createTag($em, 'A'));
+        $em->persist($product);
+
+        $product = new Product();
+        $product->setTitle('Item 3');
+        $product->addTag($this->createTag($em, 'A'));
+        $product->addTag($this->createTag($em, 'B'));
+        $em->persist($product);
+
+        $product = new Product();
+        $product->setTitle('Item 4');
+        $product->addTag($this->createTag($em, 'A'));
+        $product->addTag($this->createTag($em, 'B'));
+        $product->addTag($this->createTag($em, 'C'));
+        $em->persist($product);
+        $em->flush();
+        $this->queryAnalyzer->disable();
+    }
+
+    private function createTag(EntityManager $em, string $name): Tag
+    {
+        $tag = new Tag();
+        $tag->setName($name);
+        $em->persist($tag);
+
+        return $tag;
+    }
+
+    private function mockArgumentAccess(): ArgumentAccessInterface
+    {
+        return new class implements ArgumentAccessInterface {
+            public function has(string $name): bool
+            {
+                return false;
+            }
+
+            public function get(string $name): string|int|float|bool|null
+            {
+                return null;
+            }
+
+            public function set(string $name, float|bool|int|string|null $value): void {}
+        };
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,7 @@ public function items(ItemsEvent $event): void`
`27`	`27`	`->connection`
`28`	`28`	`->createQueryBuilder()`
`29`	`29`	`->select('COUNT(*)')`
`30`		`- ->from('(' . $target->getSQL() . ')', 'tmp')`
	`30`	`+ ->from('(' . (clone $target)->resetOrderBy()->getSQL() . ')', 'tmp')`
`31`	`31`	`->setParameters($target->getParameters(), $target->getParameterTypes())`
`32`	`32`	`;`
`33`	`33`