Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed DDC-1236: GROUP BY now supports ResultVariable and Identificati…

…onVariable. Composite PK is also supported. If you are willing to group by an aggregate function or a function itself, just place it in SELECT expression then refer to it in the GROUP BY clause. If you are not willing to have the function being part of your resultset, just mark the column as HIDDEN and you are done.
  • Loading branch information...
commit 2642daa43851878688d01625f272ff5874cac7b2 1 parent 619a319
@guilhermeblanco guilhermeblanco authored
View
20 lib/Doctrine/ORM/Query/Parser.php
@@ -2,7 +2,7 @@
/*
* 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
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHARNTABILITY 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
@@ -1305,7 +1305,7 @@ public function UpdateItem()
}
/**
- * GroupByItem ::= IdentificationVariable | SingleValuedPathExpression
+ * GroupByItem ::= IdentificationVariable | ResultVariable | SingleValuedPathExpression
*
* @return string | \Doctrine\ORM\Query\AST\PathExpression
*/
@@ -1314,18 +1314,20 @@ public function GroupByItem()
// We need to check if we are in a IdentificationVariable or SingleValuedPathExpression
$glimpse = $this->_lexer->glimpse();
- if ($glimpse['type'] == Lexer::T_DOT) {
+ if ($glimpse['type'] === Lexer::T_DOT) {
return $this->SingleValuedPathExpression();
}
- $token = $this->_lexer->lookahead;
- $identVariable = $this->IdentificationVariable();
+ // Still need to decide between IdentificationVariable or ResultVariable
+ $lookaheadValue = $this->_lexer->lookahead['value'];
- if ( ! isset($this->_queryComponents[$identVariable])) {
- $this->semanticalError('Cannot group by undefined identification variable.');
+ if ( ! isset($this->_queryComponents[$lookaheadValue])) {
+ $this->semanticalError('Cannot group by undefined identification or result variable.');
}
- return $identVariable;
+ return (isset($this->_queryComponents[$lookaheadValue]['metadata']))
+ ? $this->IdentificationVariable()
+ : $this->ResultVariable();
}
/**
@@ -2633,7 +2635,7 @@ public function AggregateExpression()
$isDistinct = true;
}
- $pathExp = ($lookaheadType === Lexer::T_COUNT)
+ $pathExp = ($lookaheadType === Lexer::T_COUNT)
? $this->SingleValuedPathExpression()
: $this->SimpleArithmeticExpression();
View
76 lib/Doctrine/ORM/Query/SqlWalker.php
@@ -1014,6 +1014,8 @@ public function walkSelectExpression($selectExpression)
$sql .= $col . ' AS ' . $columnAlias;
+ $this->_scalarResultAliasMap[$resultAlias] = $columnAlias;
+
if ( ! $hidden) {
$this->_rsm->addScalarResult($columnAlias, $resultAlias);
$this->_scalarFields[$dqlAlias][$fieldName] = $columnAlias;
@@ -1103,6 +1105,8 @@ public function walkSelectExpression($selectExpression)
$sqlParts[] = $col . ' AS '. $columnAlias;
+ $this->_scalarResultAliasMap[$resultAlias][] = $columnAlias;
+
$this->_rsm->addFieldResult($dqlAlias, $columnAlias, $fieldName, $class->name);
}
@@ -1132,6 +1136,8 @@ public function walkSelectExpression($selectExpression)
$sqlParts[] = $col . ' AS ' . $columnAlias;
+ $this->_scalarResultAliasMap[$resultAlias][] = $columnAlias;
+
$this->_rsm->addFieldResult($dqlAlias, $columnAlias, $fieldName, $subClassName);
}
}
@@ -1319,25 +1325,7 @@ public function walkGroupByClause($groupByClause)
$sqlParts = array();
foreach ($groupByClause->groupByItems AS $groupByItem) {
- if ( ! is_string($groupByItem)) {
- $sqlParts[] = $this->walkGroupByItem($groupByItem);
-
- continue;
- }
-
- foreach ($this->_queryComponents[$groupByItem]['metadata']->fieldNames AS $field) {
- $item = new AST\PathExpression(AST\PathExpression::TYPE_STATE_FIELD, $groupByItem, $field);
- $item->type = AST\PathExpression::TYPE_STATE_FIELD;
- $sqlParts[] = $this->walkGroupByItem($item);
- }
-
- foreach ($this->_queryComponents[$groupByItem]['metadata']->associationMappings AS $mapping) {
- if ($mapping['isOwningSide'] && $mapping['type'] & ClassMetadataInfo::TO_ONE) {
- $item = new AST\PathExpression(AST\PathExpression::TYPE_SINGLE_VALUED_ASSOCIATION, $groupByItem, $mapping['fieldName']);
- $item->type = AST\PathExpression::TYPE_SINGLE_VALUED_ASSOCIATION;
- $sqlParts[] = $this->walkGroupByItem($item);
- }
- }
+ $sqlParts[] = $this->walkGroupByItem($groupByItem);
}
return ' GROUP BY ' . implode(', ', $sqlParts);
@@ -1349,9 +1337,38 @@ public function walkGroupByClause($groupByClause)
* @param GroupByItem
* @return string The SQL.
*/
- public function walkGroupByItem(AST\PathExpression $pathExpr)
+ public function walkGroupByItem($groupByItem)
{
- return $this->walkPathExpression($pathExpr);
+ // StateFieldPathExpression
+ if ( ! is_string($groupByItem)) {
+ return $this->walkPathExpression($groupByItem);
+ }
+
+ // ResultVariable
+ if (isset($this->_queryComponents[$groupByItem]['resultVariable'])) {
+ return $this->walkResultVariable($groupByItem);
+ }
+
+ // IdentificationVariable
+ $sqlParts = array();
+
+ foreach ($this->_queryComponents[$groupByItem]['metadata']->fieldNames AS $field) {
+ $item = new AST\PathExpression(AST\PathExpression::TYPE_STATE_FIELD, $groupByItem, $field);
+ $item->type = AST\PathExpression::TYPE_STATE_FIELD;
+
+ $sqlParts[] = $this->walkPathExpression($item);
+ }
+
+ foreach ($this->_queryComponents[$groupByItem]['metadata']->associationMappings AS $mapping) {
+ if ($mapping['isOwningSide'] && $mapping['type'] & ClassMetadataInfo::TO_ONE) {
+ $item = new AST\PathExpression(AST\PathExpression::TYPE_SINGLE_VALUED_ASSOCIATION, $groupByItem, $mapping['fieldName']);
+ $item->type = AST\PathExpression::TYPE_SINGLE_VALUED_ASSOCIATION;
+
+ $sqlParts[] = $this->walkPathExpression($item);
+ }
+ }
+
+ return implode(', ', $sqlParts);
}
/**
@@ -1997,4 +2014,21 @@ public function walkStringPrimary($stringPrimary)
? $this->_conn->quote($stringPrimary)
: $stringPrimary->dispatch($this);
}
+
+ /**
+ * Walks down a ResultVriable that represents an AST node, thereby generating the appropriate SQL.
+ *
+ * @param string $resultVariable
+ * @return string The SQL.
+ */
+ public function walkResultVariable($resultVariable)
+ {
+ $resultAlias = $this->_scalarResultAliasMap[$resultVariable];
+
+ if (is_array($resultAlias)) {
+ return implode(', ', $resultAlias);
+ }
+
+ return $resultAlias;
+ }
}
View
10 lib/Doctrine/ORM/Query/TreeWalker.php
@@ -168,7 +168,7 @@ function walkGroupByClause($groupByClause);
* @param GroupByItem
* @return string The SQL.
*/
- function walkGroupByItem(AST\PathExpression $pathExpr);
+ function walkGroupByItem($groupByItem);
/**
* Walks down an UpdateStatement AST node, thereby generating the appropriate SQL.
@@ -395,6 +395,14 @@ function walkSimpleArithmeticExpression($simpleArithmeticExpr);
function walkPathExpression($pathExpr);
/**
+ * Walks down an ResultVariable AST node, thereby generating the appropriate SQL.
+ *
+ * @param string $resultVariable
+ * @return string The SQL.
+ */
+ function walkResultVariable($resultVariable);
+
+ /**
* Gets an executor that can be used to execute the result of this walker.
*
* @return AbstractExecutor
View
22 lib/Doctrine/ORM/Query/TreeWalkerAdapter.php
@@ -24,7 +24,7 @@
/**
* An adapter implementation of the TreeWalker interface. The methods in this class
* are empty. This class exists as convenience for creating tree walkers.
- *
+ *
* @author Roman Borschel <roman@code-factory.org>
* @since 2.0
*/
@@ -33,7 +33,7 @@
private $_query;
private $_parserResult;
private $_queryComponents;
-
+
/**
* {@inheritdoc}
*/
@@ -71,7 +71,7 @@ protected function _getParserResult()
{
return $this->_parserResult;
}
-
+
/**
* Walks down a SelectStatement AST node, thereby generating the appropriate SQL.
*
@@ -202,7 +202,7 @@ public function walkGroupByClause($groupByClause) {}
* @param GroupByItem
* @return string The SQL.
*/
- public function walkGroupByItem(AST\PathExpression $pathExpr) {}
+ public function walkGroupByItem($groupByItem) {}
/**
* Walks down an UpdateStatement AST node, thereby generating the appropriate SQL.
@@ -291,7 +291,7 @@ public function walkConditionalPrimary($primary) {}
* @return string The SQL.
*/
public function walkExistsExpression($existsExpr) {}
-
+
/**
* Walks down a CollectionMemberExpression AST node, thereby generating the appropriate SQL.
*
@@ -427,10 +427,18 @@ public function walkSimpleArithmeticExpression($simpleArithmeticExpr) {}
* @return string The SQL.
*/
public function walkPathExpression($pathExpr) {}
-
+
+ /**
+ * Walks down an ResultVariable AST node, thereby generating the appropriate SQL.
+ *
+ * @param string $resultVariable
+ * @return string The SQL.
+ */
+ public function walkResultVariable($resultVariable) {}
+
/**
* Gets an executor that can be used to execute the result of this walker.
- *
+ *
* @return AbstractExecutor
*/
public function getExecutor($AST) {}
View
33 lib/Doctrine/ORM/Query/TreeWalkerChain.php
@@ -25,7 +25,7 @@
* Represents a chain of tree walkers that modify an AST and finally emit output.
* Only the last walker in the chain can emit output. Any previous walkers can modify
* the AST to influence the final output produced by the last walker.
- *
+ *
* @author Roman Borschel <roman@code-factory.org>
* @since 2.0
*/
@@ -39,7 +39,7 @@ class TreeWalkerChain implements TreeWalker
private $_parserResult;
/** The query components of the original query (the "symbol table") that was produced by the Parser. */
private $_queryComponents;
-
+
/**
* @inheritdoc
*/
@@ -49,17 +49,17 @@ public function __construct($query, $parserResult, array $queryComponents)
$this->_parserResult = $parserResult;
$this->_queryComponents = $queryComponents;
}
-
+
/**
* Adds a tree walker to the chain.
- *
+ *
* @param string $walkerClass The class of the walker to instantiate.
*/
public function addTreeWalker($walkerClass)
{
$this->_walkers[] = new $walkerClass($this->_query, $this->_parserResult, $this->_queryComponents);
}
-
+
/**
* Walks down a SelectStatement AST node, thereby generating the appropriate SQL.
*
@@ -270,10 +270,10 @@ public function walkGroupByClause($groupByClause)
* @param GroupByItem
* @return string The SQL.
*/
- public function walkGroupByItem(AST\PathExpression $pathExpr)
+ public function walkGroupByItem($groupByItem)
{
foreach ($this->_walkers as $walker) {
- $walker->walkGroupByItem($pathExpr);
+ $walker->walkGroupByItem($groupByItem);
}
}
@@ -419,7 +419,7 @@ public function walkExistsExpression($existsExpr)
$walker->walkExistsExpression($existsExpr);
}
}
-
+
/**
* Walks down a CollectionMemberExpression AST node, thereby generating the appropriate SQL.
*
@@ -640,10 +640,23 @@ public function walkPathExpression($pathExpr)
$walker->walkPathExpression($pathExpr);
}
}
-
+
+ /**
+ * Walks down an ResultVariable AST node, thereby generating the appropriate SQL.
+ *
+ * @param string $resultVariable
+ * @return string The SQL.
+ */
+ public function walkResultVariable($resultVariable)
+ {
+ foreach ($this->_walkers as $walker) {
+ $walker->walkResultVariable($resultVariable);
+ }
+ }
+
/**
* Gets an executor that can be used to execute the result of this walker.
- *
+ *
* @return AbstractExecutor
*/
public function getExecutor($AST)
View
49 tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php
@@ -40,7 +40,14 @@ public function assertSqlGeneration($dqlToBeTested, $sqlToBeConfirmed, array $qu
$query->setHint($name, $value);
}
- parent::assertEquals($sqlToBeConfirmed, $query->getSQL());
+ $sqlGenerated = $query->getSQL();
+
+ parent::assertEquals(
+ $sqlToBeConfirmed,
+ $sqlGenerated,
+ sprintf('"%s" is not equal of "%s"', $sqlGenerated, $sqlToBeConfirmed)
+ );
+
$query->free();
} catch (\Exception $e) {
$this->fail($e->getMessage() ."\n".$e->getTraceAsString());
@@ -1302,7 +1309,7 @@ public function testForeignKeyAsPrimaryKeySubselect()
"SELECT d0_.article_id AS article_id0, d0_.title AS title1 FROM DDC117Article d0_ WHERE EXISTS (SELECT d1_.source_id, d1_.target_id FROM DDC117Reference d1_ WHERE d1_.source_id = d0_.article_id)"
);
}
-
+
/**
* @group DDC-1474
*/
@@ -1312,13 +1319,13 @@ public function testSelectWithArithmeticExpressionBeforeField()
'SELECT - e.value AS value, e.id FROM ' . __NAMESPACE__ . '\DDC1474Entity e',
'SELECT -d0_.value AS sclr0, d0_.id AS id1 FROM DDC1474Entity d0_'
);
-
+
$this->assertSqlGeneration(
'SELECT e.id, + e.value AS value FROM ' . __NAMESPACE__ . '\DDC1474Entity e',
'SELECT d0_.id AS id0, +d0_.value AS sclr1 FROM DDC1474Entity d0_'
);
}
-
+
/**
* @group DDC-1430
*/
@@ -1328,13 +1335,35 @@ public function testGroupByAllFieldsWhenObjectHasForeignKeys()
'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u GROUP BY u',
'SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ GROUP BY c0_.id, c0_.status, c0_.username, c0_.name, c0_.email_id'
);
-
+
$this->assertSqlGeneration(
'SELECT e FROM Doctrine\Tests\Models\CMS\CmsEmployee e GROUP BY e',
'SELECT c0_.id AS id0, c0_.name AS name1 FROM cms_employees c0_ GROUP BY c0_.id, c0_.name, c0_.spouse_id'
);
}
+ /**
+ * @group DDC-1236
+ */
+ public function testGroupBySupportsResultVariable()
+ {
+ $this->assertSqlGeneration(
+ 'SELECT u, u.status AS st FROM Doctrine\Tests\Models\CMS\CmsUser u GROUP BY st',
+ 'SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3, c0_.status AS status4 FROM cms_users c0_ GROUP BY status4'
+ );
+ }
+
+ /**
+ * @group DDC-1236
+ */
+ public function testGroupBySupportsIdentificationVariable()
+ {
+ $this->assertSqlGeneration(
+ 'SELECT u AS user FROM Doctrine\Tests\Models\CMS\CmsUser u GROUP BY user',
+ 'SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ GROUP BY id0, status1, username2, name3'
+ );
+ }
+
public function testCustomTypeValueSql()
{
if (DBALType::hasType('negative_to_positive')) {
@@ -1441,19 +1470,19 @@ class DDC1474Entity
{
/**
- * @Id
+ * @Id
* @Column(type="integer")
* @GeneratedValue()
*/
protected $id;
/**
- * @column(type="float")
+ * @column(type="float")
*/
private $value;
/**
- * @param string $float
+ * @param string $float
*/
public function __construct($float)
{
@@ -1469,7 +1498,7 @@ public function getId()
}
/**
- * @return float
+ * @return float
*/
public function getValue()
{
@@ -1477,7 +1506,7 @@ public function getValue()
}
/**
- * @param float $value
+ * @param float $value
*/
public function setValue($value)
{
Please sign in to comment.
Something went wrong with that request. Please try again.