From 81d02278ad3d8d08852b9237a618e5f370d58303 Mon Sep 17 00:00:00 2001 From: beberlei Date: Sun, 21 Feb 2010 00:06:34 +0000 Subject: [PATCH] [2.0] DDC-338 - Changed DQL Parser to comply with ordered collections when they are fetch joined (OMG, thanks to roman and guilherme for the detailed discussion on how to implement this) --- lib/Doctrine/ORM/Query/SqlWalker.php | 41 ++++++++++++++++++- .../ORM/Functional/OrderedCollectionTest.php | 22 +++++++++- ...edJoinedTableInheritanceCollectionTest.php | 18 ++++++-- .../ORM/Query/SelectSqlGenerationTest.php | 12 ++++++ 4 files changed, 87 insertions(+), 6 deletions(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 19120afd1..8e69389d1 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -284,6 +284,32 @@ class SqlWalker implements TreeWalker return $sql; } + private function _generateOrderedCollectionOrderByItems() + { + $sql = ''; + foreach ($this->_selectedClasses AS $dqlAlias => $class) { + $qComp = $this->_queryComponents[$dqlAlias]; + if (isset($qComp['relation']) && ($qComp['relation']->isManyToMany() || $qComp['relation']->isOneToMany()) + && $qComp['relation']->orderBy != null) { + + foreach ($qComp['relation']->orderBy AS $fieldName => $orientation) { + if ($qComp['metadata']->isInheritanceTypeJoined()) { + $tableName = $this->_em->getUnitOfWork()->getEntityPersister($class->name)->getOwningTable($fieldName); + } else { + $tableName = $qComp['metadata']->primaryTable['name']; + } + + if ($sql != '') { + $sql .= ', '; + } + $sql .= $this->getSqlTableAlias($tableName, $dqlAlias) . "." . + $qComp['metadata']->getQuotedColumnName($fieldName, $this->_platform) . " ".$orientation; + } + } + } + return $sql; + } + /** * Generates a discriminator column SQL condition for the class with the given DQL alias. * @@ -334,7 +360,13 @@ class SqlWalker implements TreeWalker $sql .= $AST->groupByClause ? $this->walkGroupByClause($AST->groupByClause) : ''; $sql .= $AST->havingClause ? $this->walkHavingClause($AST->havingClause) : ''; - $sql .= $AST->orderByClause ? $this->walkOrderByClause($AST->orderByClause) : ''; + + if (($orderByClause = $AST->orderByClause) !== null) { + $sql .= $AST->orderByClause ? $this->walkOrderByClause($AST->orderByClause) : ''; + } else if (($orderBySql = $this->_generateOrderedCollectionOrderByItems()) !== '') { + $sql .= ' ORDER BY '.$orderBySql; + } + $sql = $this->_platform->modifyLimitQuery( $sql, $this->_query->getMaxResults(), $this->_query->getFirstResult() @@ -589,10 +621,15 @@ class SqlWalker implements TreeWalker */ public function walkOrderByClause($orderByClause) { + $colSql = $this->_generateOrderedCollectionOrderByItems(); + if ($colSql != '') { + $colSql = ", ".$colSql; + } + // OrderByClause ::= "ORDER" "BY" OrderByItem {"," OrderByItem}* return ' ORDER BY ' . implode( ', ', array_map(array($this, 'walkOrderByItem'), $orderByClause->orderByItems) - ); + ) . $colSql; } /** diff --git a/tests/Doctrine/Tests/ORM/Functional/OrderedCollectionTest.php b/tests/Doctrine/Tests/ORM/Functional/OrderedCollectionTest.php index f44fe4798..f22c964b8 100644 --- a/tests/Doctrine/Tests/ORM/Functional/OrderedCollectionTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/OrderedCollectionTest.php @@ -29,7 +29,7 @@ class OrderedCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->_em->flush(); } - public function testLazyManyToManyCollection_IsRetrievedWithOrderByClause() + public function createPersistedRouteWithLegs() { $route = new RoutingRoute(); @@ -53,6 +53,13 @@ class OrderedCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $routeId = $route->id; $this->_em->clear(); + return $routeId; + } + + public function testLazyManyToManyCollection_IsRetrievedWithOrderByClause() + { + $routeId = $this->createPersistedRouteWithLegs(); + $route = $this->_em->find('Doctrine\Tests\Models\Routing\RoutingRoute', $routeId); $this->assertEquals(2, count($route->legs)); @@ -90,4 +97,17 @@ class OrderedCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertEquals('Benjamin', $route->bookings[0]->getPassengerName()); $this->assertEquals('Guilherme', $route->bookings[1]->getPassengerName()); } + + public function testOrderedResultFromDqlQuery() + { + $routeId = $this->createPersistedRouteWithLegs(); + + $route = $this->_em->createQuery("SELECT r, l FROM Doctrine\Tests\Models\Routing\RoutingRoute r JOIN r.legs l WHERE r.id = ?1") + ->setParameter(1, $routeId) + ->getSingleResult(); + + $this->assertEquals(2, count($route->legs)); + $this->assertEquals("Berlin", $route->legs[0]->fromLocation->getName()); + $this->assertEquals("Bonn", $route->legs[1]->fromLocation->getName()); + } } \ No newline at end of file diff --git a/tests/Doctrine/Tests/ORM/Functional/OrderedJoinedTableInheritanceCollectionTest.php b/tests/Doctrine/Tests/ORM/Functional/OrderedJoinedTableInheritanceCollectionTest.php index 4ede6056a..7a243903c 100644 --- a/tests/Doctrine/Tests/ORM/Functional/OrderedJoinedTableInheritanceCollectionTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/OrderedJoinedTableInheritanceCollectionTest.php @@ -24,10 +24,7 @@ class OrderedJoinedTableInheritanceCollectionTest extends \Doctrine\Tests\OrmFun } catch (\Exception $e) { // Swallow all exceptions. We do not test the schema tool here. } - } - public function testOrderdOneToManyCollection() - { $dog = new OJTIC_Dog(); $dog->name = "Poofy"; @@ -47,11 +44,26 @@ class OrderedJoinedTableInheritanceCollectionTest extends \Doctrine\Tests\OrmFun $this->_em->persist($dog2); $this->_em->flush(); $this->_em->clear(); + } + public function testOrderdOneToManyCollection() + { $poofy = $this->_em->createQuery("SELECT p FROM Doctrine\Tests\ORM\Functional\OJTIC_Pet p WHERE p.name = 'Poofy'")->getSingleResult(); $this->assertEquals('Aari', $poofy->children[0]->getName()); $this->assertEquals('Zampa', $poofy->children[1]->getName()); + + $this->_em->clear(); + + $result = $this->_em->createQuery( + "SELECT p, c FROM Doctrine\Tests\ORM\Functional\OJTIC_Pet p JOIN p.children c WHERE p.name = 'Poofy'") + ->getResult(); + + $this->assertEquals(1, count($result)); + $poofy = $result[0]; + + $this->assertEquals('Aari', $poofy->children[0]->getName()); + $this->assertEquals('Zampa', $poofy->children[1]->getName()); } } diff --git a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php index 74c92ff52..b74463a3e 100644 --- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php @@ -564,4 +564,16 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase "SELECT c0_.name AS name0 FROM cms_users c0_ WHERE UPPER(c0_.name) || '_moo' LIKE ?" ); } + + /** + * @group DDC-338 + */ + public function testOrderedCollectionFetchJoined() + { + $this->assertSqlGeneration( + "SELECT r, l FROM Doctrine\Tests\Models\Routing\RoutingRoute r JOIN r.legs l", + "SELECT r0_.id AS id0, r1_.id AS id1, r1_.departureDate AS departureDate2, r1_.arrivalDate AS arrivalDate3 FROM RoutingRoute r0_ INNER JOIN RoutingRouteLegs r2_ ON r0_.id = r2_.route_id INNER JOIN RoutingLeg r1_ ON r1_.id = r2_.leg_id ". + "ORDER BY r1_.departureDate ASC" + ); + } }