diff --git a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php index 338920b2e..b1d68e350 100644 --- a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php +++ b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php @@ -13,6 +13,10 @@ 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\SqlWalker; use Doctrine\ORM\Query\AST\SelectStatement; @@ -56,6 +60,18 @@ class LimitSubqueryOutputWalker extends SqlWalker */ private $maxResults; + /** + * @var \Doctrine\ORM\EntityManager + */ + private $em; + + /** + * The quote strategy. + * + * @var \Doctrine\ORM\Mapping\QuoteStrategy + */ + private $quoteStrategy; + /** * Constructor. * @@ -78,6 +94,9 @@ class LimitSubqueryOutputWalker extends SqlWalker $this->maxResults = $query->getMaxResults(); $query->setFirstResult(null)->setMaxResults(null); + $this->em = $query->getEntityManager(); + $this->quoteStrategy = $this->em->getConfiguration()->getQuoteStrategy(); + parent::__construct($query, $parserResult, $queryComponents); } @@ -92,6 +111,11 @@ class LimitSubqueryOutputWalker extends SqlWalker */ public function walkSelectStatement(SelectStatement $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; + $AST->orderByClause = null; + // Set every select expression as visible(hidden = false) to // make $AST have scalar mappings properly - this is relevant for referencing selected // fields from outside the subquery, for example in the ORDER BY segment @@ -163,7 +187,7 @@ class LimitSubqueryOutputWalker extends SqlWalker implode(', ', $sqlIdentifier), $innerSql); // http://www.doctrine-project.org/jira/browse/DDC-1958 - $sql = $this->preserveSqlOrdering($AST, $sqlIdentifier, $innerSql, $sql); + $sql = $this->preserveSqlOrdering($sqlIdentifier, $innerSql, $sql, $orderByClause); // Apply the limit and offset. $sql = $this->platform->modifyLimitQuery( @@ -182,41 +206,29 @@ class LimitSubqueryOutputWalker extends SqlWalker } /** - * Generates new SQL for Postgresql or Oracle if necessary. + * Generates new SQL for statements with an order by clause * - * @param SelectStatement $AST * @param array $sqlIdentifier * @param string $innerSql * @param string $sql + * @param OrderByClause $orderByClause * - * @return void + * @return string */ - public function preserveSqlOrdering(SelectStatement $AST, array $sqlIdentifier, $innerSql, $sql) + public function preserveSqlOrdering(array $sqlIdentifier, $innerSql, $sql, $orderByClause) { - // For every order by, find out the SQL alias by inspecting the ResultSetMapping. - $sqlOrderColumns = array(); - $orderBy = array(); - if (isset($AST->orderByClause)) { - foreach ($AST->orderByClause->orderByItems as $item) { - $expression = $item->expression; + // If the sql statement has an order by clause, we need to wrap it in a new select distinct + // statement + if ($orderByClause instanceof OrderByClause) { + // Rebuild the order by clause to work in the scope of the new select statement + /** @var array $sqlOrderColumns an array of items that need to be included in the select list */ + /** @var array $orderBy an array of rebuilt order by items */ + list($sqlOrderColumns, $orderBy) = $this->rebuildOrderByClauseForOuterScope($orderByClause); - $possibleAliases = $expression instanceof PathExpression - ? array_keys($this->rsm->fieldMappings, $expression->field) - : array_keys($this->rsm->scalarMappings, $expression); - - foreach ($possibleAliases as $alias) { - if (!is_object($expression) || $this->rsm->columnOwnerMap[$alias] == $expression->identificationVariable) { - $sqlOrderColumns[] = $alias; - $orderBy[] = $alias . ' ' . $item->type; - break; - } - } - } - // remove identifier aliases + // Identifiers are always included in the select list, so there's no need to include them twice $sqlOrderColumns = array_diff($sqlOrderColumns, $sqlIdentifier); - } - if (count($orderBy)) { + // Build the select distinct statement $sql = sprintf( 'SELECT DISTINCT %s FROM (%s) dctrn_result ORDER BY %s', implode(', ', array_merge($sqlIdentifier, $sqlOrderColumns)), @@ -227,4 +239,60 @@ class LimitSubqueryOutputWalker extends SqlWalker return $sql; } + + /** + * Generates a new order by clause that works in the scope of a select query wrapping the original + * + * @param OrderByClause $orderByClause + * @return array + */ + protected function rebuildOrderByClauseForOuterScope(OrderByClause $orderByClause) { + $dqlAliasToSqlTableAliasMap + = $searchPatterns + = $replacements + = $dqlAliasToClassMap + = $selectListAdditions + = $orderByItems + = array(); + + // Generate DQL alias -> SQL table alias mapping + foreach(array_keys($this->rsm->aliasMap) as $dqlAlias) { + $dqlAliasToClassMap[$dqlAlias] = $class = $this->queryComponents[$dqlAlias]['metadata']; + $dqlAliasToSqlTableAliasMap[$dqlAlias] = $this->getSQLTableAlias($class->getTableName(), $dqlAlias); + } + + // Pattern to find table path expressions in the order by clause + $fieldSearchPattern = "/(?rsm->fieldMappings as $fieldAlias => $columnName) { + $dqlAliasForFieldAlias = $this->rsm->columnOwnerMap[$fieldAlias]; + $columnName = $this->quoteStrategy->getColumnName( + $columnName, + $dqlAliasToClassMap[$dqlAliasForFieldAlias], + $this->em->getConnection()->getDatabasePlatform() + ); + + $sqlTableAliasForFieldAlias = $dqlAliasToSqlTableAliasMap[$dqlAliasForFieldAlias]; + + $searchPatterns[] = sprintf($fieldSearchPattern, $sqlTableAliasForFieldAlias, $columnName); + $replacements[] = $fieldAlias; + } + + foreach($orderByClause->orderByItems as $orderByItem) { + // Walk order by item to get string representation of it + $orderByItem = $this->walkOrderByItem($orderByItem); + + // Replace path expressions in the order by clause with their column alias + $orderByItem = preg_replace($searchPatterns, $replacements, $orderByItem); + + // The order by items are not required to be in the select list on Oracle and PostgreSQL, but + // for the sake of simplicity, order by items will be included in the select list on all platforms. + // This doesn't impact functionality. + $selectListAdditions[] = trim(preg_replace('/([^ ]+) (?:asc|desc)/i', '$1', $orderByItem)); + $orderByItems[] = $orderByItem; + } + + return array($selectListAdditions, $orderByItems); + } } diff --git a/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php b/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php index a012d45df..0fb717a79 100644 --- a/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php +++ b/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php @@ -5,6 +5,7 @@ namespace Doctrine\Tests\ORM\Tools\Pagination; use Doctrine\DBAL\Platforms\MySqlPlatform; use Doctrine\DBAL\Platforms\OraclePlatform; use Doctrine\DBAL\Platforms\PostgreSqlPlatform; +use Doctrine\DBAL\Platforms\SQLServerPlatform; use Doctrine\ORM\Query; class LimitSubqueryOutputWalkerTest extends PaginationTestCase @@ -33,7 +34,7 @@ class LimitSubqueryOutputWalkerTest extends PaginationTestCase $limitQuery->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, 'Doctrine\ORM\Tools\Pagination\LimitSubqueryOutputWalker'); $this->assertEquals( - "SELECT DISTINCT id_0, title_1 FROM (SELECT m0_.id AS id_0, m0_.title AS title_1, c1_.id AS id_2, a2_.id AS id_3, a2_.name AS name_4, m0_.author_id AS author_id_5, m0_.category_id AS category_id_6 FROM MyBlogPost m0_ INNER JOIN Category c1_ ON m0_.category_id = c1_.id INNER JOIN Author a2_ ON m0_.author_id = a2_.id ORDER BY m0_.title ASC) dctrn_result ORDER BY title_1 ASC", $limitQuery->getSql() + "SELECT DISTINCT id_0, title_1 FROM (SELECT m0_.id AS id_0, m0_.title AS title_1, c1_.id AS id_2, a2_.id AS id_3, a2_.name AS name_4, m0_.author_id AS author_id_5, m0_.category_id AS category_id_6 FROM MyBlogPost m0_ INNER JOIN Category c1_ ON m0_.category_id = c1_.id INNER JOIN Author a2_ ON m0_.author_id = a2_.id) dctrn_result ORDER BY title_1 ASC", $limitQuery->getSql() ); $this->entityManager->getConnection()->setDatabasePlatform($odp); @@ -51,7 +52,7 @@ class LimitSubqueryOutputWalkerTest extends PaginationTestCase $limitQuery->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, 'Doctrine\ORM\Tools\Pagination\LimitSubqueryOutputWalker'); $this->assertEquals( - "SELECT DISTINCT id_1, sclr_0 FROM (SELECT COUNT(g0_.id) AS sclr_0, u1_.id AS id_1, g0_.id AS id_2 FROM User u1_ INNER JOIN user_group u2_ ON u1_.id = u2_.user_id INNER JOIN groups g0_ ON g0_.id = u2_.group_id ORDER BY sclr_0 ASC) dctrn_result ORDER BY sclr_0 ASC", + "SELECT DISTINCT id_1, sclr_0 FROM (SELECT COUNT(g0_.id) AS sclr_0, u1_.id AS id_1, g0_.id AS id_2 FROM User u1_ INNER JOIN user_group u2_ ON u1_.id = u2_.user_id INNER JOIN groups g0_ ON g0_.id = u2_.group_id) dctrn_result ORDER BY sclr_0 ASC", $limitQuery->getSql() ); @@ -70,7 +71,7 @@ class LimitSubqueryOutputWalkerTest extends PaginationTestCase $limitQuery->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, 'Doctrine\ORM\Tools\Pagination\LimitSubqueryOutputWalker'); $this->assertEquals( - "SELECT DISTINCT id_1, sclr_0 FROM (SELECT COUNT(g0_.id) AS sclr_0, u1_.id AS id_1, g0_.id AS id_2 FROM User u1_ INNER JOIN user_group u2_ ON u1_.id = u2_.user_id INNER JOIN groups g0_ ON g0_.id = u2_.group_id ORDER BY sclr_0 ASC, u1_.id DESC) dctrn_result ORDER BY sclr_0 ASC, id_1 DESC", + "SELECT DISTINCT id_1, sclr_0 FROM (SELECT COUNT(g0_.id) AS sclr_0, u1_.id AS id_1, g0_.id AS id_2 FROM User u1_ INNER JOIN user_group u2_ ON u1_.id = u2_.user_id INNER JOIN groups g0_ ON g0_.id = u2_.group_id) dctrn_result ORDER BY sclr_0 ASC, id_1 DESC", $limitQuery->getSql() ); @@ -89,7 +90,7 @@ class LimitSubqueryOutputWalkerTest extends PaginationTestCase $limitQuery->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, 'Doctrine\ORM\Tools\Pagination\LimitSubqueryOutputWalker'); $this->assertEquals( - "SELECT DISTINCT id_1, sclr_0 FROM (SELECT COUNT(g0_.id) AS sclr_0, u1_.id AS id_1, g0_.id AS id_2 FROM User u1_ INNER JOIN user_group u2_ ON u1_.id = u2_.user_id INNER JOIN groups g0_ ON g0_.id = u2_.group_id ORDER BY sclr_0 ASC, u1_.id DESC) dctrn_result ORDER BY sclr_0 ASC, id_1 DESC", + "SELECT DISTINCT id_1, sclr_0 FROM (SELECT COUNT(g0_.id) AS sclr_0, u1_.id AS id_1, g0_.id AS id_2 FROM User u1_ INNER JOIN user_group u2_ ON u1_.id = u2_.user_id INNER JOIN groups g0_ ON g0_.id = u2_.group_id) dctrn_result ORDER BY sclr_0 ASC, id_1 DESC", $limitQuery->getSql() ); @@ -118,7 +119,7 @@ class LimitSubqueryOutputWalkerTest extends PaginationTestCase $limitQuery->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, 'Doctrine\ORM\Tools\Pagination\LimitSubqueryOutputWalker'); $this->assertEquals( - "SELECT DISTINCT ID_0, TITLE_1 FROM (SELECT m0_.id AS ID_0, m0_.title AS TITLE_1, c1_.id AS ID_2, a2_.id AS ID_3, a2_.name AS NAME_4, m0_.author_id AS AUTHOR_ID_5, m0_.category_id AS CATEGORY_ID_6 FROM MyBlogPost m0_ INNER JOIN Category c1_ ON m0_.category_id = c1_.id INNER JOIN Author a2_ ON m0_.author_id = a2_.id ORDER BY m0_.title ASC) dctrn_result ORDER BY TITLE_1 ASC", $limitQuery->getSql() + "SELECT DISTINCT ID_0, TITLE_1 FROM (SELECT m0_.id AS ID_0, m0_.title AS TITLE_1, c1_.id AS ID_2, a2_.id AS ID_3, a2_.name AS NAME_4, m0_.author_id AS AUTHOR_ID_5, m0_.category_id AS CATEGORY_ID_6 FROM MyBlogPost m0_ INNER JOIN Category c1_ ON m0_.category_id = c1_.id INNER JOIN Author a2_ ON m0_.author_id = a2_.id) dctrn_result ORDER BY TITLE_1 ASC", $limitQuery->getSql() ); $this->entityManager->getConnection()->setDatabasePlatform($odp); @@ -137,7 +138,7 @@ class LimitSubqueryOutputWalkerTest extends PaginationTestCase $limitQuery->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, 'Doctrine\ORM\Tools\Pagination\LimitSubqueryOutputWalker'); $this->assertEquals( - "SELECT DISTINCT ID_1, SCLR_0 FROM (SELECT COUNT(g0_.id) AS SCLR_0, u1_.id AS ID_1, g0_.id AS ID_2 FROM User u1_ INNER JOIN user_group u2_ ON u1_.id = u2_.user_id INNER JOIN groups g0_ ON g0_.id = u2_.group_id ORDER BY SCLR_0 ASC) dctrn_result ORDER BY SCLR_0 ASC", + "SELECT DISTINCT ID_1, SCLR_0 FROM (SELECT COUNT(g0_.id) AS SCLR_0, u1_.id AS ID_1, g0_.id AS ID_2 FROM User u1_ INNER JOIN user_group u2_ ON u1_.id = u2_.user_id INNER JOIN groups g0_ ON g0_.id = u2_.group_id) dctrn_result ORDER BY SCLR_0 ASC", $limitQuery->getSql() ); @@ -157,7 +158,7 @@ class LimitSubqueryOutputWalkerTest extends PaginationTestCase $limitQuery->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, 'Doctrine\ORM\Tools\Pagination\LimitSubqueryOutputWalker'); $this->assertEquals( - "SELECT DISTINCT ID_1, SCLR_0 FROM (SELECT COUNT(g0_.id) AS SCLR_0, u1_.id AS ID_1, g0_.id AS ID_2 FROM User u1_ INNER JOIN user_group u2_ ON u1_.id = u2_.user_id INNER JOIN groups g0_ ON g0_.id = u2_.group_id ORDER BY SCLR_0 ASC, u1_.id DESC) dctrn_result ORDER BY SCLR_0 ASC, ID_1 DESC", + "SELECT DISTINCT ID_1, SCLR_0 FROM (SELECT COUNT(g0_.id) AS SCLR_0, u1_.id AS ID_1, g0_.id AS ID_2 FROM User u1_ INNER JOIN user_group u2_ ON u1_.id = u2_.user_id INNER JOIN groups g0_ ON g0_.id = u2_.group_id) dctrn_result ORDER BY SCLR_0 ASC, ID_1 DESC", $limitQuery->getSql() ); @@ -207,7 +208,37 @@ class LimitSubqueryOutputWalkerTest extends PaginationTestCase $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, 'Doctrine\ORM\Tools\Pagination\LimitSubqueryOutputWalker'); $this->assertSame( - 'SELECT DISTINCT id_0 FROM (SELECT a0_.id AS id_0, a0_.name AS name_1 FROM Author a0_ ORDER BY (1 - 1000) * 1 DESC) dctrn_result', + 'SELECT DISTINCT id_0, (1 - 1000) * 1 FROM (SELECT a0_.id AS id_0, a0_.name AS name_1 FROM Author a0_) dctrn_result ORDER BY (1 - 1000) * 1 DESC', + $query->getSQL() + ); + } + + public function testCountQueryWithComplexScalarOrderByItem() + { + $query = $this->entityManager->createQuery( + 'SELECT a FROM Doctrine\Tests\ORM\Tools\Pagination\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_2 * image_width_3 FROM (SELECT a0_.id AS id_0, a0_.image AS image_1, a0_.image_height AS image_height_2, a0_.image_width AS image_width_3, a0_.image_alt_desc AS image_alt_desc_4, a0_.user_id AS user_id_5 FROM Avatar a0_) dctrn_result ORDER BY image_height_2 * image_width_3 DESC', + $query->getSQL() + ); + } + + public function testCountQueryWithComplexScalarOrderByItemOracle() + { + $query = $this->entityManager->createQuery( + 'SELECT a FROM Doctrine\Tests\ORM\Tools\Pagination\Avatar a ORDER BY a.image_height * a.image_width DESC' + ); + $this->entityManager->getConnection()->setDatabasePlatform(new OraclePlatform()); + + $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, 'Doctrine\ORM\Tools\Pagination\LimitSubqueryOutputWalker'); + + $this->assertSame( + 'SELECT DISTINCT ID_0, IMAGE_HEIGHT_2 * IMAGE_WIDTH_3 FROM (SELECT a0_.id AS ID_0, a0_.image AS IMAGE_1, a0_.image_height AS IMAGE_HEIGHT_2, a0_.image_width AS IMAGE_WIDTH_3, a0_.image_alt_desc AS IMAGE_ALT_DESC_4, a0_.user_id AS USER_ID_5 FROM Avatar a0_) dctrn_result ORDER BY IMAGE_HEIGHT_2 * IMAGE_WIDTH_3 DESC', $query->getSQL() ); } @@ -224,9 +255,24 @@ class LimitSubqueryOutputWalkerTest extends PaginationTestCase $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, 'Doctrine\ORM\Tools\Pagination\LimitSubqueryOutputWalker'); $this->assertEquals( - 'SELECT DISTINCT id_0, name_2 FROM (SELECT a0_.id AS id_0, a0_.name AS name_1, a0_.name AS name_2 FROM Author a0_ ORDER BY name_2 DESC) dctrn_result ORDER BY name_2 DESC', + 'SELECT DISTINCT id_0, name_2 FROM (SELECT a0_.id AS id_0, a0_.name AS name_1, a0_.name AS name_2 FROM Author a0_) dctrn_result ORDER BY name_2 DESC', $query->getSql() ); } + + public function testLimitSubqueryWithColumnWithSortDirectionInName() + { + $query = $this->entityManager->createQuery( + 'SELECT a FROM Doctrine\Tests\ORM\Tools\Pagination\Avatar a ORDER BY a.image_alt_desc 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_alt_desc_4 FROM (SELECT a0_.id AS id_0, a0_.image AS image_1, a0_.image_height AS image_height_2, a0_.image_width AS image_width_3, a0_.image_alt_desc AS image_alt_desc_4, a0_.user_id AS user_id_5 FROM Avatar a0_) dctrn_result ORDER BY image_alt_desc_4 DESC', + $query->getSQL() + ); + } } diff --git a/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginationTestCase.php b/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginationTestCase.php index d3e77f6f0..ab52011ac 100644 --- a/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginationTestCase.php +++ b/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginationTestCase.php @@ -145,3 +145,23 @@ class User */ public $groups; } + +/** @Entity */ +class Avatar +{ + /** @Id @column(type="integer") @generatedValue */ + public $id; + /** + * @OneToOne(targetEntity="User", inversedBy="avatar") + * @JoinColumn(name="user_id", referencedColumnName="id") + */ + public $user; + /** @column(type="string", length=255) */ + public $image; + /** @column(type="integer") */ + public $image_height; + /** @column(type="integer") */ + public $image_width; + /** @column(type="string", length=255) */ + public $image_alt_desc; +} \ No newline at end of file