From 914c400a7d1446f4bcfc23113516a2299660352a Mon Sep 17 00:00:00 2001 From: Mathew Davies Date: Thu, 8 Jun 2017 17:22:07 +0100 Subject: [PATCH 1/9] Check for custom functions first. --- lib/Doctrine/ORM/Query/Parser.php | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ORM/Query/Parser.php b/lib/Doctrine/ORM/Query/Parser.php index 34679ff61..c8294be36 100644 --- a/lib/Doctrine/ORM/Query/Parser.php +++ b/lib/Doctrine/ORM/Query/Parser.php @@ -3376,8 +3376,13 @@ class Parser $token = $this->lexer->lookahead; $funcName = strtolower($token['value']); - // Check for built-in functions first! + $customFunctionDeclaration = $this->CustomFunctionDeclaration(); + + // Check for custom functions functions first! switch (true) { + case $customFunctionDeclaration !== null: + return $customFunctionDeclaration; + case (isset(self::$_STRING_FUNCTIONS[$funcName])): return $this->FunctionsReturningStrings(); @@ -3388,7 +3393,7 @@ class Parser return $this->FunctionsReturningDatetime(); default: - return $this->CustomFunctionDeclaration(); + $this->syntaxError('known function', $token); } } @@ -3416,7 +3421,7 @@ class Parser return $this->CustomFunctionsReturningDatetime(); default: - $this->syntaxError('known function', $token); + return null; } } From e4ff7a35a8fd82e6287afda67ac6b1499f705af0 Mon Sep 17 00:00:00 2001 From: Mathew Davies Date: Fri, 9 Jun 2017 10:00:07 +0100 Subject: [PATCH 2/9] Write a test case for a custom function override. --- lib/Doctrine/ORM/Query/Parser.php | 2 +- .../ORM/Functional/CustomFunctionsTest.php | 38 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Query/Parser.php b/lib/Doctrine/ORM/Query/Parser.php index c8294be36..3a917f059 100644 --- a/lib/Doctrine/ORM/Query/Parser.php +++ b/lib/Doctrine/ORM/Query/Parser.php @@ -3377,7 +3377,7 @@ class Parser $funcName = strtolower($token['value']); $customFunctionDeclaration = $this->CustomFunctionDeclaration(); - + // Check for custom functions functions first! switch (true) { case $customFunctionDeclaration !== null: diff --git a/tests/Doctrine/Tests/ORM/Functional/CustomFunctionsTest.php b/tests/Doctrine/Tests/ORM/Functional/CustomFunctionsTest.php index 63c3e4ee9..d7231219b 100644 --- a/tests/Doctrine/Tests/ORM/Functional/CustomFunctionsTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/CustomFunctionsTest.php @@ -47,6 +47,24 @@ class CustomFunctionsTest extends OrmFunctionalTestCase $this->assertEquals(1, count($users)); $this->assertSame($user, $users[0]); } + + public function testCustomFunctionOverride() + { + $user = new CmsUser(); + $user->name = 'Bob'; + $user->username = 'Dylan'; + $this->_em->persist($user); + $this->_em->flush(); + + $this->_em->getConfiguration()->addCustomStringFunction('COUNT', 'Doctrine\Tests\ORM\Functional\CustomCount'); + + $query = $this->_em->createQuery('SELECT COUNT(u.id) FROM Doctrine\Tests\Models\CMS\CmsUser u'); + + $users = $query->getResult(); + + $this->assertEquals(1, count($users)); + $this->assertSame($user, $users[0]); + } } class NoOp extends FunctionNode @@ -70,3 +88,23 @@ class NoOp extends FunctionNode } } +class CustomCount extends FunctionNode +{ + /** + * @var PathExpression + */ + private $field; + + public function parse(Parser $parser) + { + $parser->match(Lexer::T_IDENTIFIER); + $parser->match(Lexer::T_OPEN_PARENTHESIS); + $this->field = $parser->StringExpression(); + $parser->match(Lexer::T_CLOSE_PARENTHESIS); + } + + public function getSql(SqlWalker $sqlWalker) + { + return $this->field->dispatch($sqlWalker); + } +} From 866418e40f38e33f2c5dcf16ab9bccd3cdffa563 Mon Sep 17 00:00:00 2001 From: Mathew Davies Date: Fri, 9 Jun 2017 15:09:41 +0100 Subject: [PATCH 3/9] No longer treat aggregate functions as a special case. --- lib/Doctrine/ORM/Configuration.php | 12 ---- lib/Doctrine/ORM/ORMException.php | 10 ---- .../ORM/Query/AST/Functions/AvgFunction.php | 55 +++++++++++++++++++ .../ORM/Query/AST/Functions/CountFunction.php | 55 +++++++++++++++++++ .../ORM/Query/AST/Functions/MaxFunction.php | 55 +++++++++++++++++++ .../ORM/Query/AST/Functions/MinFunction.php | 55 +++++++++++++++++++ .../ORM/Query/AST/Functions/SumFunction.php | 55 +++++++++++++++++++ lib/Doctrine/ORM/Query/Parser.php | 45 +++------------ lib/Doctrine/ORM/Query/SqlWalker.php | 8 ++- .../Doctrine/Tests/ORM/ConfigurationTest.php | 6 -- .../ORM/Functional/CustomFunctionsTest.php | 18 +++--- 11 files changed, 295 insertions(+), 79 deletions(-) create mode 100644 lib/Doctrine/ORM/Query/AST/Functions/AvgFunction.php create mode 100644 lib/Doctrine/ORM/Query/AST/Functions/CountFunction.php create mode 100644 lib/Doctrine/ORM/Query/AST/Functions/MaxFunction.php create mode 100644 lib/Doctrine/ORM/Query/AST/Functions/MinFunction.php create mode 100644 lib/Doctrine/ORM/Query/AST/Functions/SumFunction.php diff --git a/lib/Doctrine/ORM/Configuration.php b/lib/Doctrine/ORM/Configuration.php index 104a4f4b2..4af482faf 100644 --- a/lib/Doctrine/ORM/Configuration.php +++ b/lib/Doctrine/ORM/Configuration.php @@ -425,10 +425,6 @@ class Configuration extends \Doctrine\DBAL\Configuration */ public function addCustomStringFunction($name, $className) { - if (Query\Parser::isInternalFunction($name)) { - throw ORMException::overwriteInternalDQLFunctionNotAllowed($name); - } - $this->_attributes['customStringFunctions'][strtolower($name)] = $className; } @@ -483,10 +479,6 @@ class Configuration extends \Doctrine\DBAL\Configuration */ public function addCustomNumericFunction($name, $className) { - if (Query\Parser::isInternalFunction($name)) { - throw ORMException::overwriteInternalDQLFunctionNotAllowed($name); - } - $this->_attributes['customNumericFunctions'][strtolower($name)] = $className; } @@ -541,10 +533,6 @@ class Configuration extends \Doctrine\DBAL\Configuration */ public function addCustomDatetimeFunction($name, $className) { - if (Query\Parser::isInternalFunction($name)) { - throw ORMException::overwriteInternalDQLFunctionNotAllowed($name); - } - $this->_attributes['customDatetimeFunctions'][strtolower($name)] = $className; } diff --git a/lib/Doctrine/ORM/ORMException.php b/lib/Doctrine/ORM/ORMException.php index 039ef2eaa..3ce9ce9dc 100644 --- a/lib/Doctrine/ORM/ORMException.php +++ b/lib/Doctrine/ORM/ORMException.php @@ -323,16 +323,6 @@ class ORMException extends Exception ); } - /** - * @param string $functionName - * - * @return ORMException - */ - public static function overwriteInternalDQLFunctionNotAllowed($functionName) - { - return new self("It is not allowed to overwrite internal function '$functionName' in the DQL parser through user-defined functions."); - } - /** * @return ORMException */ diff --git a/lib/Doctrine/ORM/Query/AST/Functions/AvgFunction.php b/lib/Doctrine/ORM/Query/AST/Functions/AvgFunction.php new file mode 100644 index 000000000..7d9703d0d --- /dev/null +++ b/lib/Doctrine/ORM/Query/AST/Functions/AvgFunction.php @@ -0,0 +1,55 @@ +. + */ + +namespace Doctrine\ORM\Query\AST\Functions; + +use Doctrine\ORM\Query\Parser; +use Doctrine\ORM\Query\SqlWalker; +use Doctrine\ORM\Query\AST\AggregateExpression; + +/** + * "AVG" "(" ["DISTINCT"] StringPrimary ")" + * + * @link www.doctrine-project.org + * @since 2.0 + * @author Mathew Davies + */ +class AvgFunction extends FunctionNode +{ + /** + * @var AggregateExpression + */ + public $aggregateExpression; + + /** + * @inheritDoc + */ + public function getSql(SqlWalker $sqlWalker) + { + return $this->aggregateExpression->dispatch($sqlWalker); + } + + /** + * @inheritDoc + */ + public function parse(Parser $parser) + { + $this->aggregateExpression = $parser->AggregateExpression(); + } +} diff --git a/lib/Doctrine/ORM/Query/AST/Functions/CountFunction.php b/lib/Doctrine/ORM/Query/AST/Functions/CountFunction.php new file mode 100644 index 000000000..e5b7220ed --- /dev/null +++ b/lib/Doctrine/ORM/Query/AST/Functions/CountFunction.php @@ -0,0 +1,55 @@ +. + */ + +namespace Doctrine\ORM\Query\AST\Functions; + +use Doctrine\ORM\Query\Parser; +use Doctrine\ORM\Query\SqlWalker; +use Doctrine\ORM\Query\AST\AggregateExpression; + +/** + * "COUNT" "(" ["DISTINCT"] StringPrimary ")" + * + * @link www.doctrine-project.org + * @since 2.0 + * @author Mathew Davies + */ +class CountFunction extends FunctionNode +{ + /** + * @var AggregateExpression + */ + public $aggregateExpression; + + /** + * @inheritDoc + */ + public function getSql(SqlWalker $sqlWalker) + { + return $this->aggregateExpression->dispatch($sqlWalker); + } + + /** + * @inheritDoc + */ + public function parse(Parser $parser) + { + $this->aggregateExpression = $parser->AggregateExpression(); + } +} diff --git a/lib/Doctrine/ORM/Query/AST/Functions/MaxFunction.php b/lib/Doctrine/ORM/Query/AST/Functions/MaxFunction.php new file mode 100644 index 000000000..c01cc2d86 --- /dev/null +++ b/lib/Doctrine/ORM/Query/AST/Functions/MaxFunction.php @@ -0,0 +1,55 @@ +. + */ + +namespace Doctrine\ORM\Query\AST\Functions; + +use Doctrine\ORM\Query\Parser; +use Doctrine\ORM\Query\SqlWalker; +use Doctrine\ORM\Query\AST\AggregateExpression; + +/** + * "MAX" "(" ["DISTINCT"] StringPrimary ")" + * + * @link www.doctrine-project.org + * @since 2.0 + * @author Mathew Davies + */ +class MaxFunction extends FunctionNode +{ + /** + * @var AggregateExpression + */ + public $aggregateExpression; + + /** + * @inheritDoc + */ + public function getSql(SqlWalker $sqlWalker) + { + return $this->aggregateExpression->dispatch($sqlWalker); + } + + /** + * @inheritDoc + */ + public function parse(Parser $parser) + { + $this->aggregateExpression = $parser->AggregateExpression(); + } +} diff --git a/lib/Doctrine/ORM/Query/AST/Functions/MinFunction.php b/lib/Doctrine/ORM/Query/AST/Functions/MinFunction.php new file mode 100644 index 000000000..1985be2d8 --- /dev/null +++ b/lib/Doctrine/ORM/Query/AST/Functions/MinFunction.php @@ -0,0 +1,55 @@ +. + */ + +namespace Doctrine\ORM\Query\AST\Functions; + +use Doctrine\ORM\Query\Parser; +use Doctrine\ORM\Query\SqlWalker; +use Doctrine\ORM\Query\AST\AggregateExpression; + +/** + * "MIN" "(" ["DISTINCT"] StringPrimary ")" + * + * @link www.doctrine-project.org + * @since 2.0 + * @author Mathew Davies + */ +class MinFunction extends FunctionNode +{ + /** + * @var AggregateExpression + */ + public $aggregateExpression; + + /** + * @inheritDoc + */ + public function getSql(SqlWalker $sqlWalker) + { + return $this->aggregateExpression->dispatch($sqlWalker); + } + + /** + * @inheritDoc + */ + public function parse(Parser $parser) + { + $this->aggregateExpression = $parser->AggregateExpression(); + } +} diff --git a/lib/Doctrine/ORM/Query/AST/Functions/SumFunction.php b/lib/Doctrine/ORM/Query/AST/Functions/SumFunction.php new file mode 100644 index 000000000..f8cd48db0 --- /dev/null +++ b/lib/Doctrine/ORM/Query/AST/Functions/SumFunction.php @@ -0,0 +1,55 @@ +. + */ + +namespace Doctrine\ORM\Query\AST\Functions; + +use Doctrine\ORM\Query\Parser; +use Doctrine\ORM\Query\SqlWalker; +use Doctrine\ORM\Query\AST\AggregateExpression; + +/** + * "SUM" "(" ["DISTINCT"] StringPrimary ")" + * + * @link www.doctrine-project.org + * @since 2.0 + * @author Mathew Davies + */ +class SumFunction extends FunctionNode +{ + /** + * @var AggregateExpression + */ + public $aggregateExpression; + + /** + * @inheritDoc + */ + public function getSql(SqlWalker $sqlWalker) + { + return $this->aggregateExpression->dispatch($sqlWalker); + } + + /** + * @inheritDoc + */ + public function parse(Parser $parser) + { + $this->aggregateExpression = $parser->AggregateExpression(); + } +} diff --git a/lib/Doctrine/ORM/Query/Parser.php b/lib/Doctrine/ORM/Query/Parser.php index 3a917f059..a06cd83a6 100644 --- a/lib/Doctrine/ORM/Query/Parser.php +++ b/lib/Doctrine/ORM/Query/Parser.php @@ -65,6 +65,13 @@ class Parser 'date_diff' => Functions\DateDiffFunction::class, 'bit_and' => Functions\BitAndFunction::class, 'bit_or' => Functions\BitOrFunction::class, + + // Aggregate functions + 'min' => Functions\MinFunction::class, + 'max' => Functions\MaxFunction::class, + 'avg' => Functions\AvgFunction::class, + 'sum' => Functions\SumFunction::class, + 'count' => Functions\CountFunction::class, ]; /** @@ -171,23 +178,6 @@ class Parser */ private $identVariableExpressions = []; - /** - * Checks if a function is internally defined. Used to prevent overwriting - * of built-in functions through user-defined functions. - * - * @param string $functionName - * - * @return bool - */ - static public function isInternalFunction($functionName) - { - $functionName = strtolower($functionName); - - return isset(self::$_STRING_FUNCTIONS[$functionName]) - || isset(self::$_DATETIME_FUNCTIONS[$functionName]) - || isset(self::$_NUMERIC_FUNCTIONS[$functionName]); - } - /** * Creates a new query parser object. * @@ -1978,9 +1968,6 @@ class Parser // SUM(u.id) + COUNT(u.id) return $this->SimpleArithmeticExpression(); - case ($this->isAggregateFunction($this->lexer->lookahead['type'])): - return $this->AggregateExpression(); - default: // IDENTITY(u) return $this->FunctionDeclaration(); @@ -2209,11 +2196,6 @@ class Parser $expression = $this->ScalarExpression(); break; - case ($this->isAggregateFunction($lookaheadType)): - // COUNT(u.id) - $expression = $this->AggregateExpression(); - break; - default: // IDENTITY(u) $expression = $this->FunctionDeclaration(); @@ -2858,10 +2840,6 @@ class Parser $peek = $this->lexer->glimpse(); if ($peek['value'] == '(') { - if ($this->isAggregateFunction($this->lexer->lookahead['type'])) { - return $this->AggregateExpression(); - } - return $this->FunctionDeclaration(); } @@ -2932,11 +2910,6 @@ class Parser case Lexer::T_COALESCE: case Lexer::T_NULLIF: return $this->CaseExpression(); - - default: - if ($this->isAggregateFunction($lookaheadType)) { - return $this->AggregateExpression(); - } } $this->syntaxError( @@ -3236,10 +3209,6 @@ class Parser $expr = $this->CoalesceExpression(); break; - case $this->isAggregateFunction($this->lexer->lookahead['type']): - $expr = $this->AggregateExpression(); - break; - case $this->isFunction(): $expr = $this->FunctionDeclaration(); break; diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index ee57cf90b..206a4676e 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -1579,10 +1579,14 @@ class SqlWalker implements TreeWalker $sql .= $this->walkPathExpression($expr); break; - case ($expr instanceof AST\AggregateExpression): + case ($expr instanceof AST\Functions\AvgFunction): + case ($expr instanceof AST\Functions\CountFunction): + case ($expr instanceof AST\Functions\MaxFunction): + case ($expr instanceof AST\Functions\MinFunction): + case ($expr instanceof AST\Functions\SumFunction): $alias = $simpleSelectExpression->fieldIdentificationVariable ?: $this->scalarResultCounter++; - $sql .= $this->walkAggregateExpression($expr) . ' AS dctrn__' . $alias; + $sql .= $expr->dispatch($this) . ' AS dctrn__' . $alias; break; case ($expr instanceof AST\Subselect): diff --git a/tests/Doctrine/Tests/ORM/ConfigurationTest.php b/tests/Doctrine/Tests/ORM/ConfigurationTest.php index ab657c5fd..92a2eeaea 100644 --- a/tests/Doctrine/Tests/ORM/ConfigurationTest.php +++ b/tests/Doctrine/Tests/ORM/ConfigurationTest.php @@ -265,8 +265,6 @@ class ConfigurationTest extends TestCase $this->assertSame(null, $this->configuration->getCustomStringFunction('NonExistingFunction')); $this->configuration->setCustomStringFunctions(['OtherFunctionName' => __CLASS__]); $this->assertSame(__CLASS__, $this->configuration->getCustomStringFunction('OtherFunctionName')); - $this->expectException(ORMException::class); - $this->configuration->addCustomStringFunction('concat', __CLASS__); } public function testAddGetCustomNumericFunction() @@ -276,8 +274,6 @@ class ConfigurationTest extends TestCase $this->assertSame(null, $this->configuration->getCustomNumericFunction('NonExistingFunction')); $this->configuration->setCustomNumericFunctions(['OtherFunctionName' => __CLASS__]); $this->assertSame(__CLASS__, $this->configuration->getCustomNumericFunction('OtherFunctionName')); - $this->expectException(ORMException::class); - $this->configuration->addCustomNumericFunction('abs', __CLASS__); } public function testAddGetCustomDatetimeFunction() @@ -287,8 +283,6 @@ class ConfigurationTest extends TestCase $this->assertSame(null, $this->configuration->getCustomDatetimeFunction('NonExistingFunction')); $this->configuration->setCustomDatetimeFunctions(['OtherFunctionName' => __CLASS__]); $this->assertSame(__CLASS__, $this->configuration->getCustomDatetimeFunction('OtherFunctionName')); - $this->expectException(ORMException::class); - $this->configuration->addCustomDatetimeFunction('date_add', __CLASS__); } public function testAddGetCustomHydrationMode() diff --git a/tests/Doctrine/Tests/ORM/Functional/CustomFunctionsTest.php b/tests/Doctrine/Tests/ORM/Functional/CustomFunctionsTest.php index d7231219b..aa56e1f93 100644 --- a/tests/Doctrine/Tests/ORM/Functional/CustomFunctionsTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/CustomFunctionsTest.php @@ -58,12 +58,11 @@ class CustomFunctionsTest extends OrmFunctionalTestCase $this->_em->getConfiguration()->addCustomStringFunction('COUNT', 'Doctrine\Tests\ORM\Functional\CustomCount'); - $query = $this->_em->createQuery('SELECT COUNT(u.id) FROM Doctrine\Tests\Models\CMS\CmsUser u'); + $query = $this->_em->createQuery('SELECT COUNT(DISTINCT u.id) FROM Doctrine\Tests\Models\CMS\CmsUser u'); - $users = $query->getResult(); + $usersCount = $query->getSingleScalarResult(); - $this->assertEquals(1, count($users)); - $this->assertSame($user, $users[0]); + $this->assertEquals(1, $usersCount); } } @@ -91,20 +90,17 @@ class NoOp extends FunctionNode class CustomCount extends FunctionNode { /** - * @var PathExpression + * @var Query\AST\AggregateExpression */ - private $field; + private $aggregateExpression; public function parse(Parser $parser) { - $parser->match(Lexer::T_IDENTIFIER); - $parser->match(Lexer::T_OPEN_PARENTHESIS); - $this->field = $parser->StringExpression(); - $parser->match(Lexer::T_CLOSE_PARENTHESIS); + $this->aggregateExpression = $parser->AggregateExpression(); } public function getSql(SqlWalker $sqlWalker) { - return $this->field->dispatch($sqlWalker); + return $this->aggregateExpression->dispatch($sqlWalker); } } From e4847534a4c2decb47947f7b32dced5f7ac3a5bf Mon Sep 17 00:00:00 2001 From: Mathew Davies Date: Mon, 19 Jun 2017 08:54:29 +0100 Subject: [PATCH 4/9] Remove @throws in Configuration where necessary. --- lib/Doctrine/ORM/Configuration.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/Doctrine/ORM/Configuration.php b/lib/Doctrine/ORM/Configuration.php index 4af482faf..6073fbe00 100644 --- a/lib/Doctrine/ORM/Configuration.php +++ b/lib/Doctrine/ORM/Configuration.php @@ -420,8 +420,6 @@ class Configuration extends \Doctrine\DBAL\Configuration * @param string|callable $className Class name or a callable that returns the function. * * @return void - * - * @throws ORMException */ public function addCustomStringFunction($name, $className) { @@ -474,8 +472,6 @@ class Configuration extends \Doctrine\DBAL\Configuration * @param string|callable $className Class name or a callable that returns the function. * * @return void - * - * @throws ORMException */ public function addCustomNumericFunction($name, $className) { @@ -528,8 +524,6 @@ class Configuration extends \Doctrine\DBAL\Configuration * @param string|callable $className Class name or a callable that returns the function. * * @return void - * - * @throws ORMException */ public function addCustomDatetimeFunction($name, $className) { From 966f9a84c26db0888bb86530264c18f8b062429e Mon Sep 17 00:00:00 2001 From: Mathew Davies Date: Mon, 19 Jun 2017 09:00:00 +0100 Subject: [PATCH 5/9] phpDoc and type hint changes. --- lib/Doctrine/ORM/Query/AST/Functions/AvgFunction.php | 9 ++++----- lib/Doctrine/ORM/Query/AST/Functions/CountFunction.php | 9 ++++----- lib/Doctrine/ORM/Query/AST/Functions/MaxFunction.php | 9 ++++----- lib/Doctrine/ORM/Query/AST/Functions/MinFunction.php | 9 ++++----- lib/Doctrine/ORM/Query/AST/Functions/SumFunction.php | 9 ++++----- .../Tests/ORM/Functional/CustomFunctionsTest.php | 4 ++-- 6 files changed, 22 insertions(+), 27 deletions(-) diff --git a/lib/Doctrine/ORM/Query/AST/Functions/AvgFunction.php b/lib/Doctrine/ORM/Query/AST/Functions/AvgFunction.php index 7d9703d0d..d217eb4b7 100644 --- a/lib/Doctrine/ORM/Query/AST/Functions/AvgFunction.php +++ b/lib/Doctrine/ORM/Query/AST/Functions/AvgFunction.php @@ -26,8 +26,7 @@ use Doctrine\ORM\Query\AST\AggregateExpression; /** * "AVG" "(" ["DISTINCT"] StringPrimary ")" * - * @link www.doctrine-project.org - * @since 2.0 + * @since 2.6 * @author Mathew Davies */ class AvgFunction extends FunctionNode @@ -35,12 +34,12 @@ class AvgFunction extends FunctionNode /** * @var AggregateExpression */ - public $aggregateExpression; + private $aggregateExpression; /** * @inheritDoc */ - public function getSql(SqlWalker $sqlWalker) + public function getSql(SqlWalker $sqlWalker): string { return $this->aggregateExpression->dispatch($sqlWalker); } @@ -48,7 +47,7 @@ class AvgFunction extends FunctionNode /** * @inheritDoc */ - public function parse(Parser $parser) + public function parse(Parser $parser): void { $this->aggregateExpression = $parser->AggregateExpression(); } diff --git a/lib/Doctrine/ORM/Query/AST/Functions/CountFunction.php b/lib/Doctrine/ORM/Query/AST/Functions/CountFunction.php index e5b7220ed..34a9a723c 100644 --- a/lib/Doctrine/ORM/Query/AST/Functions/CountFunction.php +++ b/lib/Doctrine/ORM/Query/AST/Functions/CountFunction.php @@ -26,8 +26,7 @@ use Doctrine\ORM\Query\AST\AggregateExpression; /** * "COUNT" "(" ["DISTINCT"] StringPrimary ")" * - * @link www.doctrine-project.org - * @since 2.0 + * @since 2.6 * @author Mathew Davies */ class CountFunction extends FunctionNode @@ -35,12 +34,12 @@ class CountFunction extends FunctionNode /** * @var AggregateExpression */ - public $aggregateExpression; + private $aggregateExpression; /** * @inheritDoc */ - public function getSql(SqlWalker $sqlWalker) + public function getSql(SqlWalker $sqlWalker): string { return $this->aggregateExpression->dispatch($sqlWalker); } @@ -48,7 +47,7 @@ class CountFunction extends FunctionNode /** * @inheritDoc */ - public function parse(Parser $parser) + public function parse(Parser $parser): void { $this->aggregateExpression = $parser->AggregateExpression(); } diff --git a/lib/Doctrine/ORM/Query/AST/Functions/MaxFunction.php b/lib/Doctrine/ORM/Query/AST/Functions/MaxFunction.php index c01cc2d86..d430cc428 100644 --- a/lib/Doctrine/ORM/Query/AST/Functions/MaxFunction.php +++ b/lib/Doctrine/ORM/Query/AST/Functions/MaxFunction.php @@ -26,8 +26,7 @@ use Doctrine\ORM\Query\AST\AggregateExpression; /** * "MAX" "(" ["DISTINCT"] StringPrimary ")" * - * @link www.doctrine-project.org - * @since 2.0 + * @since 2.6 * @author Mathew Davies */ class MaxFunction extends FunctionNode @@ -35,12 +34,12 @@ class MaxFunction extends FunctionNode /** * @var AggregateExpression */ - public $aggregateExpression; + private $aggregateExpression; /** * @inheritDoc */ - public function getSql(SqlWalker $sqlWalker) + public function getSql(SqlWalker $sqlWalker): string { return $this->aggregateExpression->dispatch($sqlWalker); } @@ -48,7 +47,7 @@ class MaxFunction extends FunctionNode /** * @inheritDoc */ - public function parse(Parser $parser) + public function parse(Parser $parser): void { $this->aggregateExpression = $parser->AggregateExpression(); } diff --git a/lib/Doctrine/ORM/Query/AST/Functions/MinFunction.php b/lib/Doctrine/ORM/Query/AST/Functions/MinFunction.php index 1985be2d8..345f99a94 100644 --- a/lib/Doctrine/ORM/Query/AST/Functions/MinFunction.php +++ b/lib/Doctrine/ORM/Query/AST/Functions/MinFunction.php @@ -26,8 +26,7 @@ use Doctrine\ORM\Query\AST\AggregateExpression; /** * "MIN" "(" ["DISTINCT"] StringPrimary ")" * - * @link www.doctrine-project.org - * @since 2.0 + * @since 2.6 * @author Mathew Davies */ class MinFunction extends FunctionNode @@ -35,12 +34,12 @@ class MinFunction extends FunctionNode /** * @var AggregateExpression */ - public $aggregateExpression; + private $aggregateExpression; /** * @inheritDoc */ - public function getSql(SqlWalker $sqlWalker) + public function getSql(SqlWalker $sqlWalker): string { return $this->aggregateExpression->dispatch($sqlWalker); } @@ -48,7 +47,7 @@ class MinFunction extends FunctionNode /** * @inheritDoc */ - public function parse(Parser $parser) + public function parse(Parser $parser): void { $this->aggregateExpression = $parser->AggregateExpression(); } diff --git a/lib/Doctrine/ORM/Query/AST/Functions/SumFunction.php b/lib/Doctrine/ORM/Query/AST/Functions/SumFunction.php index f8cd48db0..d77e284c0 100644 --- a/lib/Doctrine/ORM/Query/AST/Functions/SumFunction.php +++ b/lib/Doctrine/ORM/Query/AST/Functions/SumFunction.php @@ -26,8 +26,7 @@ use Doctrine\ORM\Query\AST\AggregateExpression; /** * "SUM" "(" ["DISTINCT"] StringPrimary ")" * - * @link www.doctrine-project.org - * @since 2.0 + * @since 2.6 * @author Mathew Davies */ class SumFunction extends FunctionNode @@ -35,12 +34,12 @@ class SumFunction extends FunctionNode /** * @var AggregateExpression */ - public $aggregateExpression; + private $aggregateExpression; /** * @inheritDoc */ - public function getSql(SqlWalker $sqlWalker) + public function getSql(SqlWalker $sqlWalker): string { return $this->aggregateExpression->dispatch($sqlWalker); } @@ -48,7 +47,7 @@ class SumFunction extends FunctionNode /** * @inheritDoc */ - public function parse(Parser $parser) + public function parse(Parser $parser): void { $this->aggregateExpression = $parser->AggregateExpression(); } diff --git a/tests/Doctrine/Tests/ORM/Functional/CustomFunctionsTest.php b/tests/Doctrine/Tests/ORM/Functional/CustomFunctionsTest.php index aa56e1f93..79668b8f2 100644 --- a/tests/Doctrine/Tests/ORM/Functional/CustomFunctionsTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/CustomFunctionsTest.php @@ -94,12 +94,12 @@ class CustomCount extends FunctionNode */ private $aggregateExpression; - public function parse(Parser $parser) + public function parse(Parser $parser): void { $this->aggregateExpression = $parser->AggregateExpression(); } - public function getSql(SqlWalker $sqlWalker) + public function getSql(SqlWalker $sqlWalker): string { return $this->aggregateExpression->dispatch($sqlWalker); } From 3e3751cfd9595fa1fdef491c53e8464992ebcd18 Mon Sep 17 00:00:00 2001 From: Mathew Davies Date: Mon, 19 Jun 2017 09:37:21 +0100 Subject: [PATCH 6/9] Remove aggregate function conditional in SqlWalker. --- lib/Doctrine/ORM/Query/SqlWalker.php | 10 ---------- .../Tests/ORM/Query/SelectSqlGenerationTest.php | 6 +++--- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 206a4676e..a4b5089e8 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -1579,16 +1579,6 @@ class SqlWalker implements TreeWalker $sql .= $this->walkPathExpression($expr); break; - case ($expr instanceof AST\Functions\AvgFunction): - case ($expr instanceof AST\Functions\CountFunction): - case ($expr instanceof AST\Functions\MaxFunction): - case ($expr instanceof AST\Functions\MinFunction): - case ($expr instanceof AST\Functions\SumFunction): - $alias = $simpleSelectExpression->fieldIdentificationVariable ?: $this->scalarResultCounter++; - - $sql .= $expr->dispatch($this) . ' AS dctrn__' . $alias; - break; - case ($expr instanceof AST\Subselect): $alias = $simpleSelectExpression->fieldIdentificationVariable ?: $this->scalarResultCounter++; diff --git a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php index b674c5d38..c10afbb89 100644 --- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php @@ -1031,7 +1031,7 @@ class SelectSqlGenerationTest extends OrmTestCase { $this->assertSqlGeneration( "SELECT u.name, (SELECT COUNT(p.phonenumber) FROM Doctrine\Tests\Models\CMS\CmsPhonenumber p WHERE p.phonenumber = 1234) pcount FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE u.name = 'jon'", - "SELECT c0_.name AS name_0, (SELECT COUNT(c1_.phonenumber) AS dctrn__1 FROM cms_phonenumbers c1_ WHERE c1_.phonenumber = 1234) AS sclr_1 FROM cms_users c0_ WHERE c0_.name = 'jon'" + "SELECT c0_.name AS name_0, (SELECT COUNT(c1_.phonenumber) AS sclr_2 FROM cms_phonenumbers c1_ WHERE c1_.phonenumber = 1234) AS sclr_1 FROM cms_users c0_ WHERE c0_.name = 'jon'" ); } @@ -1198,7 +1198,7 @@ class SelectSqlGenerationTest extends OrmTestCase { $this->assertSqlGeneration( "SELECT u.name, (SELECT COUNT(cfc.id) total FROM Doctrine\Tests\Models\Company\CompanyFixContract cfc) as cfc_count FROM Doctrine\Tests\Models\CMS\CmsUser u", - "SELECT c0_.name AS name_0, (SELECT COUNT(c1_.id) AS dctrn__total FROM company_contracts c1_ WHERE c1_.discr IN ('fix')) AS sclr_1 FROM cms_users c0_" + "SELECT c0_.name AS name_0, (SELECT COUNT(c1_.id) AS sclr_2 FROM company_contracts c1_ WHERE c1_.discr IN ('fix')) AS sclr_1 FROM cms_users c0_" ); } @@ -1750,7 +1750,7 @@ class SelectSqlGenerationTest extends OrmTestCase ); $this->assertSqlGeneration( 'SELECT u1 FROM Doctrine\Tests\Models\CMS\CmsUser u1 WHERE COUNT(u1.id) = ( SELECT SUM(u2.id) FROM Doctrine\Tests\Models\CMS\CmsUser u2 )', - 'SELECT c0_.id AS id_0, c0_.status AS status_1, c0_.username AS username_2, c0_.name AS name_3 FROM cms_users c0_ WHERE COUNT(c0_.id) = (SELECT SUM(c1_.id) AS dctrn__1 FROM cms_users c1_)' + 'SELECT c0_.id AS id_0, c0_.status AS status_1, c0_.username AS username_2, c0_.name AS name_3 FROM cms_users c0_ WHERE COUNT(c0_.id) = (SELECT SUM(c1_.id) AS sclr_4 FROM cms_users c1_)' ); $this->assertSqlGeneration( 'SELECT u1 FROM Doctrine\Tests\Models\CMS\CmsUser u1 WHERE COUNT(u1.id) <= ( SELECT SUM(u2.id) + COUNT(u2.email) FROM Doctrine\Tests\Models\CMS\CmsUser u2 )', From 49e4b1004c04e0ace55f3dae819b881b43cac6fe Mon Sep 17 00:00:00 2001 From: Mathew Davies Date: Mon, 19 Jun 2017 09:57:53 +0100 Subject: [PATCH 7/9] Update LimitSubqueryOutputWalkerTest --- .../Tools/Pagination/LimitSubqueryOutputWalkerTest.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php b/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php index 06e21173d..7e65ab7bc 100644 --- a/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php +++ b/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php @@ -330,7 +330,7 @@ ORDER BY b.id DESC' $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, LimitSubqueryOutputWalker::class); $this->assertEquals( - 'SELECT DISTINCT id_0 FROM (SELECT b0_.id AS id_0, b0_.author_id AS author_id_1, b0_.category_id AS category_id_2 FROM BlogPost b0_ WHERE ((SELECT COUNT(b1_.id) AS dctrn__1 FROM BlogPost b1_) = 1)) dctrn_result ORDER BY id_0 DESC', + 'SELECT DISTINCT id_0 FROM (SELECT b0_.id AS id_0, b0_.author_id AS author_id_1, b0_.category_id AS category_id_2 FROM BlogPost b0_ WHERE ((SELECT COUNT(b1_.id) AS sclr_3 FROM BlogPost b1_) = 1)) dctrn_result ORDER BY id_0 DESC', $query->getSQL() ); } @@ -346,7 +346,7 @@ ORDER BY b.id DESC' $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, LimitSubqueryOutputWalker::class); $this->assertEquals( - 'SELECT DISTINCT id_0, MIN(sclr_1) AS dctrn_minrownum FROM (SELECT b0_.id AS id_0, ROW_NUMBER() OVER(ORDER BY b0_.id DESC) AS sclr_1, b0_.author_id AS author_id_2, b0_.category_id AS category_id_3 FROM BlogPost b0_ WHERE ((SELECT COUNT(b1_.id) AS dctrn__1 FROM BlogPost b1_) = 1)) dctrn_result GROUP BY id_0 ORDER BY dctrn_minrownum ASC', + 'SELECT DISTINCT id_0, MIN(sclr_1) AS dctrn_minrownum FROM (SELECT b0_.id AS id_0, ROW_NUMBER() OVER(ORDER BY b0_.id DESC) AS sclr_1, b0_.author_id AS author_id_2, b0_.category_id AS category_id_3 FROM BlogPost b0_ WHERE ((SELECT COUNT(b1_.id) AS sclr_4 FROM BlogPost b1_) = 1)) dctrn_result GROUP BY id_0 ORDER BY dctrn_minrownum ASC', $query->getSQL() ); } @@ -390,7 +390,7 @@ ORDER BY b.id DESC' $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, LimitSubqueryOutputWalker::class); $this->assertEquals( - 'SELECT DISTINCT id_0 FROM (SELECT a0_.id AS id_0, a0_.name AS name_1, (SELECT MIN(m1_.title) AS dctrn__1 FROM MyBlogPost m1_ WHERE m1_.author_id = a0_.id) AS sclr_2 FROM Author a0_) dctrn_result ORDER BY sclr_2 DESC', + 'SELECT DISTINCT id_0 FROM (SELECT a0_.id AS id_0, a0_.name AS name_1, (SELECT MIN(m1_.title) AS sclr_3 FROM MyBlogPost m1_ WHERE m1_.author_id = a0_.id) AS sclr_2 FROM Author a0_) dctrn_result ORDER BY sclr_2 DESC', $query->getSQL() ); } @@ -415,7 +415,7 @@ ORDER BY b.id DESC' $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, LimitSubqueryOutputWalker::class); $this->assertEquals( - 'SELECT DISTINCT id_0, MIN(sclr_3) AS dctrn_minrownum FROM (SELECT a0_.id AS id_0, a0_.name AS name_1, (SELECT MIN(m1_.title) AS dctrn__1 FROM MyBlogPost m1_ WHERE m1_.author_id = a0_.id) AS sclr_2, ROW_NUMBER() OVER(ORDER BY (SELECT MIN(m1_.title) AS dctrn__2 FROM MyBlogPost m1_ WHERE m1_.author_id = a0_.id) DESC) AS sclr_3 FROM Author a0_) dctrn_result GROUP BY id_0 ORDER BY dctrn_minrownum ASC', + 'SELECT DISTINCT id_0, MIN(sclr_4) AS dctrn_minrownum FROM (SELECT a0_.id AS id_0, a0_.name AS name_1, (SELECT MIN(m1_.title) AS sclr_3 FROM MyBlogPost m1_ WHERE m1_.author_id = a0_.id) AS sclr_2, ROW_NUMBER() OVER(ORDER BY (SELECT MIN(m1_.title) AS sclr_5 FROM MyBlogPost m1_ WHERE m1_.author_id = a0_.id) DESC) AS sclr_4 FROM Author a0_) dctrn_result GROUP BY id_0 ORDER BY dctrn_minrownum ASC', $query->getSQL() ); } @@ -440,7 +440,7 @@ ORDER BY b.id DESC' $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, LimitSubqueryOutputWalker::class); $this->assertEquals( - 'SELECT DISTINCT ID_0, MIN(SCLR_3) AS dctrn_minrownum FROM (SELECT a0_.id AS ID_0, a0_.name AS NAME_1, (SELECT MIN(m1_.title) AS dctrn__1 FROM MyBlogPost m1_ WHERE m1_.author_id = a0_.id) AS SCLR_2, ROW_NUMBER() OVER(ORDER BY (SELECT MIN(m1_.title) AS dctrn__2 FROM MyBlogPost m1_ WHERE m1_.author_id = a0_.id) DESC) AS SCLR_3 FROM Author a0_) dctrn_result GROUP BY ID_0 ORDER BY dctrn_minrownum ASC', + 'SELECT DISTINCT ID_0, MIN(SCLR_4) AS dctrn_minrownum FROM (SELECT a0_.id AS ID_0, a0_.name AS NAME_1, (SELECT MIN(m1_.title) AS SCLR_3 FROM MyBlogPost m1_ WHERE m1_.author_id = a0_.id) AS SCLR_2, ROW_NUMBER() OVER(ORDER BY (SELECT MIN(m1_.title) AS SCLR_5 FROM MyBlogPost m1_ WHERE m1_.author_id = a0_.id) DESC) AS SCLR_4 FROM Author a0_) dctrn_result GROUP BY ID_0 ORDER BY dctrn_minrownum ASC', $query->getSQL() ); } From 05758f45647861f03cae4e0e9cf926354626e809 Mon Sep 17 00:00:00 2001 From: Mathew Davies Date: Mon, 19 Jun 2017 10:07:58 +0100 Subject: [PATCH 8/9] Documented changes in UPGRADE.md --- UPGRADE.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/UPGRADE.md b/UPGRADE.md index 3592fe5b6..8aba870c6 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,3 +1,17 @@ +# Upgrade to 2.6 + +## Minor BC BREAK: removed `Doctrine\ORM\Query\Parser#isInternalFunction` + +Method `Doctrine\ORM\Query\Parser#isInternalFunction` was removed because +the distinction between internal function and user defined DQL was removed. +[#6500](https://github.com/doctrine/doctrine2/pull/6500) + +## Minor BC BREAK: removed `Doctrine\ORM\ORMException#overwriteInternalDQLFunctionNotAllowed` + +Method `Doctrine\ORM\Query\Parser#overwriteInternalDQLFunctionNotAllowed` was +removed because of the choice to allow users to overwrite internal functions, ie +`AVG`, `SUM`, `COUNT`, `MIN` and `MAX`. [#6500](https://github.com/doctrine/doctrine2/pull/6500) + # Upgrade to 2.5 ## Minor BC BREAK: removed `Doctrine\ORM\Query\SqlWalker#walkCaseExpression()` From 747c1857d63fc86f76c730a1eb0f4f591835dbad Mon Sep 17 00:00:00 2001 From: Mathew Davies Date: Mon, 19 Jun 2017 11:11:42 +0100 Subject: [PATCH 9/9] Make the new aggregate function definitions final. --- lib/Doctrine/ORM/Query/AST/Functions/AvgFunction.php | 2 +- lib/Doctrine/ORM/Query/AST/Functions/CountFunction.php | 2 +- lib/Doctrine/ORM/Query/AST/Functions/MaxFunction.php | 2 +- lib/Doctrine/ORM/Query/AST/Functions/MinFunction.php | 2 +- lib/Doctrine/ORM/Query/AST/Functions/SumFunction.php | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/Doctrine/ORM/Query/AST/Functions/AvgFunction.php b/lib/Doctrine/ORM/Query/AST/Functions/AvgFunction.php index d217eb4b7..6cb8d92b6 100644 --- a/lib/Doctrine/ORM/Query/AST/Functions/AvgFunction.php +++ b/lib/Doctrine/ORM/Query/AST/Functions/AvgFunction.php @@ -29,7 +29,7 @@ use Doctrine\ORM\Query\AST\AggregateExpression; * @since 2.6 * @author Mathew Davies */ -class AvgFunction extends FunctionNode +final class AvgFunction extends FunctionNode { /** * @var AggregateExpression diff --git a/lib/Doctrine/ORM/Query/AST/Functions/CountFunction.php b/lib/Doctrine/ORM/Query/AST/Functions/CountFunction.php index 34a9a723c..f232cf43f 100644 --- a/lib/Doctrine/ORM/Query/AST/Functions/CountFunction.php +++ b/lib/Doctrine/ORM/Query/AST/Functions/CountFunction.php @@ -29,7 +29,7 @@ use Doctrine\ORM\Query\AST\AggregateExpression; * @since 2.6 * @author Mathew Davies */ -class CountFunction extends FunctionNode +final class CountFunction extends FunctionNode { /** * @var AggregateExpression diff --git a/lib/Doctrine/ORM/Query/AST/Functions/MaxFunction.php b/lib/Doctrine/ORM/Query/AST/Functions/MaxFunction.php index d430cc428..eba9b8639 100644 --- a/lib/Doctrine/ORM/Query/AST/Functions/MaxFunction.php +++ b/lib/Doctrine/ORM/Query/AST/Functions/MaxFunction.php @@ -29,7 +29,7 @@ use Doctrine\ORM\Query\AST\AggregateExpression; * @since 2.6 * @author Mathew Davies */ -class MaxFunction extends FunctionNode +final class MaxFunction extends FunctionNode { /** * @var AggregateExpression diff --git a/lib/Doctrine/ORM/Query/AST/Functions/MinFunction.php b/lib/Doctrine/ORM/Query/AST/Functions/MinFunction.php index 345f99a94..e1c086913 100644 --- a/lib/Doctrine/ORM/Query/AST/Functions/MinFunction.php +++ b/lib/Doctrine/ORM/Query/AST/Functions/MinFunction.php @@ -29,7 +29,7 @@ use Doctrine\ORM\Query\AST\AggregateExpression; * @since 2.6 * @author Mathew Davies */ -class MinFunction extends FunctionNode +final class MinFunction extends FunctionNode { /** * @var AggregateExpression diff --git a/lib/Doctrine/ORM/Query/AST/Functions/SumFunction.php b/lib/Doctrine/ORM/Query/AST/Functions/SumFunction.php index d77e284c0..c9fcf7b04 100644 --- a/lib/Doctrine/ORM/Query/AST/Functions/SumFunction.php +++ b/lib/Doctrine/ORM/Query/AST/Functions/SumFunction.php @@ -29,7 +29,7 @@ use Doctrine\ORM\Query\AST\AggregateExpression; * @since 2.6 * @author Mathew Davies */ -class SumFunction extends FunctionNode +final class SumFunction extends FunctionNode { /** * @var AggregateExpression