Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed PostgreSQL and Oracle pagination issues #1325

Closed
wants to merge 8 commits into from
Closed

fixed PostgreSQL and Oracle pagination issues #1325

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8fbd32b
fixed PostgreSQL and Oracle pagination issues
vaheshadunts Mar 9, 2015
b68c93c
Fixed missing spaces around if on line 250
vaheshadunts Mar 9, 2015
827d53f
fixed array declaration syntax for <5.4 versions
vaheshadunts Mar 9, 2015
43d7ef7
updated testcases relaying on changes
vaheshadunts Mar 10, 2015
fca3767
Tests
vaheshadunts Mar 10, 2015
a09f1cb
lexer check 'as' > 'AS'
vaheshadunts Mar 10, 2015
dfc1264
Syntax fixing as suggested
vaheshadunts Mar 17, 2015
c674a15
Stylistic changes
vaheshadunts Mar 17, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 47 additions & 6 deletions lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,12 +217,53 @@ public function preserveSqlOrdering(SelectStatement $AST, array $sqlIdentifier,
}

if (count($orderBy)) {
$sql = sprintf(
'SELECT DISTINCT %s FROM (%s) dctrn_result ORDER BY %s',
implode(', ', array_merge($sqlIdentifier, $sqlOrderColumns)),
$innerSql,
implode(', ', $orderBy)
);

// http://www.doctrine-project.org/jira/browse/DDC-1958
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this entire block be moved to a private method?

if ($this->platform->getName() == 'postgresql' || $this->platform->getName() == 'oracle') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use instanceof here

if (preg_match('/SELECT (.*?) FROM /i', $innerSql, $regs)) {
$selectedParts = explode(",", $regs[1]);
}
$globalOrderByStmt = SqlWalker::walkOrderByClause($AST->orderByClause);

$innerRowNumberSelectPart = ' ROW_NUMBER() OVER(%s) as ROWNUM';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AS should be kept capital, to be consistent in the tests.

$mainRowNumberSelectPart = 'MIN(ROWNUM) AS MINROWNUM';
$mainRowNumberOrderPart = 'MINROWNUM ASC';


/*
* Adding row_number in select statement, instead of selecting all fields in order clause
* being away from the DISTINCTION leaks, (e.g. 1-a, a 1-b)
*/
$selectedParts[] = sprintf($innerRowNumberSelectPart, $globalOrderByStmt);

$selectClause = implode(',', $selectedParts);
$innerSql = preg_replace("/^\s*select .+ from (.*)$/im", "SELECT {$selectClause} FROM $1", $innerSql);

// Grouping by primary key and min(rownumber) for correct result
$sql = sprintf(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Return directly

'SELECT %s FROM (%s) dctrn_result GROUP BY %s ORDER BY %s',
implode(', ', array_merge($sqlIdentifier, array($mainRowNumberSelectPart))),
$innerSql, implode(',', $sqlIdentifier),
$mainRowNumberOrderPart);
} else {

/*
* MySQL does'nt reset the ordering of subselect even when the fields
* which are participated in order by statement, arent selected in
* main select statement.
*/
if ($this->platform->getName() == 'mysql') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instanceof

$selectFields = array_merge($sqlIdentifier);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This array_merge() call is not needed

} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid the else clause if not needed

$selectFields = array_merge($sqlIdentifier, $sqlOrderColumns);
}
$sql = sprintf(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Return directly

'SELECT DISTINCT %s FROM (%s) dctrn_result ORDER BY %s',
implode(', ', $selectFields),
$innerSql,
implode(', ', $orderBy)
);
}
}

return $sql;
Expand Down
14 changes: 7 additions & 7 deletions tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public function testLimitSubqueryWithSortPg()
$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 id_0, MIN(ROWNUM) AS MINROWNUM 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, ROW_NUMBER() OVER( ORDER BY m0_.title ASC) as ROWNUM 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 GROUP BY id_0 ORDER BY MINROWNUM ASC", $limitQuery->getSql()
);

$this->entityManager->getConnection()->setDatabasePlatform($odp);
Expand All @@ -51,7 +51,7 @@ public function testLimitSubqueryWithScalarSortPg()
$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 id_1, MIN(ROWNUM) AS MINROWNUM FROM (SELECT COUNT(g0_.id) AS sclr_0, u1_.id AS id_1, g0_.id AS id_2, ROW_NUMBER() OVER( ORDER BY sclr_0 ASC) as ROWNUM 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 GROUP BY id_1 ORDER BY MINROWNUM ASC",
$limitQuery->getSql()
);

Expand All @@ -70,7 +70,7 @@ public function testLimitSubqueryWithMixedSortPg()
$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 id_1, MIN(ROWNUM) AS MINROWNUM FROM (SELECT COUNT(g0_.id) AS sclr_0, u1_.id AS id_1, g0_.id AS id_2, ROW_NUMBER() OVER( ORDER BY sclr_0 ASC, u1_.id DESC) as ROWNUM 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 GROUP BY id_1 ORDER BY MINROWNUM ASC",
$limitQuery->getSql()
);

Expand All @@ -89,7 +89,7 @@ public function testLimitSubqueryWithHiddenScalarSortPg()
$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 id_1, MIN(ROWNUM) AS MINROWNUM FROM (SELECT COUNT(g0_.id) AS sclr_0, u1_.id AS id_1, g0_.id AS id_2, ROW_NUMBER() OVER( ORDER BY sclr_0 ASC, u1_.id DESC) as ROWNUM 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 GROUP BY id_1 ORDER BY MINROWNUM ASC",
$limitQuery->getSql()
);

Expand Down Expand Up @@ -118,7 +118,7 @@ public function testLimitSubqueryWithSortOracle()
$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 ID_0, MIN(ROWNUM) AS MINROWNUM 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, ROW_NUMBER() OVER( ORDER BY m0_.title ASC) as ROWNUM 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 GROUP BY ID_0 ORDER BY MINROWNUM ASC", $limitQuery->getSql()
);

$this->entityManager->getConnection()->setDatabasePlatform($odp);
Expand All @@ -137,7 +137,7 @@ public function testLimitSubqueryWithScalarSortOracle()
$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 ID_1, MIN(ROWNUM) AS MINROWNUM FROM (SELECT COUNT(g0_.id) AS SCLR_0, u1_.id AS ID_1, g0_.id AS ID_2, ROW_NUMBER() OVER( ORDER BY SCLR_0 ASC) as ROWNUM 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 GROUP BY ID_1 ORDER BY MINROWNUM ASC",
$limitQuery->getSql()
);

Expand All @@ -157,7 +157,7 @@ public function testLimitSubqueryWithMixedSortOracle()
$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 ID_1, MIN(ROWNUM) AS MINROWNUM FROM (SELECT COUNT(g0_.id) AS SCLR_0, u1_.id AS ID_1, g0_.id AS ID_2, ROW_NUMBER() OVER( ORDER BY SCLR_0 ASC, u1_.id DESC) as ROWNUM 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 GROUP BY ID_1 ORDER BY MINROWNUM ASC",
$limitQuery->getSql()
);

Expand Down