From eebce88146adcccb39c4d710d3ba419fa7dadec8 Mon Sep 17 00:00:00 2001 From: Bill Schaller Date: Tue, 17 Mar 2015 13:58:50 -0400 Subject: [PATCH 01/10] Revert "Revert "Merge branch 'hotfix/#1220-sort-paginator-subquery-output-only-once'"" This reverts commit 6a1755972d19af4c1c07bc9c555d456d66426bda. --- .../Pagination/LimitSubqueryOutputWalker.php | 120 ++++++++++++++---- .../LimitSubqueryOutputWalkerTest.php | 64 ++++++++-- .../Tools/Pagination/PaginationTestCase.php | 20 +++ 3 files changed, 169 insertions(+), 35 deletions(-) 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 e8d7910b3..24bfa63e6 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,11 +255,26 @@ 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() + ); + } + public function testLimitSubqueryWithOrderByInnerJoined() { $query = $this->entityManager->createQuery( 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 From df0875c596afc5625d3b39eab4fae0c8943ff8c7 Mon Sep 17 00:00:00 2001 From: Bill Schaller Date: Tue, 17 Mar 2015 17:32:28 -0400 Subject: [PATCH 02/10] 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 */ From d7105552653bb514131aade43c2e8a2d0b77c4b6 Mon Sep 17 00:00:00 2001 From: Bill Schaller Date: Tue, 17 Mar 2015 18:32:57 -0400 Subject: [PATCH 03/10] Remove shameful hack in LimitSubqueryOutputWalker - replace with significantly less shameful hack --- .../Pagination/LimitSubqueryOutputWalker.php | 77 ++++++++++++++----- 1 file changed, 59 insertions(+), 18 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php index a61557b92..0616562c2 100644 --- a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php +++ b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php @@ -15,7 +15,9 @@ namespace Doctrine\ORM\Tools\Pagination; use Doctrine\ORM\Query\AST\OrderByClause; use Doctrine\ORM\Query\AST\PartialObjectExpression; +use Doctrine\ORM\Query\AST\PathExpression; use Doctrine\ORM\Query\AST\SelectExpression; +use Doctrine\ORM\Query\Expr\OrderBy; use Doctrine\ORM\Query\SqlWalker; use Doctrine\ORM\Query\AST\SelectStatement; @@ -62,6 +64,11 @@ class LimitSubqueryOutputWalker extends SqlWalker */ private $em; + /** + * @var array + */ + private $orderByPathExpressions = []; + /** * The quote strategy. * @@ -101,17 +108,21 @@ class LimitSubqueryOutputWalker extends SqlWalker * Walks down a SelectStatement AST node, wrapping it in a SELECT DISTINCT. * * @param SelectStatement $AST + * @param bool $addMissingItemsFromOrderByToSelect * * @return string * * @throws \RuntimeException */ - public function walkSelectStatement(SelectStatement $AST) + public function walkSelectStatement(SelectStatement $AST, $addMissingItemsFromOrderByToSelect = true) { - // 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); + // We don't want to call this recursively! + if ($AST->orderByClause instanceof OrderByClause && $addMissingItemsFromOrderByToSelect) { + // 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 @@ -218,24 +229,32 @@ class LimitSubqueryOutputWalker extends SqlWalker */ 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; + $this->orderByPathExpressions = []; + + // We need to do this in another walker because otherwise we'll end up + // polluting the state of this one. + $walker = clone $this; + + // This will populate $orderByPathExpressions via + // LimitSubqueryOutputWalker::walkPathExpression, which will be called + // as the select statement is walked. We'll end up with an array of all + // path expressions referenced in the query. + $walker->walkSelectStatement($AST, false); + $orderByPathExpressions = $walker->getOrderByPathExpressions(); + + // Get a map of referenced identifiers to field names. $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; + foreach ($orderByPathExpressions as $pathExpression) { + $idVar = $pathExpression->identificationVariable; + $field = $pathExpression->field; + 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. + // that are already being selected in the query. foreach ($AST->selectClause->selectExpressions as $selectExpression) { if ($selectExpression instanceof SelectExpression) { $idVar = $selectExpression->expression; @@ -348,4 +367,26 @@ class LimitSubqueryOutputWalker extends SqlWalker return array($selectListAdditions, $orderByItems); } + + /** + * {@inheritdoc} + */ + public function walkPathExpression($pathExpr) + { + if (!in_array($pathExpr, $this->orderByPathExpressions)) { + $this->orderByPathExpressions[] = $pathExpr; + } + + return parent::walkPathExpression($pathExpr); + } + + /** + * getter for $orderByPathExpressions + * + * @return array + */ + public function getOrderByPathExpressions() + { + return $this->orderByPathExpressions; + } } From 5c4b6a214084107a99fbedb76b056a89871e92d1 Mon Sep 17 00:00:00 2001 From: Bill Schaller Date: Tue, 17 Mar 2015 18:44:03 -0400 Subject: [PATCH 04/10] resolve nitpicks from @Ocramius and @deeky666 --- .../Pagination/LimitSubqueryOutputWalker.php | 40 ++++++++++--------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php index 0616562c2..2366b5b63 100644 --- a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php +++ b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php @@ -291,24 +291,26 @@ class LimitSubqueryOutputWalker extends SqlWalker { // 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); - - // Identifiers are always included in the select list, so there's no need to include them twice - $sqlOrderColumns = array_diff($sqlOrderColumns, $sqlIdentifier); - - // Build the select distinct statement - $sql = sprintf( - 'SELECT DISTINCT %s FROM (%s) dctrn_result ORDER BY %s', - implode(', ', array_merge($sqlIdentifier, $sqlOrderColumns)), - $innerSql, - implode(', ', $orderBy) - ); + if (!$orderByClause instanceof OrderByClause) { + return $sql; } + // 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); + + // Identifiers are always included in the select list, so there's no need to include them twice + $sqlOrderColumns = array_diff($sqlOrderColumns, $sqlIdentifier); + + // Build the select distinct statement + $sql = sprintf( + 'SELECT DISTINCT %s FROM (%s) dctrn_result ORDER BY %s', + implode(', ', array_merge($sqlIdentifier, $sqlOrderColumns)), + $innerSql, + implode(', ', $orderBy) + ); + return $sql; } @@ -318,14 +320,14 @@ class LimitSubqueryOutputWalker extends SqlWalker * @param OrderByClause $orderByClause * @return array */ - protected function rebuildOrderByClauseForOuterScope(OrderByClause $orderByClause) { + private 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) { @@ -334,7 +336,7 @@ class LimitSubqueryOutputWalker extends SqlWalker } // Pattern to find table path expressions in the order by clause - $fieldSearchPattern = "/(?rsm->fieldMappings as $fieldAlias => $columnName) { From 591fd00d73c8643a69ad7469d78f54b83f23dc4a Mon Sep 17 00:00:00 2001 From: Bill Schaller Date: Thu, 19 Mar 2015 16:47:04 -0400 Subject: [PATCH 05/10] Fix capitalization of mapping annotations in PaginationTestCase.php --- .../Tools/Pagination/PaginationTestCase.php | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginationTestCase.php b/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginationTestCase.php index d8c983f90..f1553c13c 100644 --- a/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginationTestCase.php +++ b/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginationTestCase.php @@ -24,7 +24,7 @@ abstract class PaginationTestCase extends OrmTestCase class MyBlogPost { - /** @Id @column(type="integer") @generatedValue */ + /** @Id @Column(type="integer") @GeneratedValue */ public $id; /** * @ManyToOne(targetEntity="Author") @@ -34,7 +34,7 @@ class MyBlogPost * @ManyToOne(targetEntity="Category") */ public $category; - /** @column(type="string") */ + /** @Column(type="string") */ public $title; } @@ -44,7 +44,7 @@ class MyBlogPost class MyAuthor { - /** @Id @column(type="integer") @generatedValue */ + /** @Id @Column(type="integer") @GeneratedValue */ public $id; } @@ -55,7 +55,7 @@ class MyAuthor class MyCategory { - /** @id @column(type="integer") @generatedValue */ + /** @Id @Column(type="integer") @GeneratedValue */ public $id; } @@ -67,7 +67,7 @@ class MyCategory class BlogPost { - /** @Id @column(type="integer") @generatedValue */ + /** @Id @Column(type="integer") @GeneratedValue */ public $id; /** * @ManyToOne(targetEntity="Author") @@ -85,7 +85,7 @@ class BlogPost class Author { - /** @Id @column(type="integer") @generatedValue */ + /** @Id @Column(type="integer") @GeneratedValue */ public $id; /** @Column(type="string") */ public $name; @@ -98,7 +98,7 @@ class Author class Person { - /** @Id @column(type="integer") @generatedValue */ + /** @Id @Column(type="integer") @GeneratedValue */ public $id; /** @Column(type="string") */ public $name; @@ -113,7 +113,7 @@ class Person class Category { - /** @id @column(type="integer") @generatedValue */ + /** @Id @Column(type="integer") @GeneratedValue */ public $id; } @@ -123,7 +123,7 @@ class Category class Group { - /** @Id @column(type="integer") @generatedValue */ + /** @Id @Column(type="integer") @GeneratedValue */ public $id; /** @ManyToMany(targetEntity="User", mappedBy="groups") */ public $users; @@ -133,7 +133,7 @@ class Group class User { - /** @Id @column(type="integer") @generatedValue */ + /** @Id @Column(type="integer") @GeneratedValue */ public $id; /** * @ManyToMany(targetEntity="Group", inversedBy="users") @@ -153,19 +153,19 @@ class User /** @Entity */ class Avatar { - /** @Id @column(type="integer") @generatedValue */ + /** @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) */ + /** @Column(type="string", length=255) */ public $image; - /** @column(type="integer") */ + /** @Column(type="integer") */ public $image_height; - /** @column(type="integer") */ + /** @Column(type="integer") */ public $image_width; - /** @column(type="string", length=255) */ + /** @Column(type="string", length=255) */ public $image_alt_desc; } \ No newline at end of file From 4c84f544931edec5b9dce346b150c0fbd7220de4 Mon Sep 17 00:00:00 2001 From: Bill Schaller Date: Thu, 19 Mar 2015 17:09:10 -0400 Subject: [PATCH 06/10] Fix failures on SQL Server due to scalar select items not having an alias --- .../Pagination/LimitSubqueryOutputWalker.php | 19 +++++++++++++++---- .../LimitSubqueryOutputWalkerTest.php | 10 +++++----- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php index 2366b5b63..8ff3630a5 100644 --- a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php +++ b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php @@ -13,6 +13,8 @@ namespace Doctrine\ORM\Tools\Pagination; +use Doctrine\ORM\Query\AST\ArithmeticExpression; +use Doctrine\ORM\Query\AST\ArithmeticTerm; use Doctrine\ORM\Query\AST\OrderByClause; use Doctrine\ORM\Query\AST\PartialObjectExpression; use Doctrine\ORM\Query\AST\PathExpression; @@ -353,18 +355,27 @@ class LimitSubqueryOutputWalker extends SqlWalker $replacements[] = $fieldAlias; } + $complexAddedOrderByAliases = 0; foreach($orderByClause->orderByItems as $orderByItem) { // Walk order by item to get string representation of it - $orderByItem = $this->walkOrderByItem($orderByItem); + $orderByItemString = $this->walkOrderByItem($orderByItem); // Replace path expressions in the order by clause with their column alias - $orderByItem = preg_replace($searchPatterns, $replacements, $orderByItem); + $orderByItemString = preg_replace($searchPatterns, $replacements, $orderByItemString); // 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; + $selectListAddition = trim(preg_replace('/([^ ]+) (?:asc|desc)/i', '$1', $orderByItemString)); + + // If the expression is an arithmetic expression, we need to create an alias for it. + if ($orderByItem->expression instanceof ArithmeticTerm) { + $orderByAlias = "ordr_" . $complexAddedOrderByAliases++; + $orderByItemString = $orderByAlias . " " . $orderByItem->type; + $selectListAddition .= " AS $orderByAlias"; + } + $selectListAdditions[] = $selectListAddition; + $orderByItems[] = $orderByItemString; } 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 78394c603..c733c228b 100644 --- a/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php +++ b/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php @@ -208,7 +208,7 @@ class LimitSubqueryOutputWalkerTest extends PaginationTestCase $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, 'Doctrine\ORM\Tools\Pagination\LimitSubqueryOutputWalker'); $this->assertSame( - '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', + 'SELECT DISTINCT id_0, (1 - 1000) * 1 AS ordr_0 FROM (SELECT a0_.id AS id_0, a0_.name AS name_1 FROM Author a0_) dctrn_result ORDER BY ordr_0 DESC', $query->getSQL() ); } @@ -223,7 +223,7 @@ class LimitSubqueryOutputWalkerTest extends PaginationTestCase $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', + 'SELECT DISTINCT id_0, image_height_2 * image_width_3 AS ordr_0 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 ordr_0 DESC', $query->getSQL() ); } @@ -238,7 +238,7 @@ class LimitSubqueryOutputWalkerTest extends PaginationTestCase $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', + 'SELECT DISTINCT id_0, image_height_1 * image_width_2 AS ordr_0 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 ordr_0 DESC', $query->getSQL() ); } @@ -253,7 +253,7 @@ class LimitSubqueryOutputWalkerTest extends PaginationTestCase $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', + 'SELECT DISTINCT id_0, image_height_3 * image_width_4 AS ordr_0 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 ordr_0 DESC', $query->getSQL() ); } @@ -268,7 +268,7 @@ class LimitSubqueryOutputWalkerTest extends PaginationTestCase $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', + 'SELECT DISTINCT ID_0, IMAGE_HEIGHT_2 * IMAGE_WIDTH_3 AS ordr_0 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 ordr_0 DESC', $query->getSQL() ); } From 81ccd93b74e5104f42b655c73862facfc1595726 Mon Sep 17 00:00:00 2001 From: Bill Schaller Date: Thu, 19 Mar 2015 21:59:15 -0400 Subject: [PATCH 07/10] Add thorough functional tests for Paginator, expand existing ones. --- .../Tests/Models/Pagination/Company.php | 35 +++ .../Doctrine/Tests/Models/Pagination/Logo.php | 46 ++++ .../Tests/ORM/Functional/PaginationTest.php | 252 ++++++++++++++++-- .../Doctrine/Tests/OrmFunctionalTestCase.php | 9 + 4 files changed, 313 insertions(+), 29 deletions(-) create mode 100644 tests/Doctrine/Tests/Models/Pagination/Company.php create mode 100644 tests/Doctrine/Tests/Models/Pagination/Logo.php diff --git a/tests/Doctrine/Tests/Models/Pagination/Company.php b/tests/Doctrine/Tests/Models/Pagination/Company.php new file mode 100644 index 000000000..2ff740d39 --- /dev/null +++ b/tests/Doctrine/Tests/Models/Pagination/Company.php @@ -0,0 +1,35 @@ +useModelSet('cms'); + $this->useModelSet('pagination'); parent::setUp(); $this->populate(); } @@ -35,7 +34,7 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase $paginator = new Paginator($query); $paginator->setUseOutputWalkers($useOutputWalkers); - $this->assertCount(3, $paginator); + $this->assertCount(9, $paginator); } /** @@ -48,7 +47,7 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase $paginator = new Paginator($query); $paginator->setUseOutputWalkers($useOutputWalkers); - $this->assertCount(3, $paginator); + $this->assertCount(9, $paginator); } public function testCountComplexWithOutputWalker() @@ -58,33 +57,144 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase $paginator = new Paginator($query); $paginator->setUseOutputWalkers(true); + $this->assertCount(3, $paginator); + } + + /** + * @expectedException + */ + public function testCountComplexWithoutOutputWalker() + { + $dql = "SELECT g, COUNT(u.id) AS userCount FROM Doctrine\Tests\Models\CMS\CmsGroup g LEFT JOIN g.users u GROUP BY g HAVING COUNT(u.id) > 0"; + $query = $this->_em->createQuery($dql); + + $paginator = new Paginator($query); + $paginator->setUseOutputWalkers(false); + + $this->setExpectedException( + 'RuntimeException', + 'Cannot count query that uses a HAVING clause. Use the output walkers for pagination' + ); + $this->assertCount(3, $paginator); + } + + /** + * @dataProvider useOutputWalkers + */ + public function testCountWithComplexScalarOrderBy($useOutputWalkers) + { + $dql = 'SELECT l FROM Doctrine\Tests\Models\Pagination\Logo l ORDER BY l.image_width * l.image_height DESC'; + $query = $this->_em->createQuery($dql); + + $paginator = new Paginator($query); + $paginator->setUseOutputWalkers($useOutputWalkers); $this->assertCount(9, $paginator); } /** - * @dataProvider useOutputWalkers + * @dataProvider useOutputWalkersAndFetchJoinCollection */ - public function testIterateSimpleWithoutJoinFetchJoinHandlingOff($useOutputWalkers) + public function testIterateSimpleWithoutJoin($useOutputWalkers, $fetchJoinCollection) { $dql = "SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u"; $query = $this->_em->createQuery($dql); - $paginator = new Paginator($query, false); + $paginator = new Paginator($query, $fetchJoinCollection); + $paginator->setUseOutputWalkers($useOutputWalkers); + $this->assertCount(9, $paginator->getIterator()); + + // Test with limit + $query->setMaxResults(3); + $paginator = new Paginator($query, $fetchJoinCollection); + $paginator->setUseOutputWalkers($useOutputWalkers); + $this->assertCount(3, $paginator->getIterator()); + + // Test with limit and offset + $query->setMaxResults(3)->setFirstResult(4); + $paginator = new Paginator($query, $fetchJoinCollection); $paginator->setUseOutputWalkers($useOutputWalkers); $this->assertCount(3, $paginator->getIterator()); } - /** - * @dataProvider useOutputWalkers - */ - public function testIterateSimpleWithoutJoinFetchJoinHandlingOn($useOutputWalkers) + private function iterateWithOrder($useOutputWalkers, $fetchJoinCollection, $baseDql, $checkField) { - $dql = "SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u"; + // Ascending + $dql = "$baseDql ASC"; $query = $this->_em->createQuery($dql); - $paginator = new Paginator($query, true); + $paginator = new Paginator($query, $fetchJoinCollection); $paginator->setUseOutputWalkers($useOutputWalkers); - $this->assertCount(3, $paginator->getIterator()); + $iter = $paginator->getIterator(); + $this->assertCount(9, $iter); + $result = iterator_to_array($iter); + $this->assertEquals($checkField . "0", $result[0]->$checkField); + + // With limit + $query->setMaxResults(3); + $paginator = new Paginator($query, $fetchJoinCollection); + $paginator->setUseOutputWalkers($useOutputWalkers); + $iter = $paginator->getIterator(); + $this->assertCount(3, $iter); + $result = iterator_to_array($iter); + $this->assertEquals($checkField . "0", $result[0]->$checkField); + + // With offset + $query->setMaxResults(3)->setFirstResult(3); + $paginator = new Paginator($query, $fetchJoinCollection); + $paginator->setUseOutputWalkers($useOutputWalkers); + $iter = $paginator->getIterator(); + $this->assertCount(3, $iter); + $result = iterator_to_array($iter); + $this->assertEquals($checkField . "3", $result[0]->$checkField); + + // Descending + $dql = "$baseDql DESC"; + $query = $this->_em->createQuery($dql); + + $paginator = new Paginator($query, $fetchJoinCollection); + $paginator->setUseOutputWalkers($useOutputWalkers); + $iter = $paginator->getIterator(); + $this->assertCount(9, $iter); + $result = iterator_to_array($iter); + $this->assertEquals($checkField . "8", $result[0]->$checkField); + + // With limit + $query->setMaxResults(3); + $paginator = new Paginator($query, $fetchJoinCollection); + $paginator->setUseOutputWalkers($useOutputWalkers); + $iter = $paginator->getIterator(); + $this->assertCount(3, $iter); + $result = iterator_to_array($iter); + $this->assertEquals($checkField . "8", $result[0]->$checkField); + + // With offset + $query->setMaxResults(3)->setFirstResult(3); + $paginator = new Paginator($query, $fetchJoinCollection); + $paginator->setUseOutputWalkers($useOutputWalkers); + $iter = $paginator->getIterator(); + $this->assertCount(3, $iter); + $result = iterator_to_array($iter); + $this->assertEquals($checkField . "5", $result[0]->$checkField); + } + + /** + * @dataProvider useOutputWalkersAndFetchJoinCollection + */ + public function testIterateSimpleWithoutJoinWithOrder($useOutputWalkers, $fetchJoinCollection) + { + // Ascending + $dql = "SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u ORDER BY u.username"; + $this->iterateWithOrder($useOutputWalkers, $fetchJoinCollection, $dql, "username"); + } + + /** + * @dataProvider fetchJoinCollection + */ + public function testIterateSimpleWithOutputWalkerWithoutJoinWithComplexOrder($fetchJoinCollection) + { + // Ascending + $dql = "SELECT l FROM Doctrine\Tests\Models\Pagination\Logo l ORDER BY l.image_width * l.image_height"; + $this->iterateWithOrder(true, $fetchJoinCollection, $dql, "image"); } /** @@ -97,7 +207,54 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase $paginator = new Paginator($query, true); $paginator->setUseOutputWalkers($useOutputWalkers); - $this->assertCount(3, $paginator->getIterator()); + $this->assertCount(9, $paginator->getIterator()); + } + + /** + * @dataProvider useOutputWalkers + */ + public function testIterateWithFetchJoinWithOrder($useOutputWalkers) + { + $dql = 'SELECT u,g FROM Doctrine\Tests\Models\CMS\CmsUser u JOIN u.groups g ORDER BY u.username'; + $this->iterateWithOrder($useOutputWalkers, true, $dql, "username"); + } + + /** + * @dataProvider useOutputWalkersAndFetchJoinCollection + */ + public function testIterateWithRegularJoinWithOrderByColumnFromJoined($useOutputWalkers, $fetchJoinCollection) + { + $dql = 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u JOIN u.email e ORDER BY e.email'; + $this->iterateWithOrder($useOutputWalkers, $fetchJoinCollection, $dql, "username"); + } + + /** + * @dataProvider fetchJoinCollection + */ + public function testIterateWithOutputWalkersWithRegularJoinWithComplexOrderByReferencingJoined($fetchJoinCollection) + { + // long function name is loooooooooooong + + $dql = "SELECT c FROM Doctrine\Tests\Models\Pagination\Company c JOIN c.logo l ORDER BY l.image_height * l.image_width"; + $this->iterateWithOrder(true, $fetchJoinCollection, $dql, "name"); + } + + /** + * @dataProvider useOutputWalkers + */ + public function testIterateWithFetchJoinWithOrderByColumnFromJoined($useOutputWalkers) + { + $dql = "SELECT u,g,e FROM Doctrine\Tests\Models\CMS\CmsUser u JOIN u.groups g JOIN u.email e ORDER BY e.email"; + $this->iterateWithOrder($useOutputWalkers, true, $dql, "username"); + } + + /** + * @dataProvider fetchJoinCollection + */ + public function testIterateWithOutputWalkersWithFetchJoinWithComplexOrderByReferencingJoined($fetchJoinCollection) + { + $dql = "SELECT c,l FROM Doctrine\Tests\Models\Pagination\Company c JOIN c.logo l ORDER BY l.image_width * l.image_height"; + $this->iterateWithOrder(true, $fetchJoinCollection, $dql, "name"); } public function testIterateComplexWithOutputWalker() @@ -107,7 +264,7 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase $paginator = new Paginator($query); $paginator->setUseOutputWalkers(true); - $this->assertCount(9, $paginator->getIterator()); + $this->assertCount(3, $paginator->getIterator()); } public function testDetectOutputWalker() @@ -170,7 +327,7 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase $getCountQuery->setAccessible(true); $this->assertCount(2, $getCountQuery->invoke($paginator)->getParameters()); - $this->assertCount(3, $paginator); + $this->assertCount(9, $paginator); $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, 'Doctrine\ORM\Query\SqlWalker'); @@ -179,25 +336,44 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase // if select part of query is replaced with count(...) paginator should remove // parameters from query object not used in new query. $this->assertCount(1, $getCountQuery->invoke($paginator)->getParameters()); - $this->assertCount(3, $paginator); + $this->assertCount(9, $paginator); } public function populate() { - for ($i = 0; $i < 3; $i++) { + $groups = []; + for ($j = 0; $j < 3; $j++) {; + $group = new CmsGroup(); + $group->name = "group$j"; + $groups[] = $group; + $this->_em->persist($group); + } + + for ($i = 0; $i < 9; $i++) { $user = new CmsUser(); $user->name = "Name$i"; $user->username = "username$i"; $user->status = "active"; - $this->_em->persist($user); - - for ($j = 0; $j < 3; $j++) {; - $group = new CmsGroup(); - $group->name = "group$j"; - $user->addGroup($group); - $this->_em->persist($group); + $user->email = new CmsEmail(); + $user->email->user = $user; + $user->email->email = "email$i"; + for ($j = 0; $j < 3; $j++) { + $user->addGroup($groups[$j]); } + $this->_em->persist($user); } + + for ($i = 0; $i < 9; $i++) { + $company = new Company(); + $company->name = "name$i"; + $company->logo = new Logo(); + $company->logo->image = "image$i"; + $company->logo->image_width = 100 + $i; + $company->logo->image_height = 100 + $i; + $company->logo->company = $company; + $this->_em->persist($company); + } + $this->_em->flush(); } @@ -208,6 +384,24 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase array(false), ); } + + public function fetchJoinCollection() + { + return array( + array(true), + array(false), + ); + } + + public function useOutputWalkersAndFetchJoinCollection() + { + return array( + array(true, false), + array(true, true), + array(false, false), + array(false, true), + ); + } } class CustomPaginationTestTreeWalker extends Query\TreeWalkerAdapter diff --git a/tests/Doctrine/Tests/OrmFunctionalTestCase.php b/tests/Doctrine/Tests/OrmFunctionalTestCase.php index 6feb8ab97..c03a4d04a 100644 --- a/tests/Doctrine/Tests/OrmFunctionalTestCase.php +++ b/tests/Doctrine/Tests/OrmFunctionalTestCase.php @@ -267,6 +267,10 @@ abstract class OrmFunctionalTestCase extends OrmTestCase 'Doctrine\Tests\Models\CustomType\CustomIdObjectTypeParent', 'Doctrine\Tests\Models\CustomType\CustomIdObjectTypeChild', ), + 'pagination' => array( + 'Doctrine\Tests\Models\Pagination\Company', + 'Doctrine\Tests\Models\Pagination\Logo', + ), ); /** @@ -515,6 +519,11 @@ abstract class OrmFunctionalTestCase extends OrmTestCase $conn->executeUpdate('DELETE FROM custom_id_type_parent'); } + if (isset($this->_usedModelSets['pagination'])) { + $conn->executeUpdate('DELETE FROM pagination_logo'); + $conn->executeUpdate('DELETE FROM pagination_company'); + } + $this->_em->clear(); } From 147bdd8ede1eebac97b4a157594b07538bc6d95c Mon Sep 17 00:00:00 2001 From: Bill Schaller Date: Fri, 20 Mar 2015 12:28:55 -0400 Subject: [PATCH 08/10] Fixed nitpicks from @stof --- .../Tests/Models/Pagination/Company.php | 7 +- .../Doctrine/Tests/Models/Pagination/Logo.php | 7 +- .../Tests/ORM/Functional/PaginationTest.php | 238 +++++++++++++++--- 3 files changed, 212 insertions(+), 40 deletions(-) diff --git a/tests/Doctrine/Tests/Models/Pagination/Company.php b/tests/Doctrine/Tests/Models/Pagination/Company.php index 2ff740d39..f14d1b91e 100644 --- a/tests/Doctrine/Tests/Models/Pagination/Company.php +++ b/tests/Doctrine/Tests/Models/Pagination/Company.php @@ -1,10 +1,4 @@ _em->createQuery($dql); $paginator = new Paginator($query); @@ -42,7 +42,7 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase */ public function testCountWithFetchJoin($useOutputWalkers) { - $dql = "SELECT u,g FROM Doctrine\Tests\Models\CMS\CmsUser u JOIN u.groups g"; + $dql = 'SELECT u,g FROM Doctrine\Tests\Models\CMS\CmsUser u JOIN u.groups g'; $query = $this->_em->createQuery($dql); $paginator = new Paginator($query); @@ -52,7 +52,7 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testCountComplexWithOutputWalker() { - $dql = "SELECT g, COUNT(u.id) AS userCount FROM Doctrine\Tests\Models\CMS\CmsGroup g LEFT JOIN g.users u GROUP BY g HAVING COUNT(u.id) > 0"; + $dql = 'SELECT g, COUNT(u.id) AS userCount FROM Doctrine\Tests\Models\CMS\CmsGroup g LEFT JOIN g.users u GROUP BY g HAVING COUNT(u.id) > 0'; $query = $this->_em->createQuery($dql); $paginator = new Paginator($query); @@ -60,12 +60,9 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertCount(3, $paginator); } - /** - * @expectedException - */ public function testCountComplexWithoutOutputWalker() { - $dql = "SELECT g, COUNT(u.id) AS userCount FROM Doctrine\Tests\Models\CMS\CmsGroup g LEFT JOIN g.users u GROUP BY g HAVING COUNT(u.id) > 0"; + $dql = 'SELECT g, COUNT(u.id) AS userCount FROM Doctrine\Tests\Models\CMS\CmsGroup g LEFT JOIN g.users u GROUP BY g HAVING COUNT(u.id) > 0'; $query = $this->_em->createQuery($dql); $paginator = new Paginator($query); @@ -96,7 +93,7 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase */ public function testIterateSimpleWithoutJoin($useOutputWalkers, $fetchJoinCollection) { - $dql = "SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u"; + $dql = 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u'; $query = $this->_em->createQuery($dql); $paginator = new Paginator($query, $fetchJoinCollection); @@ -116,8 +113,9 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertCount(3, $paginator->getIterator()); } - private function iterateWithOrder($useOutputWalkers, $fetchJoinCollection, $baseDql, $checkField) + private function iterateWithOrderAsc($useOutputWalkers, $fetchJoinCollection, $baseDql, $checkField) { + // Ascending $dql = "$baseDql ASC"; $query = $this->_em->createQuery($dql); @@ -128,6 +126,14 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertCount(9, $iter); $result = iterator_to_array($iter); $this->assertEquals($checkField . "0", $result[0]->$checkField); + } + + private function iterateWithOrderAscWithLimit($useOutputWalkers, $fetchJoinCollection, $baseDql, $checkField) + { + + // Ascending + $dql = "$baseDql ASC"; + $query = $this->_em->createQuery($dql); // With limit $query->setMaxResults(3); @@ -137,6 +143,13 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertCount(3, $iter); $result = iterator_to_array($iter); $this->assertEquals($checkField . "0", $result[0]->$checkField); + } + + private function iterateWithOrderAscWithLimitAndOffset($useOutputWalkers, $fetchJoinCollection, $baseDql, $checkField) + { + // Ascending + $dql = "$baseDql ASC"; + $query = $this->_em->createQuery($dql); // With offset $query->setMaxResults(3)->setFirstResult(3); @@ -146,8 +159,10 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertCount(3, $iter); $result = iterator_to_array($iter); $this->assertEquals($checkField . "3", $result[0]->$checkField); + } - // Descending + private function iterateWithOrderDesc($useOutputWalkers, $fetchJoinCollection, $baseDql, $checkField) + { $dql = "$baseDql DESC"; $query = $this->_em->createQuery($dql); @@ -157,6 +172,12 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertCount(9, $iter); $result = iterator_to_array($iter); $this->assertEquals($checkField . "8", $result[0]->$checkField); + } + + private function iterateWithOrderDescWithLimit($useOutputWalkers, $fetchJoinCollection, $baseDql, $checkField) + { + $dql = "$baseDql DESC"; + $query = $this->_em->createQuery($dql); // With limit $query->setMaxResults(3); @@ -166,6 +187,12 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertCount(3, $iter); $result = iterator_to_array($iter); $this->assertEquals($checkField . "8", $result[0]->$checkField); + } + + private function iterateWithOrderDescWithLimitAndOffset($useOutputWalkers, $fetchJoinCollection, $baseDql, $checkField) + { + $dql = "$baseDql DESC"; + $query = $this->_em->createQuery($dql); // With offset $query->setMaxResults(3)->setFirstResult(3); @@ -183,8 +210,31 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testIterateSimpleWithoutJoinWithOrder($useOutputWalkers, $fetchJoinCollection) { // Ascending - $dql = "SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u ORDER BY u.username"; - $this->iterateWithOrder($useOutputWalkers, $fetchJoinCollection, $dql, "username"); + $dql = 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u ORDER BY u.username'; + $this->iterateWithOrderAsc($useOutputWalkers, $fetchJoinCollection, $dql, "username"); + $this->iterateWithOrderDesc($useOutputWalkers, $fetchJoinCollection, $dql, "username"); + } + + /** + * @dataProvider useOutputWalkersAndFetchJoinCollection + */ + public function testIterateSimpleWithoutJoinWithOrderAndLimit($useOutputWalkers, $fetchJoinCollection) + { + // Ascending + $dql = 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u ORDER BY u.username'; + $this->iterateWithOrderAscWithLimit($useOutputWalkers, $fetchJoinCollection, $dql, "username"); + $this->iterateWithOrderDescWithLimit($useOutputWalkers, $fetchJoinCollection, $dql, "username"); + } + + /** + * @dataProvider useOutputWalkersAndFetchJoinCollection + */ + public function testIterateSimpleWithoutJoinWithOrderAndLimitAndOffset($useOutputWalkers, $fetchJoinCollection) + { + // Ascending + $dql = 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u ORDER BY u.username'; + $this->iterateWithOrderAscWithLimitAndOffset($useOutputWalkers, $fetchJoinCollection, $dql, "username"); + $this->iterateWithOrderDescWithLimitAndOffset($useOutputWalkers, $fetchJoinCollection, $dql, "username"); } /** @@ -193,8 +243,31 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testIterateSimpleWithOutputWalkerWithoutJoinWithComplexOrder($fetchJoinCollection) { // Ascending - $dql = "SELECT l FROM Doctrine\Tests\Models\Pagination\Logo l ORDER BY l.image_width * l.image_height"; - $this->iterateWithOrder(true, $fetchJoinCollection, $dql, "image"); + $dql = 'SELECT l FROM Doctrine\Tests\Models\Pagination\Logo l ORDER BY l.image_width * l.image_height'; + $this->iterateWithOrderAsc(true, $fetchJoinCollection, $dql, "image"); + $this->iterateWithOrderDesc(true, $fetchJoinCollection, $dql, "image"); + } + + /** + * @dataProvider fetchJoinCollection + */ + public function testIterateSimpleWithOutputWalkerWithoutJoinWithComplexOrderAndLimit($fetchJoinCollection) + { + // Ascending + $dql = 'SELECT l FROM Doctrine\Tests\Models\Pagination\Logo l ORDER BY l.image_width * l.image_height'; + $this->iterateWithOrderAscWithLimit(true, $fetchJoinCollection, $dql, "image"); + $this->iterateWithOrderDescWithLimit(true, $fetchJoinCollection, $dql, "image"); + } + + /** + * @dataProvider fetchJoinCollection + */ + public function testIterateSimpleWithOutputWalkerWithoutJoinWithComplexOrderAndLimitAndOffset($fetchJoinCollection) + { + // Ascending + $dql = 'SELECT l FROM Doctrine\Tests\Models\Pagination\Logo l ORDER BY l.image_width * l.image_height'; + $this->iterateWithOrderAscWithLimitAndOffset(true, $fetchJoinCollection, $dql, "image"); + $this->iterateWithOrderDescWithLimitAndOffset(true, $fetchJoinCollection, $dql, "image"); } /** @@ -202,7 +275,7 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase */ public function testIterateWithFetchJoin($useOutputWalkers) { - $dql = "SELECT u,g FROM Doctrine\Tests\Models\CMS\CmsUser u JOIN u.groups g"; + $dql = 'SELECT u,g FROM Doctrine\Tests\Models\CMS\CmsUser u JOIN u.groups g'; $query = $this->_em->createQuery($dql); $paginator = new Paginator($query, true); @@ -216,7 +289,28 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testIterateWithFetchJoinWithOrder($useOutputWalkers) { $dql = 'SELECT u,g FROM Doctrine\Tests\Models\CMS\CmsUser u JOIN u.groups g ORDER BY u.username'; - $this->iterateWithOrder($useOutputWalkers, true, $dql, "username"); + $this->iterateWithOrderAsc($useOutputWalkers, true, $dql, "username"); + $this->iterateWithOrderDesc($useOutputWalkers, true, $dql, "username"); + } + + /** + * @dataProvider useOutputWalkers + */ + public function testIterateWithFetchJoinWithOrderAndLimit($useOutputWalkers) + { + $dql = 'SELECT u,g FROM Doctrine\Tests\Models\CMS\CmsUser u JOIN u.groups g ORDER BY u.username'; + $this->iterateWithOrderAscWithLimit($useOutputWalkers, true, $dql, "username"); + $this->iterateWithOrderDescWithLimit($useOutputWalkers, true, $dql, "username"); + } + + /** + * @dataProvider useOutputWalkers + */ + public function testIterateWithFetchJoinWithOrderAndLimitAndOffset($useOutputWalkers) + { + $dql = 'SELECT u,g FROM Doctrine\Tests\Models\CMS\CmsUser u JOIN u.groups g ORDER BY u.username'; + $this->iterateWithOrderAscWithLimitAndOffset($useOutputWalkers, true, $dql, "username"); + $this->iterateWithOrderDescWithLimitAndOffset($useOutputWalkers, true, $dql, "username"); } /** @@ -225,7 +319,28 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testIterateWithRegularJoinWithOrderByColumnFromJoined($useOutputWalkers, $fetchJoinCollection) { $dql = 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u JOIN u.email e ORDER BY e.email'; - $this->iterateWithOrder($useOutputWalkers, $fetchJoinCollection, $dql, "username"); + $this->iterateWithOrderAsc($useOutputWalkers, $fetchJoinCollection, $dql, "username"); + $this->iterateWithOrderDesc($useOutputWalkers, $fetchJoinCollection, $dql, "username"); + } + + /** + * @dataProvider useOutputWalkersAndFetchJoinCollection + */ + public function testIterateWithRegularJoinWithOrderByColumnFromJoinedWithLimit($useOutputWalkers, $fetchJoinCollection) + { + $dql = 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u JOIN u.email e ORDER BY e.email'; + $this->iterateWithOrderAscWithLimit($useOutputWalkers, $fetchJoinCollection, $dql, "username"); + $this->iterateWithOrderDescWithLimit($useOutputWalkers, $fetchJoinCollection, $dql, "username"); + } + + /** + * @dataProvider useOutputWalkersAndFetchJoinCollection + */ + public function testIterateWithRegularJoinWithOrderByColumnFromJoinedWithLimitAndOffset($useOutputWalkers, $fetchJoinCollection) + { + $dql = 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u JOIN u.email e ORDER BY e.email'; + $this->iterateWithOrderAscWithLimitAndOffset($useOutputWalkers, $fetchJoinCollection, $dql, "username"); + $this->iterateWithOrderDescWithLimitAndOffset($useOutputWalkers, $fetchJoinCollection, $dql, "username"); } /** @@ -235,8 +350,33 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase { // long function name is loooooooooooong - $dql = "SELECT c FROM Doctrine\Tests\Models\Pagination\Company c JOIN c.logo l ORDER BY l.image_height * l.image_width"; - $this->iterateWithOrder(true, $fetchJoinCollection, $dql, "name"); + $dql = 'SELECT c FROM Doctrine\Tests\Models\Pagination\Company c JOIN c.logo l ORDER BY l.image_height * l.image_width'; + $this->iterateWithOrderAsc(true, $fetchJoinCollection, $dql, "name"); + $this->iterateWithOrderDesc(true, $fetchJoinCollection, $dql, "name"); + } + + /** + * @dataProvider fetchJoinCollection + */ + public function testIterateWithOutputWalkersWithRegularJoinWithComplexOrderByReferencingJoinedWithLimit($fetchJoinCollection) + { + // long function name is loooooooooooong + + $dql = 'SELECT c FROM Doctrine\Tests\Models\Pagination\Company c JOIN c.logo l ORDER BY l.image_height * l.image_width'; + $this->iterateWithOrderAscWithLimit(true, $fetchJoinCollection, $dql, "name"); + $this->iterateWithOrderDescWithLimit(true, $fetchJoinCollection, $dql, "name"); + } + + /** + * @dataProvider fetchJoinCollection + */ + public function testIterateWithOutputWalkersWithRegularJoinWithComplexOrderByReferencingJoinedWithLimitAndOffset($fetchJoinCollection) + { + // long function name is loooooooooooong + + $dql = 'SELECT c FROM Doctrine\Tests\Models\Pagination\Company c JOIN c.logo l ORDER BY l.image_height * l.image_width'; + $this->iterateWithOrderAscWithLimitAndOffset(true, $fetchJoinCollection, $dql, "name"); + $this->iterateWithOrderDescWithLimitAndOffset(true, $fetchJoinCollection, $dql, "name"); } /** @@ -244,8 +384,29 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase */ public function testIterateWithFetchJoinWithOrderByColumnFromJoined($useOutputWalkers) { - $dql = "SELECT u,g,e FROM Doctrine\Tests\Models\CMS\CmsUser u JOIN u.groups g JOIN u.email e ORDER BY e.email"; - $this->iterateWithOrder($useOutputWalkers, true, $dql, "username"); + $dql = 'SELECT u,g,e FROM Doctrine\Tests\Models\CMS\CmsUser u JOIN u.groups g JOIN u.email e ORDER BY e.email'; + $this->iterateWithOrderAsc($useOutputWalkers, true, $dql, "username"); + $this->iterateWithOrderDesc($useOutputWalkers, true, $dql, "username"); + } + + /** + * @dataProvider useOutputWalkers + */ + public function testIterateWithFetchJoinWithOrderByColumnFromJoinedWithLimit($useOutputWalkers) + { + $dql = 'SELECT u,g,e FROM Doctrine\Tests\Models\CMS\CmsUser u JOIN u.groups g JOIN u.email e ORDER BY e.email'; + $this->iterateWithOrderAscWithLimit($useOutputWalkers, true, $dql, "username"); + $this->iterateWithOrderDescWithLimit($useOutputWalkers, true, $dql, "username"); + } + + /** + * @dataProvider useOutputWalkers + */ + public function testIterateWithFetchJoinWithOrderByColumnFromJoinedWithLimitAndOffset($useOutputWalkers) + { + $dql = 'SELECT u,g,e FROM Doctrine\Tests\Models\CMS\CmsUser u JOIN u.groups g JOIN u.email e ORDER BY e.email'; + $this->iterateWithOrderAscWithLimitAndOffset($useOutputWalkers, true, $dql, "username"); + $this->iterateWithOrderDescWithLimitAndOffset($useOutputWalkers, true, $dql, "username"); } /** @@ -253,13 +414,34 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase */ public function testIterateWithOutputWalkersWithFetchJoinWithComplexOrderByReferencingJoined($fetchJoinCollection) { - $dql = "SELECT c,l FROM Doctrine\Tests\Models\Pagination\Company c JOIN c.logo l ORDER BY l.image_width * l.image_height"; - $this->iterateWithOrder(true, $fetchJoinCollection, $dql, "name"); + $dql = 'SELECT c,l FROM Doctrine\Tests\Models\Pagination\Company c JOIN c.logo l ORDER BY l.image_width * l.image_height'; + $this->iterateWithOrderAsc(true, $fetchJoinCollection, $dql, "name"); + $this->iterateWithOrderDesc(true, $fetchJoinCollection, $dql, "name"); + } + + /** + * @dataProvider fetchJoinCollection + */ + public function testIterateWithOutputWalkersWithFetchJoinWithComplexOrderByReferencingJoinedWithLimit($fetchJoinCollection) + { + $dql = 'SELECT c,l FROM Doctrine\Tests\Models\Pagination\Company c JOIN c.logo l ORDER BY l.image_width * l.image_height'; + $this->iterateWithOrderAscWithLimit(true, $fetchJoinCollection, $dql, "name"); + $this->iterateWithOrderDescWithLimit(true, $fetchJoinCollection, $dql, "name"); + } + + /** + * @dataProvider fetchJoinCollection + */ + public function testIterateWithOutputWalkersWithFetchJoinWithComplexOrderByReferencingJoinedWithLimitAndOffset($fetchJoinCollection) + { + $dql = 'SELECT c,l FROM Doctrine\Tests\Models\Pagination\Company c JOIN c.logo l ORDER BY l.image_width * l.image_height'; + $this->iterateWithOrderAscWithLimitAndOffset(true, $fetchJoinCollection, $dql, "name"); + $this->iterateWithOrderDescWithLimitAndOffset(true, $fetchJoinCollection, $dql, "name"); } public function testIterateComplexWithOutputWalker() { - $dql = "SELECT g, COUNT(u.id) AS userCount FROM Doctrine\Tests\Models\CMS\CmsGroup g LEFT JOIN g.users u GROUP BY g HAVING COUNT(u.id) > 0"; + $dql = 'SELECT g, COUNT(u.id) AS userCount FROM Doctrine\Tests\Models\CMS\CmsGroup g LEFT JOIN g.users u GROUP BY g HAVING COUNT(u.id) > 0'; $query = $this->_em->createQuery($dql); $paginator = new Paginator($query); @@ -270,7 +452,7 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testDetectOutputWalker() { // This query works using the output walkers but causes an exception using the TreeWalker - $dql = "SELECT g, COUNT(u.id) AS userCount FROM Doctrine\Tests\Models\CMS\CmsGroup g LEFT JOIN g.users u GROUP BY g HAVING COUNT(u.id) > 0"; + $dql = 'SELECT g, COUNT(u.id) AS userCount FROM Doctrine\Tests\Models\CMS\CmsGroup g LEFT JOIN g.users u GROUP BY g HAVING COUNT(u.id) > 0'; $query = $this->_em->createQuery($dql); // If the Paginator detects the custom output walker it should fall back to using the @@ -288,7 +470,7 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testCloneQuery() { - $dql = "SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u"; + $dql = 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u'; $query = $this->_em->createQuery($dql); $paginator = new Paginator($query); @@ -299,7 +481,7 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testQueryWalkerIsKept() { - $dql = "SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u"; + $dql = 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u'; $query = $this->_em->createQuery($dql); $query->setHint(Query::HINT_CUSTOM_TREE_WALKERS, array('Doctrine\Tests\ORM\Functional\CustomPaginationTestTreeWalker')); @@ -341,7 +523,7 @@ class PaginationTest extends \Doctrine\Tests\OrmFunctionalTestCase public function populate() { - $groups = []; + $groups = array(); for ($j = 0; $j < 3; $j++) {; $group = new CmsGroup(); $group->name = "group$j"; From 45d39dd50d3434656c09a28a7817308a057047f6 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Tue, 24 Mar 2015 00:36:13 +0000 Subject: [PATCH 09/10] Making `preserveSqlOrdering` API `private` --- .../ORM/Tools/Pagination/LimitSubqueryOutputWalker.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php index 8ff3630a5..fb2354a49 100644 --- a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php +++ b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php @@ -289,11 +289,11 @@ class LimitSubqueryOutputWalker extends SqlWalker * * @return string */ - public function preserveSqlOrdering(array $sqlIdentifier, $innerSql, $sql, $orderByClause) + private function preserveSqlOrdering(array $sqlIdentifier, $innerSql, $sql, $orderByClause) { // If the sql statement has an order by clause, we need to wrap it in a new select distinct // statement - if (!$orderByClause instanceof OrderByClause) { + if (! $orderByClause instanceof OrderByClause) { return $sql; } From d97d3ec0e57bcdc1a76ec7df2ec1ca208c865a01 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Tue, 24 Mar 2015 00:37:17 +0000 Subject: [PATCH 10/10] Minor CS fixes (braces) --- .../ORM/Tools/Pagination/LimitSubqueryOutputWalker.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php index fb2354a49..0c17a1357 100644 --- a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php +++ b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php @@ -322,7 +322,8 @@ class LimitSubqueryOutputWalker extends SqlWalker * @param OrderByClause $orderByClause * @return array */ - private function rebuildOrderByClauseForOuterScope(OrderByClause $orderByClause) { + private function rebuildOrderByClauseForOuterScope(OrderByClause $orderByClause) + { $dqlAliasToSqlTableAliasMap = $searchPatterns = $replacements