From c7a75f477f9f1d3aad79ab1f5d7c42a4cf166cfd Mon Sep 17 00:00:00 2001 From: Miha Vrhovnik Date: Tue, 18 Sep 2012 13:56:32 +0200 Subject: [PATCH 1/2] The distinct query should replicate the fields in order by clause and the order by clause itself from inner query This fixes DDC-1958 --- .../Pagination/LimitSubqueryOutputWalker.php | 41 ++++++++++++++++++- .../LimitSubqueryOutputWalkerTest.php | 30 +++++++++++++- .../Pagination/LimitSubqueryWalkerTest.php | 12 ++++++ .../Tools/Pagination/PaginationTestCase.php | 2 + 4 files changed, 82 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php index 2888805cb..f09217ffd 100644 --- a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php +++ b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php @@ -15,6 +15,7 @@ namespace Doctrine\ORM\Tools\Pagination; use Doctrine\ORM\Query\SqlWalker; use Doctrine\ORM\Query\AST\SelectStatement; +use Doctrine\DBAL\Platforms\PostgreSqlPlatform; /** * Wrap the query in order to select root entity IDs for pagination @@ -132,8 +133,44 @@ class LimitSubqueryOutputWalker extends SqlWalker } // Build the counter query - $sql = sprintf('SELECT DISTINCT %s FROM (%s) dctrn_result', - implode(', ', $sqlIdentifier), $sql); + if ($this->platform instanceof PostgreSqlPlatform) { + //http://www.doctrine-project.org/jira/browse/DDC-1958 + + // 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) { + $possibleAliases = array_keys($this->rsm->fieldMappings, $item->expression->field); + + foreach ($possibleAliases as $alias) { + if ($this->rsm->columnOwnerMap[$alias] == $item->expression->identificationVariable) { + $sqlOrderColumns[] = $alias; + $orderBy[] = $alias . ' ' . $item->type; + break; + } + } + } + //remove identifier aliases + $sqlOrderColumns = array_diff($sqlOrderColumns, $sqlIdentifier); + } + + //we don't need orderBy in inner query + //However at least on 5.4.6 I'm getting a segmentation fault and thus we don't clear it for now + /*$AST->orderByClause = null; + $sql = parent::walkSelectStatement($AST);*/ + + if (count($orderBy)) { + $sql = sprintf('SELECT DISTINCT %s FROM (%s) dctrn_result ORDER BY %s', + implode(', ', array_merge($sqlIdentifier, $sqlOrderColumns)), $sql, implode(', ', $orderBy)); + } else { + $sql = sprintf('SELECT DISTINCT %s FROM (%s) dctrn_result', + implode(', ', $sqlIdentifier), $sql); + } + } else { + $sql = sprintf('SELECT DISTINCT %s FROM (%s) dctrn_result', + implode(', ', $sqlIdentifier), $sql); + } // Apply the limit and offset $sql = $this->platform->modifyLimitQuery( diff --git a/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php b/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php index 92c262cbd..362ffc0c8 100644 --- a/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php +++ b/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php @@ -10,14 +10,42 @@ class LimitSubqueryOutputWalkerTest extends PaginationTestCase { $query = $this->entityManager->createQuery( 'SELECT p, c, a FROM Doctrine\Tests\ORM\Tools\Pagination\MyBlogPost p JOIN p.category c JOIN p.author a'); + $query->expireQueryCache(true); $limitQuery = clone $query; $limitQuery->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, 'Doctrine\ORM\Tools\Pagination\LimitSubqueryOutputWalker'); $this->assertEquals( - "SELECT DISTINCT id0 FROM (SELECT m0_.id AS id0, c1_.id AS id1, a2_.id AS id2, a2_.name AS name3, m0_.author_id AS author_id4, m0_.category_id AS category_id5 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", $limitQuery->getSql() + "SELECT DISTINCT id0 FROM (SELECT m0_.id AS id0, m0_.title AS title1, c1_.id AS id2, a2_.id AS id3, a2_.name AS name4, m0_.author_id AS author_id5, m0_.category_id AS category_id6 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", $limitQuery->getSql() ); } + public function testLimitSubqueryWithSortPg() + { + $odp = $this->entityManager->getConnection()->getDatabasePlatform(); + $this->entityManager->getConnection()->setDatabasePlatform(new \Doctrine\DBAL\Platforms\PostgreSqlPlatform); + + $query = $this->entityManager->createQuery( + 'SELECT p, c, a FROM Doctrine\Tests\ORM\Tools\Pagination\MyBlogPost p JOIN p.category c JOIN p.author a ORDER BY p.title'); + $limitQuery = clone $query; + $limitQuery->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, 'Doctrine\ORM\Tools\Pagination\LimitSubqueryOutputWalker'); + + $this->assertEquals( + "SELECT DISTINCT id0, title1 FROM (SELECT m0_.id AS id0, m0_.title AS title1, c1_.id AS id2, a2_.id AS id3, a2_.name AS name4, m0_.author_id AS author_id5, m0_.category_id AS category_id6 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 title1 ASC", $limitQuery->getSql() + ); + + $this->entityManager->getConnection()->setDatabasePlatform($odp); + } + + public function testLimitSubqueryPg() + { + $odp = $this->entityManager->getConnection()->getDatabasePlatform(); + $this->entityManager->getConnection()->setDatabasePlatform(new \Doctrine\DBAL\Platforms\PostgreSqlPlatform); + + $this->testLimitSubquery(); + + $this->entityManager->getConnection()->setDatabasePlatform($odp); + } + public function testCountQuery_MixedResultsWithName() { $query = $this->entityManager->createQuery( diff --git a/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryWalkerTest.php b/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryWalkerTest.php index 2fd6046dd..bfec02461 100644 --- a/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryWalkerTest.php +++ b/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryWalkerTest.php @@ -21,6 +21,18 @@ class LimitSubqueryWalkerTest extends PaginationTestCase ); } + public function testLimitSubqueryWithSort() + { + $query = $this->entityManager->createQuery( + 'SELECT p, c, a FROM Doctrine\Tests\ORM\Tools\Pagination\MyBlogPost p JOIN p.category c JOIN p.author a ORDER BY p.title'); + $limitQuery = clone $query; + $limitQuery->setHint(Query::HINT_CUSTOM_TREE_WALKERS, array('Doctrine\ORM\Tools\Pagination\LimitSubqueryWalker')); + + $this->assertEquals( + "SELECT DISTINCT m0_.id AS id0, m0_.title AS title1 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", $limitQuery->getSql() + ); + } + public function testCountQuery_MixedResultsWithName() { $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 a6e4c67be..d503e81aa 100644 --- a/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginationTestCase.php +++ b/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginationTestCase.php @@ -31,6 +31,8 @@ class MyBlogPost * @ManyToOne(targetEntity="Category") */ public $category; + /** @column(type="string") */ + public $title; } /** From 8fe9fa0dc7d62ba9f9a4157c643c73d8c7436fbd Mon Sep 17 00:00:00 2001 From: Miha Vrhovnik Date: Mon, 12 Nov 2012 13:48:55 +0100 Subject: [PATCH 2/2] extracted pgsql sql generation into a helper method --- .../Pagination/LimitSubqueryOutputWalker.php | 84 +++++++++++-------- 1 file changed, 48 insertions(+), 36 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php index f09217ffd..a3e1df2dc 100644 --- a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php +++ b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php @@ -86,7 +86,7 @@ class LimitSubqueryOutputWalker extends SqlWalker */ public function walkSelectStatement(SelectStatement $AST) { - $sql = parent::walkSelectStatement($AST); + $innerSql = parent::walkSelectStatement($AST); // Find out the SQL alias of the identifier column of the root entity // It may be possible to make this work with multiple root entities but that @@ -133,43 +133,12 @@ class LimitSubqueryOutputWalker extends SqlWalker } // Build the counter query + $sql = sprintf('SELECT DISTINCT %s FROM (%s) dctrn_result', + implode(', ', $sqlIdentifier), $innerSql); + if ($this->platform instanceof PostgreSqlPlatform) { //http://www.doctrine-project.org/jira/browse/DDC-1958 - - // 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) { - $possibleAliases = array_keys($this->rsm->fieldMappings, $item->expression->field); - - foreach ($possibleAliases as $alias) { - if ($this->rsm->columnOwnerMap[$alias] == $item->expression->identificationVariable) { - $sqlOrderColumns[] = $alias; - $orderBy[] = $alias . ' ' . $item->type; - break; - } - } - } - //remove identifier aliases - $sqlOrderColumns = array_diff($sqlOrderColumns, $sqlIdentifier); - } - - //we don't need orderBy in inner query - //However at least on 5.4.6 I'm getting a segmentation fault and thus we don't clear it for now - /*$AST->orderByClause = null; - $sql = parent::walkSelectStatement($AST);*/ - - if (count($orderBy)) { - $sql = sprintf('SELECT DISTINCT %s FROM (%s) dctrn_result ORDER BY %s', - implode(', ', array_merge($sqlIdentifier, $sqlOrderColumns)), $sql, implode(', ', $orderBy)); - } else { - $sql = sprintf('SELECT DISTINCT %s FROM (%s) dctrn_result', - implode(', ', $sqlIdentifier), $sql); - } - } else { - $sql = sprintf('SELECT DISTINCT %s FROM (%s) dctrn_result', - implode(', ', $sqlIdentifier), $sql); + $this->getPostgresqlSql($AST, $sqlIdentifier, $innerSql, $sql); } // Apply the limit and offset @@ -187,4 +156,47 @@ class LimitSubqueryOutputWalker extends SqlWalker return $sql; } + + /** + * Generate new SQL for postgresql if necessary + * + * @param SelectStatement $AST + * @param array sqlIdentifier + * @param string $sql + */ + public function getPostgresqlSql(SelectStatement $AST, array $sqlIdentifier, $innerSql, &$sql) + { + // 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) { + $possibleAliases = array_keys($this->rsm->fieldMappings, $item->expression->field); + + foreach ($possibleAliases as $alias) { + if ($this->rsm->columnOwnerMap[$alias] == $item->expression->identificationVariable) { + $sqlOrderColumns[] = $alias; + $orderBy[] = $alias . ' ' . $item->type; + break; + } + } + } + //remove identifier aliases + $sqlOrderColumns = array_diff($sqlOrderColumns, $sqlIdentifier); + } + + //we don't need orderBy in inner query + //However at least on 5.4.6 I'm getting a segmentation fault and thus we don't clear it for now + /*$AST->orderByClause = null; + $innerSql = parent::walkSelectStatement($AST);*/ + + if (count($orderBy)) { + $sql = sprintf( + 'SELECT DISTINCT %s FROM (%s) dctrn_result ORDER BY %s', + implode(', ', array_merge($sqlIdentifier, $sqlOrderColumns)), + $innerSql, + implode(', ', $orderBy) + ); + } + } }