From 608dfa2f571f2b93ccdb2fcd9ab66c4160f02a9e Mon Sep 17 00:00:00 2001 From: Bill Schaller Date: Mon, 30 Mar 2015 14:56:28 -0400 Subject: [PATCH] Add more detection in LimitSubqueryWalker for conditions that must be handled by LimitSubqueryOutputWalker --- .../Tools/Pagination/LimitSubqueryWalker.php | 40 +++++++++++++++++++ .../Tests/ORM/Functional/PaginationTest.php | 35 +++++----------- 2 files changed, 50 insertions(+), 25 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryWalker.php b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryWalker.php index c88191153..e65cfcaeb 100644 --- a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryWalker.php +++ b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryWalker.php @@ -19,6 +19,9 @@ namespace Doctrine\ORM\Tools\Pagination; use Doctrine\DBAL\Types\Type; +use Doctrine\ORM\Mapping\ClassMetadataInfo; +use Doctrine\ORM\ORMException; +use Doctrine\ORM\Query; use Doctrine\ORM\Query\TreeWalkerAdapter; use Doctrine\ORM\Query\AST\Functions\IdentityFunction; use Doctrine\ORM\Query\AST\PathExpression; @@ -68,6 +71,8 @@ class LimitSubqueryWalker extends TreeWalkerAdapter $rootClass = $queryComponents[$rootAlias]['metadata']; $selectExpressions = array(); + $this->validate($AST); + foreach ($queryComponents as $dqlAlias => $qComp) { // Preserve mixed data in query for ordering. if (isset($qComp['resultVariable'])) { @@ -112,6 +117,41 @@ class LimitSubqueryWalker extends TreeWalkerAdapter $AST->selectClause->isDistinct = true; } + + /** + * Validate the AST to ensure that this walker is able to properly manipulate it. + * + * @param SelectStatement $AST + */ + private function validate(SelectStatement $AST) + { + // Prevent LimitSubqueryWalker from being used with queries that include + // a limit, a fetched to-many join, and an order by condition that + // references a column from the fetch joined table. + $queryComponents = $this->getQueryComponents(); + $query = $this->_getQuery(); + $from = $AST->fromClause->identificationVariableDeclarations; + $fromRoot = reset($from); + + if ($query instanceof Query + && $query->getMaxResults() + && $AST->orderByClause + && count($fromRoot->joins)) { + // Check each orderby item. + // TODO: check complex orderby items too... + foreach ($AST->orderByClause->orderByItems as $orderByItem) { + $expression = $orderByItem->expression; + if ($orderByItem->expression instanceof PathExpression + && isset($queryComponents[$expression->identificationVariable])) { + $queryComponent = $queryComponents[$expression->identificationVariable]; + if (isset($queryComponent['parent']) + && $queryComponent['relation']['type'] & ClassMetadataInfo::TO_MANY) { + throw new \RuntimeException("Cannot select distinct identifiers from query with LIMIT and ORDER BY on a column from a fetch joined to-many association. Use output walkers."); + } + } + } + } + } /** * Retrieve either an IdentityFunction (IDENTITY(u.assoc)) or a state field (u.name). diff --git a/tests/Doctrine/Tests/ORM/Functional/PaginationTest.php b/tests/Doctrine/Tests/ORM/Functional/PaginationTest.php index b7db9b1f6..e0e5cc1d7 100644 --- a/tests/Doctrine/Tests/ORM/Functional/PaginationTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/PaginationTest.php @@ -120,7 +120,6 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase private function iterateWithOrderAsc($useOutputWalkers, $fetchJoinCollection, $baseDql, $checkField) { - // Ascending $dql = "$baseDql ASC"; $query = $this->_em->createQuery($dql); @@ -135,7 +134,6 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase private function iterateWithOrderAscWithLimit($useOutputWalkers, $fetchJoinCollection, $baseDql, $checkField) { - // Ascending $dql = "$baseDql ASC"; $query = $this->_em->createQuery($dql); @@ -496,18 +494,12 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testIterateWithFetchJoinOneToManyWithOrderByColumnFromBothWithLimitWithoutOutputWalker() { - $this->setExpectedException("RuntimeException", "Cannot select distinct identifiers from query with LIMIT and ORDER BY on a joined entity. Use output walkers."); - - $dql = 'SELECT c, d FROM Doctrine\Tests\Models\Pagination\Company c JOIN c.departments d ORDER BY c.name ASC'; - // Ascending - $query = $this->_em->createQuery($dql); - - // With limit - $query->setMaxResults(3); - $paginator = new Paginator($query, true); - $paginator->setUseOutputWalkers(false); - $iter = $paginator->getIterator(); - iterator_to_array($iter); + $this->setExpectedException("RuntimeException", "Cannot select distinct identifiers from query with LIMIT and ORDER BY on a column from a fetch joined to-many association. Use output walkers."); + $dql = 'SELECT c, d FROM Doctrine\Tests\Models\Pagination\Company c JOIN c.departments d ORDER BY c.name'; + $dqlAsc = $dql . " ASC, d.name"; + $dqlDesc = $dql . " DESC, d.name"; + $this->iterateWithOrderAscWithLimit(false, true, $dqlAsc, "name"); + $this->iterateWithOrderDescWithLimit(false, true, $dqlDesc, "name"); } /** @@ -559,18 +551,11 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testIterateWithFetchJoinOneToManyWithOrderByColumnFromJoinedWithLimitWithoutOutputWalker() { - $this->setExpectedException("RuntimeException", "Cannot select distinct identifiers from query with LIMIT and ORDER BY on a joined entity. Use output walkers."); - $dql = 'SELECT c, d FROM Doctrine\Tests\Models\Pagination\Company c JOIN c.departments d ORDER BY d.name ASC'; + $this->setExpectedException("RuntimeException", "Cannot select distinct identifiers from query with LIMIT and ORDER BY on a column from a fetch joined to-many association. Use output walkers."); + $dql = 'SELECT c, d FROM Doctrine\Tests\Models\Pagination\Company c JOIN c.departments d ORDER BY d.name'; - // Ascending - $query = $this->_em->createQuery($dql); - - // With limit - $query->setMaxResults(3); - $paginator = new Paginator($query, true); - $paginator->setUseOutputWalkers(false); - $iter = $paginator->getIterator(); - iterator_to_array($iter); + $this->iterateWithOrderAscWithLimit(false, true, $dql, "name"); + $this->iterateWithOrderDescWithLimit(false, true, $dql, "name"); } public function testCountWithCountSubqueryInWhereClauseWithOutputWalker()