Skip to content

Commit

Permalink
Merge branch 'hotfix/#1342-paginator-functional-test-integration-take2'
Browse files Browse the repository at this point in the history
Close #1342
Close #1337
Close #1325
  • Loading branch information
Ocramius committed Mar 24, 2015
2 parents 06998d0 + d97d3ec commit dc99ed2
Show file tree
Hide file tree
Showing 7 changed files with 821 additions and 89 deletions.
244 changes: 210 additions & 34 deletions lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,15 @@

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;
use Doctrine\ORM\Query\AST\SelectExpression;
use Doctrine\ORM\Query\Expr\OrderBy;
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.
Expand Down Expand Up @@ -56,6 +61,23 @@ class LimitSubqueryOutputWalker extends SqlWalker
*/
private $maxResults;

/**
* @var \Doctrine\ORM\EntityManager
*/
private $em;

/**
* @var array
*/
private $orderByPathExpressions = [];

/**
* The quote strategy.
*
* @var \Doctrine\ORM\Mapping\QuoteStrategy
*/
private $quoteStrategy;

/**
* Constructor.
*
Expand All @@ -78,20 +100,37 @@ public function __construct($query, $parserResult, array $queryComponents)
$this->maxResults = $query->getMaxResults();
$query->setFirstResult(null)->setMaxResults(null);

$this->em = $query->getEntityManager();
$this->quoteStrategy = $this->em->getConfiguration()->getQuoteStrategy();

parent::__construct($query, $parserResult, $queryComponents);
}

/**
* 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)
{
// 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
$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
Expand All @@ -104,6 +143,9 @@ public function walkSelectStatement(SelectStatement $AST)

$innerSql = parent::walkSelectStatement($AST);

// Restore orderByClause
$AST->orderByClause = $orderByClause;

// Restore hiddens
foreach ($AST->selectClause->selectExpressions as $idx => $expr) {
$expr->hiddenAliasResultVariable = $hiddens[$idx];
Expand Down Expand Up @@ -163,7 +205,7 @@ public function walkSelectStatement(SelectStatement $AST)
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(
Expand All @@ -182,49 +224,183 @@ public function walkSelectStatement(SelectStatement $AST)
}

/**
* Generates new SQL for Postgresql or Oracle if necessary.
* 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->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 = [];
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 in the query.
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
*
* @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)
private 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;

$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
$sqlOrderColumns = array_diff($sqlOrderColumns, $sqlIdentifier);
// If the sql statement has an order by clause, we need to wrap it in a new select distinct
// statement
if (! $orderByClause instanceof OrderByClause) {
return $sql;
}

if (count($orderBy)) {
$sql = sprintf(
'SELECT DISTINCT %s FROM (%s) dctrn_result ORDER BY %s',
implode(', ', array_merge($sqlIdentifier, $sqlOrderColumns)),
$innerSql,
implode(', ', $orderBy)
// 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;
}

/**
* Generates a new order by clause that works in the scope of a select query wrapping the original
*
* @param OrderByClause $orderByClause
* @return array
*/
private function rebuildOrderByClauseForOuterScope(OrderByClause $orderByClause)
{
$dqlAliasToSqlTableAliasMap
= $searchPatterns
= $replacements
= $dqlAliasToClassMap
= $selectListAdditions
= $orderByItems
= [];

// 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;
}

return $sql;
$complexAddedOrderByAliases = 0;
foreach($orderByClause->orderByItems as $orderByItem) {
// Walk order by item to get string representation of it
$orderByItemString = $this->walkOrderByItem($orderByItem);

// Replace path expressions in the order by clause with their column alias
$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.
$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);
}

/**
* {@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;
}
}
30 changes: 30 additions & 0 deletions tests/Doctrine/Tests/Models/Pagination/Company.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php
namespace Doctrine\Tests\Models\Pagination;

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

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

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

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

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

/**
* @Column(type="integer")
*/
public $image_height;

/**
* @Column(type="integer")
*/
public $image_width;

/**
* @OneToOne(targetEntity="Company", inversedBy="logo", cascade={"persist"})
* @JoinColumn(name="company_id")
*/
public $company;
}
Loading

0 comments on commit dc99ed2

Please sign in to comment.