Merge pull request #2611 from soyuka/micro-optimization-doctrine-distinct

Do not use DISTINCT where there are no joins

Author: teohhanhui

src/Bridge/Doctrine/Orm/Extension/PaginationExtension.php

@@ -23,6 +23,7 @@
 use ApiPlatform\Core\Metadata\Resource\ResourceMetadata;
 use Doctrine\Common\Persistence\ManagerRegistry;
 use Doctrine\ORM\QueryBuilder;
+use Doctrine\ORM\Tools\Pagination\CountWalker;
 use Doctrine\ORM\Tools\Pagination\Paginator as DoctrineOrmPaginator;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\RequestStack;
@@ -143,7 +144,14 @@ public function supportsResult(string $resourceClass, string $operationName = nu
      */
     public function getResult(QueryBuilder $queryBuilder, string $resourceClass = null, string $operationName = null, array $context = [])
     {
-        $doctrineOrmPaginator = new DoctrineOrmPaginator($queryBuilder, $this->useFetchJoinCollection($queryBuilder, $resourceClass, $operationName));
+        $query = $queryBuilder->getQuery();
+
+        // Only one alias, without joins, disable the DISTINCT on the COUNT
+        if (1 === \count($queryBuilder->getAllAliases())) {
+            $query->setHint(CountWalker::HINT_DISTINCT, false);
+        }
+
+        $doctrineOrmPaginator = new DoctrineOrmPaginator($query, $this->useFetchJoinCollection($queryBuilder, $resourceClass, $operationName));
         $doctrineOrmPaginator->setUseOutputWalkers($this->useOutputWalkers($queryBuilder));
 
         if (null === $this->requestStack) {

tests/Bridge/Doctrine/Orm/Extension/PaginationExtensionTest.php

@@ -27,6 +27,7 @@
 use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\Query;
 use Doctrine\ORM\QueryBuilder;
+use Doctrine\ORM\Tools\Pagination\CountWalker;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Symfony\Component\HttpFoundation\Request;
@@ -887,6 +888,35 @@ public function testGetResult()
         $this->assertInstanceOf(PaginatorInterface::class, $result);
     }
 
+    public function testGetResultWithoutDistinct()
+    {
+        $configuration = new Configuration();
+
+        $entityManagerProphecy = $this->prophesize(EntityManagerInterface::class);
+        $entityManagerProphecy->getConfiguration()->willReturn($configuration)->shouldBeCalled();
+
+        $query = new Query($entityManagerProphecy->reveal());
+        $query->setFirstResult(0);
+        $query->setMaxResults(42);
+
+        $queryBuilderProphecy = $this->prophesize(QueryBuilder::class);
+        $queryBuilderProphecy->getRootEntities()->willReturn([])->shouldBeCalled();
+        $queryBuilderProphecy->getAllAliases()->willReturn(['o'])->shouldBeCalled();
+        $queryBuilderProphecy->getQuery()->willReturn($query)->shouldBeCalled();
+        $queryBuilderProphecy->getDQLPart(Argument::that(function ($arg) {
+            return \in_array($arg, ['having', 'orderBy', 'join'], true);
+        }))->willReturn('')->shouldBeCalled();
+        $queryBuilderProphecy->getMaxResults()->willReturn(42)->shouldBeCalled();
+        $queryBuilder = $queryBuilderProphecy->reveal();
+
+        $result = $this->getPaginationExtensionResult(false, false, true, $queryBuilder);
+
+        $this->assertInstanceOf(PartialPaginatorInterface::class, $result);
+        $this->assertInstanceOf(PaginatorInterface::class, $result);
+
+        $this->assertFalse($query->getHint(CountWalker::HINT_DISTINCT));
+    }
+
     /**
      * @group legacy
      * @expectedDeprecation Passing an instance of "Symfony\Component\HttpFoundation\RequestStack" as second argument of "ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\PaginationExtension" is deprecated since API Platform 2.4 and will not be possible anymore in API Platform 3. Pass an instance of "ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface" instead.
@@ -963,7 +993,7 @@ public function testLegacySimpleGetResult()
         $this->assertInstanceOf(PaginatorInterface::class, $result);
     }
 
-    private function getPaginationExtensionResult(bool $partial = false, bool $legacy = false, bool $fetchJoinCollection = true)
+    private function getPaginationExtensionResult(bool $partial = false, bool $legacy = false, bool $fetchJoinCollection = true, QueryBuilder $queryBuilder = null)
     {
         $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class);
         $resourceMetadataFactory = $resourceMetadataFactoryProphecy->reveal();
@@ -979,25 +1009,29 @@ private function getPaginationExtensionResult(bool $partial = false, bool $legac
         $entityManagerProphecy = $this->prophesize(EntityManagerInterface::class);
         $entityManagerProphecy->getConfiguration()->willReturn($configuration);
 
-        $query = new Query($entityManagerProphecy->reveal());
-        $query->setFirstResult(0);
-        $query->setMaxResults(42);
-
-        $queryBuilderProphecy = $this->prophesize(QueryBuilder::class);
-        $queryBuilderProphecy->getRootEntities()->willReturn([]);
-        $queryBuilderProphecy->getQuery()->willReturn($query);
-        $queryBuilderProphecy->getDQLPart(Argument::that(function ($arg) {
-            return \in_array($arg, ['having', 'orderBy', 'join'], true);
-        }))->willReturn('');
-        $queryBuilderProphecy->getMaxResults()->willReturn(42);
+        if (null === $queryBuilder) {
+            $query = new Query($entityManagerProphecy->reveal());
+            $query->setFirstResult(0);
+            $query->setMaxResults(42);
+
+            $queryBuilderProphecy = $this->prophesize(QueryBuilder::class);
+            $queryBuilderProphecy->getRootEntities()->willReturn([])->shouldBeCalled();
+            $queryBuilderProphecy->getAllAliases()->willReturn([])->shouldBeCalled();
+            $queryBuilderProphecy->getQuery()->willReturn($query)->shouldBeCalled();
+            $queryBuilderProphecy->getDQLPart(Argument::that(function ($arg) {
+                return \in_array($arg, ['having', 'orderBy', 'join'], true);
+            }))->willReturn('')->shouldBeCalled();
+            $queryBuilderProphecy->getMaxResults()->willReturn(42)->shouldBeCalled();
+            $queryBuilder = $queryBuilderProphecy->reveal();
+        }
 
         $paginationExtension = new PaginationExtension(
             $this->prophesize(ManagerRegistry::class)->reveal(),
             $resourceMetadataFactory,
             $pagination
         );
 
-        $args = [$queryBuilderProphecy->reveal(), null, null, ['filters' => ['partial' => $partial]]];
+        $args = [$queryBuilder, null, null, ['filters' => ['partial' => $partial]]];
 
         if (!$legacy) {
             $args[1] = 'Foo';
@@ -1029,6 +1063,7 @@ private function getLegacyPaginationExtensionResult(bool $partial = false, bool
 
         $queryBuilderProphecy = $this->prophesize(QueryBuilder::class);
         $queryBuilderProphecy->getRootEntities()->willReturn([])->shouldBeCalled();
+        $queryBuilderProphecy->getAllAliases()->willReturn([])->shouldBeCalled();
         $queryBuilderProphecy->getQuery()->willReturn($query)->shouldBeCalled();
         $queryBuilderProphecy->getDQLPart(Argument::that(function ($arg) {
             return \in_array($arg, ['having', 'orderBy', 'join'], true);