From d7105552653bb514131aade43c2e8a2d0b77c4b6 Mon Sep 17 00:00:00 2001 From: Bill Schaller Date: Tue, 17 Mar 2015 18:32:57 -0400 Subject: [PATCH] Remove shameful hack in LimitSubqueryOutputWalker - replace with significantly less shameful hack --- .../Pagination/LimitSubqueryOutputWalker.php | 77 ++++++++++++++----- 1 file changed, 59 insertions(+), 18 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php index a61557b92..0616562c2 100644 --- a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php +++ b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php @@ -15,7 +15,9 @@ namespace Doctrine\ORM\Tools\Pagination; 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; @@ -62,6 +64,11 @@ class LimitSubqueryOutputWalker extends SqlWalker */ private $em; + /** + * @var array + */ + private $orderByPathExpressions = []; + /** * The quote strategy. * @@ -101,17 +108,21 @@ class LimitSubqueryOutputWalker extends SqlWalker * 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) { - // 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); + // 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 @@ -218,24 +229,32 @@ class LimitSubqueryOutputWalker extends SqlWalker */ private function addMissingItemsFromOrderByToSelect(SelectStatement $AST) { - // This block dumps the order by clause node using Node::dump(). - // It then finds all PathExpressions within and captures the - // identificationVariable and field name of each. - $orderByDump = (string)$AST->orderByClause; + $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 = []; - if (preg_match_all('/PathExpression\([^\)]+"identificationVariable": \'([^\']*)\'[^\)]+"field": \'([^\']*)\'[^\)]+\)/i', $orderByDump, $matches, PREG_SET_ORDER)) { - foreach($matches as $match) { - $idVar = $match[1]; - $field = $match[2]; - if (!isset($selects[$idVar])) { - $selects[$idVar] = []; - } - $selects[$idVar][$field] = true; + 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. + // that are already being selected in the query. foreach ($AST->selectClause->selectExpressions as $selectExpression) { if ($selectExpression instanceof SelectExpression) { $idVar = $selectExpression->expression; @@ -348,4 +367,26 @@ class LimitSubqueryOutputWalker extends SqlWalker 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; + } }