[2.0] DDC-338 - Changed DQL Parser to comply with ordered collections when...

[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

@@ -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;
     }
 
     /**

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

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());
     }
 }
 

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"
+        );
+    }
 }