From df0875c596afc5625d3b39eab4fae0c8943ff8c7 Mon Sep 17 00:00:00 2001 From: Bill Schaller Date: Tue, 17 Mar 2015 17:32:28 -0400 Subject: [PATCH] Fix Paginator OrderBy clauses when ordering by columns from non-fetched joined tables --- .../Pagination/LimitSubqueryOutputWalker.php | 63 +++++++++++++++++-- .../LimitSubqueryOutputWalkerTest.php | 32 +++++++++- .../Tools/Pagination/PaginationTestCase.php | 4 ++ 3 files changed, 93 insertions(+), 6 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php index b1d68e350..a61557b92 100644 --- a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php +++ b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php @@ -13,14 +13,11 @@ namespace Doctrine\ORM\Tools\Pagination; -use Doctrine\DBAL\Platforms\MySqlPlatform; -use Doctrine\DBAL\Platforms\OraclePlatform; -use Doctrine\DBAL\Platforms\SQLServerPlatform; use Doctrine\ORM\Query\AST\OrderByClause; -use Doctrine\ORM\Query\AST\PathExpression; +use Doctrine\ORM\Query\AST\PartialObjectExpression; +use Doctrine\ORM\Query\AST\SelectExpression; use Doctrine\ORM\Query\SqlWalker; use Doctrine\ORM\Query\AST\SelectStatement; -use Doctrine\DBAL\Platforms\PostgreSqlPlatform; /** * Wraps the query in order to select root entity IDs for pagination. @@ -111,6 +108,11 @@ class LimitSubqueryOutputWalker extends SqlWalker */ public function walkSelectStatement(SelectStatement $AST) { + // In the case of ordering a query by columns from joined tables, we + // must add those columns to the select clause of the query BEFORE + // the SQL is generated. + $this->addMissingItemsFromOrderByToSelect($AST); + // Remove order by clause from the inner query // It will be re-appended in the outer select generated by this method $orderByClause = $AST->orderByClause; @@ -128,6 +130,9 @@ class LimitSubqueryOutputWalker extends SqlWalker $innerSql = parent::walkSelectStatement($AST); + // Restore orderByClause + $AST->orderByClause = $orderByClause; + // Restore hiddens foreach ($AST->selectClause->selectExpressions as $idx => $expr) { $expr->hiddenAliasResultVariable = $hiddens[$idx]; @@ -205,6 +210,54 @@ class LimitSubqueryOutputWalker extends SqlWalker return $sql; } + /** + * Finds all PathExpressions in an AST's OrderByClause, and ensures that + * the referenced fields are present in the SelectClause of the passed AST. + * + * @param SelectStatement $AST + */ + private function addMissingItemsFromOrderByToSelect(SelectStatement $AST) + { + // This block dumps the order by clause node using Node::dump(). + // It then finds all PathExpressions within and captures the + // identificationVariable and field name of each. + $orderByDump = (string)$AST->orderByClause; + $selects = []; + if (preg_match_all('/PathExpression\([^\)]+"identificationVariable": \'([^\']*)\'[^\)]+"field": \'([^\']*)\'[^\)]+\)/i', $orderByDump, $matches, PREG_SET_ORDER)) { + foreach($matches as $match) { + $idVar = $match[1]; + $field = $match[2]; + if (!isset($selects[$idVar])) { + $selects[$idVar] = []; + } + $selects[$idVar][$field] = true; + } + } + + // Loop the select clause of the AST and exclude items from $select + // that are already being selected. + foreach ($AST->selectClause->selectExpressions as $selectExpression) { + if ($selectExpression instanceof SelectExpression) { + $idVar = $selectExpression->expression; + if (!is_string($idVar)) { + continue; + } + $field = $selectExpression->fieldIdentificationVariable; + if ($field === null) { + // No need to add this select, as we're already fetching the whole object. + unset($selects[$idVar]); + } else { + unset($selects[$idVar][$field]); + } + } + } + + // Add select items which were not excluded to the AST's select clause. + foreach ($selects as $idVar => $fields) { + $AST->selectClause->selectExpressions[] = new SelectExpression(new PartialObjectExpression($idVar, array_keys($fields)), null, true); + } + } + /** * Generates new SQL for statements with an order by clause * diff --git a/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php b/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php index 24bfa63e6..78394c603 100644 --- a/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php +++ b/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php @@ -228,6 +228,36 @@ class LimitSubqueryOutputWalkerTest extends PaginationTestCase ); } + public function testCountQueryWithComplexScalarOrderByItemJoined() + { + $query = $this->entityManager->createQuery( + 'SELECT u FROM Doctrine\Tests\ORM\Tools\Pagination\User u JOIN u.avatar a ORDER BY a.image_height * a.image_width DESC' + ); + $this->entityManager->getConnection()->setDatabasePlatform(new MySqlPlatform()); + + $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, 'Doctrine\ORM\Tools\Pagination\LimitSubqueryOutputWalker'); + + $this->assertSame( + 'SELECT DISTINCT id_0, image_height_1 * image_width_2 FROM (SELECT u0_.id AS id_0, a1_.image_height AS image_height_1, a1_.image_width AS image_width_2, a1_.user_id AS user_id_3 FROM User u0_ INNER JOIN Avatar a1_ ON u0_.id = a1_.user_id) dctrn_result ORDER BY image_height_1 * image_width_2 DESC', + $query->getSQL() + ); + } + + public function testCountQueryWithComplexScalarOrderByItemJoinedWithPartial() + { + $query = $this->entityManager->createQuery( + 'SELECT u, partial a.{id, image_alt_desc} FROM Doctrine\Tests\ORM\Tools\Pagination\User u JOIN u.avatar a ORDER BY a.image_height * a.image_width DESC' + ); + $this->entityManager->getConnection()->setDatabasePlatform(new MySqlPlatform()); + + $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, 'Doctrine\ORM\Tools\Pagination\LimitSubqueryOutputWalker'); + + $this->assertSame( + 'SELECT DISTINCT id_0, image_height_3 * image_width_4 FROM (SELECT u0_.id AS id_0, a1_.id AS id_1, a1_.image_alt_desc AS image_alt_desc_2, a1_.image_height AS image_height_3, a1_.image_width AS image_width_4, a1_.user_id AS user_id_5 FROM User u0_ INNER JOIN Avatar a1_ ON u0_.id = a1_.user_id) dctrn_result ORDER BY image_height_3 * image_width_4 DESC', + $query->getSQL() + ); + } + public function testCountQueryWithComplexScalarOrderByItemOracle() { $query = $this->entityManager->createQuery( @@ -284,7 +314,7 @@ class LimitSubqueryOutputWalkerTest extends PaginationTestCase $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, 'Doctrine\ORM\Tools\Pagination\LimitSubqueryOutputWalker'); $this->assertEquals( - 'SELECT DISTINCT id_0 FROM (SELECT b0_.id AS id_0, b0_.author_id AS author_id_1, b0_.category_id AS category_id_2 FROM BlogPost b0_ INNER JOIN Author a1_ ON b0_.author_id = a1_.id ORDER BY a1_.name ASC) dctrn_result', + 'SELECT DISTINCT id_0, name_1 FROM (SELECT b0_.id AS id_0, a1_.name AS name_1, b0_.author_id AS author_id_2, b0_.category_id AS category_id_3 FROM BlogPost b0_ INNER JOIN Author a1_ ON b0_.author_id = a1_.id) dctrn_result ORDER BY name_1 ASC', $query->getSQL() ); } diff --git a/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginationTestCase.php b/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginationTestCase.php index ab52011ac..d8c983f90 100644 --- a/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginationTestCase.php +++ b/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginationTestCase.php @@ -144,6 +144,10 @@ class User * ) */ public $groups; + /** + * @OneToOne(targetEntity="Avatar", mappedBy="user") + */ + public $avatar; } /** @Entity */