Skip to content

Commit

Permalink
Merge branch 'hotfix/#1353-#1347-#1351-fix-paginator-sorting-and-fetc…
Browse files Browse the repository at this point in the history
…h-joining'

Close #1353
  • Loading branch information
Ocramius committed Mar 31, 2015
2 parents b923c93 + ba00fc1 commit 82230cc
Show file tree
Hide file tree
Showing 10 changed files with 694 additions and 115 deletions.
376 changes: 278 additions & 98 deletions lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php

Large diffs are not rendered by default.

40 changes: 40 additions & 0 deletions lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryWalker.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
namespace Doctrine\ORM\Tools\Pagination;

use Doctrine\DBAL\Types\Type;
use Doctrine\ORM\Mapping\ClassMetadataInfo;
use Doctrine\ORM\ORMException;
use Doctrine\ORM\Query;
use Doctrine\ORM\Query\TreeWalkerAdapter;
use Doctrine\ORM\Query\AST\Functions\IdentityFunction;
use Doctrine\ORM\Query\AST\PathExpression;
Expand Down Expand Up @@ -68,6 +71,8 @@ public function walkSelectStatement(SelectStatement $AST)
$rootClass = $queryComponents[$rootAlias]['metadata'];
$selectExpressions = array();

$this->validate($AST);

foreach ($queryComponents as $dqlAlias => $qComp) {
// Preserve mixed data in query for ordering.
if (isset($qComp['resultVariable'])) {
Expand Down Expand Up @@ -112,6 +117,41 @@ public function walkSelectStatement(SelectStatement $AST)

$AST->selectClause->isDistinct = true;
}

/**
* Validate the AST to ensure that this walker is able to properly manipulate it.
*
* @param SelectStatement $AST
*/
private function validate(SelectStatement $AST)
{
// Prevent LimitSubqueryWalker from being used with queries that include
// a limit, a fetched to-many join, and an order by condition that
// references a column from the fetch joined table.
$queryComponents = $this->getQueryComponents();
$query = $this->_getQuery();
$from = $AST->fromClause->identificationVariableDeclarations;
$fromRoot = reset($from);

if ($query instanceof Query
&& $query->getMaxResults()
&& $AST->orderByClause
&& count($fromRoot->joins)) {
// Check each orderby item.
// TODO: check complex orderby items too...
foreach ($AST->orderByClause->orderByItems as $orderByItem) {
$expression = $orderByItem->expression;
if ($orderByItem->expression instanceof PathExpression
&& isset($queryComponents[$expression->identificationVariable])) {
$queryComponent = $queryComponents[$expression->identificationVariable];
if (isset($queryComponent['parent'])
&& $queryComponent['relation']['type'] & ClassMetadataInfo::TO_MANY) {
throw new \RuntimeException("Cannot select distinct identifiers from query with LIMIT and ORDER BY on a column from a fetch joined to-many association. Use output walkers.");
}
}
}
}
}

/**
* Retrieve either an IdentityFunction (IDENTITY(u.assoc)) or a state field (u.name).
Expand Down
61 changes: 61 additions & 0 deletions lib/Doctrine/ORM/Tools/Pagination/RowNumberOverFunction.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php

/*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*
* This software consists of voluntary contributions made by many individuals
* and is licensed under the MIT license. For more information, see
* <http://www.doctrine-project.org>.
*/

namespace Doctrine\ORM\Tools\Pagination;

use Doctrine\ORM\ORMException;
use Doctrine\ORM\Query\AST\Functions\FunctionNode;


/**
* RowNumberOverFunction
*
* Provides ROW_NUMBER() OVER(ORDER BY...) construct for use in LimitSubqueryOutputWalker
*
* @since 2.5
* @author Bill Schaller <[email protected]>
*/
class RowNumberOverFunction extends FunctionNode
{
/**
* @var \Doctrine\ORM\Query\AST\OrderByClause
*/
public $orderByClause;

/**
* @override
*/
public function getSql(\Doctrine\ORM\Query\SqlWalker $sqlWalker)
{
return 'ROW_NUMBER() OVER(' . trim($sqlWalker->walkOrderByClause(
$this->orderByClause
)) . ')';
}

/**
* @override
*
* @throws ORMException
*/
public function parse(\Doctrine\ORM\Query\Parser $parser)
{
throw new ORMException("The RowNumberOverFunction is not intended for, nor is it enabled for use in DQL.");
}
}
10 changes: 10 additions & 0 deletions tests/Doctrine/Tests/Models/Pagination/Company.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,18 @@ class Company
*/
public $name;

/**
* @Column(type="string", name="jurisdiction_code", nullable=true)
*/
public $jurisdiction;

/**
* @OneToOne(targetEntity="Logo", mappedBy="company", cascade={"persist"}, orphanRemoval=true)
*/
public $logo;

/**
* @OneToMany(targetEntity="Department", mappedBy="company", cascade={"persist"}, orphanRemoval=true)
*/
public $departments;
}
30 changes: 30 additions & 0 deletions tests/Doctrine/Tests/Models/Pagination/Department.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php
namespace Doctrine\Tests\Models\Pagination;

/**
* Department
*
* @package Doctrine\Tests\Models\Pagination
*
* @author Bill Schaller
* @Entity
* @Table(name="pagination_department")
*/
class Department
{
/**
* @Id @Column(type="integer")
* @GeneratedValue
*/
public $id;

/**
* @Column(type="string")
*/
public $name;

/**
* @ManyToOne(targetEntity="Company", inversedBy="departments", cascade={"persist"})
*/
public $company;
}
27 changes: 27 additions & 0 deletions tests/Doctrine/Tests/Models/Pagination/User.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

namespace Doctrine\Tests\Models\Pagination;


/**
* @package Doctrine\Tests\Models\Pagination
*
* @Entity
* @Table(name="pagination_user")
* @InheritanceType("SINGLE_TABLE")
* @DiscriminatorColumn(name="type", type="string")
* @DiscriminatorMap({"user1"="User1"})
*/
abstract class User
{
/**
* @Id @Column(type="integer")
* @GeneratedValue
*/
private $id;

/**
* @Column(type="string")
*/
public $name;
}
17 changes: 17 additions & 0 deletions tests/Doctrine/Tests/Models/Pagination/User1.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

namespace Doctrine\Tests\Models\Pagination;

/**
* Class User1
* @package Doctrine\Tests\Models\Pagination
*
* @Entity()
*/
class User1 extends User
{
/**
* @Column(type="string")
*/
public $email;
}
Loading

0 comments on commit 82230cc

Please sign in to comment.