From a4ebc08c4f25e3a6e9d2a284f07c5d46afd72249 Mon Sep 17 00:00:00 2001
From: Bill Schaller <bill@zeroedin.com>
Date: Mon, 15 Dec 2014 17:46:48 -0500
Subject: [PATCH 1/8] * Modified tests in LimitSubqueryOutputWalkerTest.php to
not have duplicated order by clauses * Modified LimitSubqueryOutputWalker to
not duplicate order by clauses
---
.../Pagination/LimitSubqueryOutputWalker.php | 23 ++++++++++++++-----
.../LimitSubqueryOutputWalkerTest.php | 18 +++++++--------
2 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php
index 338920b2e..ce3c171de 100644
--- a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php
+++ b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php
@@ -13,6 +13,7 @@
namespace Doctrine\ORM\Tools\Pagination;
+use Doctrine\ORM\Query\AST\OrderByClause;
use Doctrine\ORM\Query\AST\PathExpression;
use Doctrine\ORM\Query\SqlWalker;
use Doctrine\ORM\Query\AST\SelectStatement;
@@ -92,6 +93,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 +169,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(
@@ -184,20 +190,20 @@ class LimitSubqueryOutputWalker extends SqlWalker
/**
* Generates new SQL for Postgresql or Oracle if necessary.
*
- * @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) {
+ if ($orderByClause instanceof OrderByClause) {
+ foreach ($orderByClause->orderByItems as $item) {
$expression = $item->expression;
$possibleAliases = $expression instanceof PathExpression
@@ -211,12 +217,17 @@ class LimitSubqueryOutputWalker extends SqlWalker
break;
}
}
+ if(empty($possibleAliases)) {
+ $orderBy[] = $this->walkOrderByItem($item);
+ }
+
}
// remove identifier aliases
$sqlOrderColumns = array_diff($sqlOrderColumns, $sqlIdentifier);
}
if (count($orderBy)) {
+
$sql = sprintf(
'SELECT DISTINCT %s FROM (%s) dctrn_result ORDER BY %s',
implode(', ', array_merge($sqlIdentifier, $sqlOrderColumns)),
diff --git a/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php b/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php
index a012d45df..027ae11e2 100644
--- a/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php
+++ b/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php
@@ -33,7 +33,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 +51,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 +70,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 +89,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 +118,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 +137,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 +157,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 +207,7 @@ 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 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()
);
}
@@ -224,7 +224,7 @@ 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()
);
}
From 42bea80a6a1e21c839fe1fa8ed7b6191408f3ab2 Mon Sep 17 00:00:00 2001
From: Bill Schaller <bill@zeroedin.com>
Date: Tue, 16 Dec 2014 11:59:13 -0500
Subject: [PATCH 2/8] Added failing test cases for limit queries with with
complex scalar order by items
---
.../LimitSubqueryOutputWalkerTest.php | 31 +++++++++++++++++++
.../Tools/Pagination/PaginationTestCase.php | 18 +++++++++++
2 files changed, 49 insertions(+)
diff --git a/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php b/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php
index 027ae11e2..3015a7044 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
@@ -212,6 +213,36 @@ class LimitSubqueryOutputWalkerTest extends PaginationTestCase
);
}
+ 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_.user_id AS user_id_4 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_.user_id AS USER_ID_4 FROM Avatar a0_) dctrn_result ORDER BY IMAGE_HEIGHT_2 * IMAGE_WIDTH_3 DESC',
+ $query->getSQL()
+ );
+ }
+
/**
* @group DDC-3434
*/
diff --git a/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginationTestCase.php b/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginationTestCase.php
index d3e77f6f0..ffc4c4669 100644
--- a/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginationTestCase.php
+++ b/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginationTestCase.php
@@ -145,3 +145,21 @@ 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;
+}
\ No newline at end of file
From ed800e4b862e8626eab953a520ca64fd2e31fa7d Mon Sep 17 00:00:00 2001
From: Bill Schaller <bill@zeroedin.com>
Date: Tue, 16 Dec 2014 12:00:35 -0500
Subject: [PATCH 3/8] Added function to LimitSubqueryOutputWalker which takes
an order by clause and rebuilds it to work in the scope of the wrapping query
---
.../Pagination/LimitSubqueryOutputWalker.php | 117 +++++++++++++-----
1 file changed, 89 insertions(+), 28 deletions(-)
diff --git a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php
index ce3c171de..4a5a38c10 100644
--- a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php
+++ b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php
@@ -57,6 +57,18 @@ class LimitSubqueryOutputWalker extends SqlWalker
*/
private $maxResults;
+ /**
+ * @var \Doctrine\ORM\EntityManager
+ */
+ private $em;
+
+ /**
+ * The quote strategy.
+ *
+ * @var \Doctrine\ORM\Mapping\QuoteStrategy
+ */
+ private $quoteStrategy;
+
/**
* Constructor.
*
@@ -79,6 +91,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);
}
@@ -188,7 +203,7 @@ class LimitSubqueryOutputWalker extends SqlWalker
}
/**
- * Generates new SQL for Postgresql or Oracle if necessary.
+ * Generates new SQL for SQL Server, Postgresql, or Oracle if necessary.
*
* @param array $sqlIdentifier
* @param string $innerSql
@@ -199,43 +214,89 @@ class LimitSubqueryOutputWalker extends SqlWalker
*/
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();
+ // Get order by clause as a string
+ $orderBy = null;
if ($orderByClause instanceof OrderByClause) {
- foreach ($orderByClause->orderByItems as $item) {
- $expression = $item->expression;
-
- $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;
- }
- }
- if(empty($possibleAliases)) {
- $orderBy[] = $this->walkOrderByItem($item);
- }
-
- }
- // remove identifier aliases
- $sqlOrderColumns = array_diff($sqlOrderColumns, $sqlIdentifier);
+ $orderBy = $this->walkOrderByClause($orderByClause);
}
- if (count($orderBy)) {
+ // If the sql statement has an order by clause, we need to wrap it in a new select distinct
+ // statement
+ if ($orderBy) {
+ // Rebuild the order by clause to work in the scope of the new select statement
+ list($sqlOrderColumns, $orderBy) = $this->rebuildOrderByClauseForOuterScope($orderBy);
+ // 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',
+ 'SELECT DISTINCT %s FROM (%s) dctrn_result%s',
implode(', ', array_merge($sqlIdentifier, $sqlOrderColumns)),
$innerSql,
- implode(', ', $orderBy)
+ $orderBy
);
}
return $sql;
}
+
+ /**
+ * Generates a new order by clause that works in the scope of a select query wrapping the original
+ *
+ * @param string $orderByClause
+ * @return array
+ */
+ protected function rebuildOrderByClauseForOuterScope($orderByClause) {
+ $dqlAliasToSqlTableAliasMap
+ = $searchPatterns
+ = $replacements
+ = $dqlAliasToClassMap
+ = $replacedAliases
+ = 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 = "/(?<![a-z0-9_])%s\.%s(?![a-z0-9_])/i";
+
+ // Generate search patterns for each field's path expression in the order by clause
+ foreach($this->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;
+ }
+
+ // Scalar expression aliases will not be modified in the order by clause, but will
+ // be included in the select list of the wrapping query
+ $scalarSearchPattern = "/(?<![a-z0-9_])%s(?![a-z0-9_])/i";
+ foreach(array_keys($this->rsm->scalarMappings) as $scalarField) {
+ if(preg_match(sprintf($scalarSearchPattern, $scalarField), $orderByClause)) {
+ $replacedAliases[] = $scalarField;
+ }
+ }
+
+ // Replace path expressions in the order by clause with their column alias
+ foreach($searchPatterns as $index => $pattern) {
+ $newOrderByClause = preg_replace($pattern, $replacements[$index], $orderByClause);
+ if ($newOrderByClause !== $orderByClause) {
+ $orderByClause = $newOrderByClause;
+ $replacedAliases[] = $replacements[$index];
+ }
+ }
+
+ return array($replacedAliases, $orderByClause);
+ }
}
From 7031539314dfb61663c84ca11a999530f792f1c9 Mon Sep 17 00:00:00 2001
From: Bill Schaller <bill@zeroedin.com>
Date: Tue, 16 Dec 2014 12:57:06 -0500
Subject: [PATCH 4/8] Fixed SQL that could be considered invalid on the
targeted platforms in LimitSubqueryOutputWalkerTest
---
.../ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php b/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php
index 3015a7044..9430a8769 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 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 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()
);
}
@@ -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_.user_id AS user_id_4 FROM Avatar a0_) dctrn_result ORDER BY image_height_2 * image_width_3 DESC',
+ '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_.user_id AS user_id_4 FROM Avatar a0_) dctrn_result ORDER BY image_height_2 * image_width_3 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_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_.user_id AS USER_ID_4 FROM Avatar a0_) dctrn_result ORDER BY IMAGE_HEIGHT_2 * IMAGE_WIDTH_3 DESC',
+ '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_.user_id AS USER_ID_4 FROM Avatar a0_) dctrn_result ORDER BY IMAGE_HEIGHT_2 * IMAGE_WIDTH_3 DESC',
$query->getSQL()
);
}
From dfc091075660d63d2776fea0c6bdd60be18958a0 Mon Sep 17 00:00:00 2001
From: Bill Schaller <bill@zeroedin.com>
Date: Tue, 16 Dec 2014 13:00:29 -0500
Subject: [PATCH 5/8] Fixed how order by items are included in the select list
of the select distinct wrapper statement
---
.../Pagination/LimitSubqueryOutputWalker.php | 56 +++++++++----------
1 file changed, 26 insertions(+), 30 deletions(-)
diff --git a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php
index 4a5a38c10..eda839265 100644
--- a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php
+++ b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php
@@ -13,6 +13,9 @@
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;
@@ -214,27 +217,23 @@ class LimitSubqueryOutputWalker extends SqlWalker
*/
public function preserveSqlOrdering(array $sqlIdentifier, $innerSql, $sql, $orderByClause)
{
- // Get order by clause as a string
- $orderBy = null;
- if ($orderByClause instanceof OrderByClause) {
- $orderBy = $this->walkOrderByClause($orderByClause);
- }
-
// If the sql statement has an order by clause, we need to wrap it in a new select distinct
// statement
- if ($orderBy) {
+ if ($orderByClause instanceof OrderByClause) {
// Rebuild the order by clause to work in the scope of the new select statement
- list($sqlOrderColumns, $orderBy) = $this->rebuildOrderByClauseForOuterScope($orderBy);
+ /** @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%s',
+ 'SELECT DISTINCT %s FROM (%s) dctrn_result ORDER BY %s',
implode(', ', array_merge($sqlIdentifier, $sqlOrderColumns)),
$innerSql,
- $orderBy
+ implode(', ', $orderBy)
);
}
@@ -244,15 +243,16 @@ class LimitSubqueryOutputWalker extends SqlWalker
/**
* Generates a new order by clause that works in the scope of a select query wrapping the original
*
- * @param string $orderByClause
+ * @param OrderByClause $orderByClause
* @return array
*/
- protected function rebuildOrderByClauseForOuterScope($orderByClause) {
+ protected function rebuildOrderByClauseForOuterScope(OrderByClause $orderByClause) {
$dqlAliasToSqlTableAliasMap
= $searchPatterns
= $replacements
= $dqlAliasToClassMap
- = $replacedAliases
+ = $selectListAdditions
+ = $orderByItems
= array();
// Generate DQL alias -> SQL table alias mapping
@@ -279,24 +279,20 @@ class LimitSubqueryOutputWalker extends SqlWalker
$replacements[] = $fieldAlias;
}
- // Scalar expression aliases will not be modified in the order by clause, but will
- // be included in the select list of the wrapping query
- $scalarSearchPattern = "/(?<![a-z0-9_])%s(?![a-z0-9_])/i";
- foreach(array_keys($this->rsm->scalarMappings) as $scalarField) {
- if(preg_match(sprintf($scalarSearchPattern, $scalarField), $orderByClause)) {
- $replacedAliases[] = $scalarField;
- }
+ 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(str_ireplace(array("asc", "desc"), "", $orderByItem));
+ $orderByItems[] = $orderByItem;
}
- // Replace path expressions in the order by clause with their column alias
- foreach($searchPatterns as $index => $pattern) {
- $newOrderByClause = preg_replace($pattern, $replacements[$index], $orderByClause);
- if ($newOrderByClause !== $orderByClause) {
- $orderByClause = $newOrderByClause;
- $replacedAliases[] = $replacements[$index];
- }
- }
-
- return array($replacedAliases, $orderByClause);
+ return array($selectListAdditions, $orderByItems);
}
}
From 8350de781fdb42fae274f62735b6822d9b2fa767 Mon Sep 17 00:00:00 2001
From: Bill Schaller <bill@zeroedin.com>
Date: Tue, 16 Dec 2014 13:03:38 -0500
Subject: [PATCH 6/8] Doc fix
---
lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php
index eda839265..654177324 100644
--- a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php
+++ b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php
@@ -206,7 +206,7 @@ class LimitSubqueryOutputWalker extends SqlWalker
}
/**
- * Generates new SQL for SQL Server, Postgresql, or Oracle if necessary.
+ * Generates new SQL for statements with an order by clause
*
* @param array $sqlIdentifier
* @param string $innerSql
From 39aeb9935b4aaa723918ac7a4d53ba83ccb856c5 Mon Sep 17 00:00:00 2001
From: Bill Schaller <bill@zeroedin.com>
Date: Wed, 17 Dec 2014 16:00:43 -0500
Subject: [PATCH 7/8] Added failing testcase for Limit queries on entities with
column names containing 'asc' or desc'
---
.../LimitSubqueryOutputWalkerTest.php | 19 +++++++++++++++++--
.../Tools/Pagination/PaginationTestCase.php | 2 ++
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php b/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php
index 9430a8769..0fb717a79 100644
--- a/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php
+++ b/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php
@@ -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_.user_id AS user_id_4 FROM Avatar a0_) dctrn_result ORDER BY image_height_2 * image_width_3 DESC',
+ '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()
);
}
@@ -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_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_.user_id AS USER_ID_4 FROM Avatar a0_) dctrn_result ORDER BY IMAGE_HEIGHT_2 * IMAGE_WIDTH_3 DESC',
+ '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()
);
}
@@ -259,5 +259,20 @@ class LimitSubqueryOutputWalkerTest extends PaginationTestCase
$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 ffc4c4669..ab52011ac 100644
--- a/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginationTestCase.php
+++ b/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginationTestCase.php
@@ -162,4 +162,6 @@ class Avatar
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 3fd3da3d46e4c863b63e69dffa442ee5edb62f30 Mon Sep 17 00:00:00 2001
From: Bill Schaller <bill@zeroedin.com>
Date: Wed, 17 Dec 2014 16:00:54 -0500
Subject: [PATCH 8/8] Fixed removal of ASC and DESC keywords from orderby items
that will be included in select list
---
lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php
index 654177324..b1d68e350 100644
--- a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php
+++ b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php
@@ -289,7 +289,7 @@ class LimitSubqueryOutputWalker extends SqlWalker
// 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(str_ireplace(array("asc", "desc"), "", $orderByItem));
+ $selectListAdditions[] = trim(preg_replace('/([^ ]+) (?:asc|desc)/i', '$1', $orderByItem));
$orderByItems[] = $orderByItem;
}