From 4dcd5a1286a65e6d9db2b0d34b706675908ecf6a Mon Sep 17 00:00:00 2001 From: "Fabio B. Silva" Date: Fri, 18 Jan 2013 23:18:50 -0200 Subject: [PATCH 1/3] Fix DDC-2234 --- lib/Doctrine/ORM/Query/Parser.php | 151 +++++++++++------- lib/Doctrine/ORM/Query/SqlWalker.php | 18 +-- .../ORM/Query/SelectSqlGenerationTest.php | 41 +++++ 3 files changed, 141 insertions(+), 69 deletions(-) diff --git a/lib/Doctrine/ORM/Query/Parser.php b/lib/Doctrine/ORM/Query/Parser.php index a978e498a..d6c0077fe 100644 --- a/lib/Doctrine/ORM/Query/Parser.php +++ b/lib/Doctrine/ORM/Query/Parser.php @@ -2400,76 +2400,92 @@ class Parser */ public function SimpleConditionalExpression() { - $token = $this->lexer->lookahead; + if ($this->lexer->isNextToken(Lexer::T_EXISTS)) { + return $this->ExistsExpression(); + } + + $token = $this->lexer->lookahead; + $peek = $this->lexer->glimpse(); + $lookahead = $token; if ($this->lexer->isNextToken(Lexer::T_NOT)) { $token = $this->lexer->glimpse(); } - if ($token['type'] === Lexer::T_EXISTS) { - return $this->ExistsExpression(); - } + if ($token['type'] === Lexer::T_IDENTIFIER || $token['type'] === Lexer::T_INPUT_PARAMETER || $this->isFunction()) { + // Peek beyond the matching closing paranthesis. + $beyond = $this->lexer->peek(); - $peek = $this->lexer->glimpse(); + switch ($peek['value']) { + case '(': + //Peeks beyond the matched closing parenthesis. + $token = $this->peekBeyondClosingParenthesis(false); - if ($token['type'] === Lexer::T_IDENTIFIER || $token['type'] === Lexer::T_INPUT_PARAMETER) { - if ($peek['value'] == '(') { - // Peek beyond the matching closing paranthesis ')' - $this->lexer->peek(); - $token = $this->peekBeyondClosingParenthesis(false); + if ($token['type'] === Lexer::T_NOT) { + $token = $this->lexer->peek(); + } - if ($token['type'] === Lexer::T_NOT) { - $token = $this->lexer->peek(); - } + if ($token['type'] === Lexer::T_IS) { + $lookahead = $this->lexer->peek(); + } + break; - $this->lexer->resetPeek(); - } else { - // Peek beyond the PathExpression (or InputParameter) - $peek = $this->lexer->peek(); + default: + // Peek beyond the PathExpression or InputParameter. + $token = $beyond; - while ($peek['value'] === '.') { - $this->lexer->peek(); - $peek = $this->lexer->peek(); - } + while ($token['value'] === '.') { + $this->lexer->peek(); - // Also peek beyond a NOT if there is one - if ($peek['type'] === Lexer::T_NOT) { - $peek = $this->lexer->peek(); - } + $token = $this->lexer->peek(); + } - $token = $peek; + // Also peek beyond a NOT if there is one. + if ($token['type'] === Lexer::T_NOT) { + $token = $this->lexer->peek(); + } - // We need to go even further in case of IS (differenciate between NULL and EMPTY) - $lookahead = $this->lexer->peek(); - - // Also peek beyond a NOT if there is one - if ($lookahead['type'] === Lexer::T_NOT) { + // We need to go even further in case of IS (differenciate between NULL and EMPTY) $lookahead = $this->lexer->peek(); - } - - $this->lexer->resetPeek(); } + + // Also peek beyond a NOT if there is one. + if ($lookahead['type'] === Lexer::T_NOT) { + $lookahead = $this->lexer->peek(); + } + + $this->lexer->resetPeek(); } - switch ($token['type']) { - case Lexer::T_BETWEEN: - return $this->BetweenExpression(); - case Lexer::T_LIKE: - return $this->LikeExpression(); - case Lexer::T_IN: - return $this->InExpression(); - case Lexer::T_INSTANCE: - return $this->InstanceOfExpression(); - case Lexer::T_IS: - if ($lookahead['type'] == Lexer::T_NULL) { - return $this->NullComparisonExpression(); - } - return $this->EmptyCollectionComparisonExpression(); - case Lexer::T_MEMBER: - return $this->CollectionMemberExpression(); - default: - return $this->ComparisonExpression(); + if ($token['type'] === Lexer::T_BETWEEN) { + return $this->BetweenExpression(); } + + if ($token['type'] === Lexer::T_LIKE) { + return $this->LikeExpression(); + } + + if ($token['type'] === Lexer::T_IN) { + return $this->InExpression(); + } + + if ($token['type'] === Lexer::T_INSTANCE) { + return $this->InstanceOfExpression(); + } + + if ($token['type'] === Lexer::T_MEMBER) { + return $this->CollectionMemberExpression(); + } + + if ($token['type'] === Lexer::T_IS && $lookahead['type'] === Lexer::T_NULL) { + return $this->NullComparisonExpression(); + } + + if ($token['type'] === Lexer::T_IS && $lookahead['type'] === Lexer::T_EMPTY) { + return $this->EmptyCollectionComparisonExpression(); + } + + return $this->ComparisonExpression(); } /** @@ -3085,24 +3101,43 @@ class Parser } /** - * NullComparisonExpression ::= (SingleValuedPathExpression | InputParameter) "IS" ["NOT"] "NULL" + * NullComparisonExpression ::= InputParameter | NullIfExpression | CoalesceExpression | SingleValuedPathExpression "IS" ["NOT"] "NULL" * * @return \Doctrine\ORM\Query\AST\NullComparisonExpression */ public function NullComparisonExpression() { - if ($this->lexer->isNextToken(Lexer::T_INPUT_PARAMETER)) { - $this->match(Lexer::T_INPUT_PARAMETER); - $expr = new AST\InputParameter($this->lexer->token['value']); - } else { - $expr = $this->SingleValuedPathExpression(); + switch (true) { + case $this->lexer->isNextToken(Lexer::T_INPUT_PARAMETER): + $this->match(Lexer::T_INPUT_PARAMETER); + + $expr = new AST\InputParameter($this->lexer->token['value']); + break; + + case $this->lexer->isNextToken(Lexer::T_NULLIF): + $expr = $this->NullIfExpression(); + break; + + case $this->lexer->isNextToken(Lexer::T_COALESCE): + $expr = $this->CoalesceExpression(); + break; + + case $this->isFunction(): + $expr = $this->FunctionDeclaration(); + break; + + default: + $expr = $this->SingleValuedPathExpression(); + break; } $nullCompExpr = new AST\NullComparisonExpression($expr); + $this->match(Lexer::T_IS); if ($this->lexer->isNextToken(Lexer::T_NOT)) { $this->match(Lexer::T_NOT); + $nullCompExpr->not = true; } diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 0e01d9c5b..62e436ee5 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -1874,20 +1874,16 @@ class SqlWalker implements TreeWalker */ public function walkNullComparisonExpression($nullCompExpr) { - $sql = ''; - $innerExpr = $nullCompExpr->expression; + $expression = $nullCompExpr->expression; + $comparison = ' IS' . ($nullCompExpr->not ? ' NOT' : '') . ' NULL'; - if ($innerExpr instanceof AST\InputParameter) { - $dqlParamKey = $innerExpr->name; - $this->parserResult->addParameterMapping($dqlParamKey, $this->sqlParamIndex++); - $sql .= ' ?'; - } else { - $sql .= $this->walkPathExpression($innerExpr); + if ($expression instanceof AST\InputParameter) { + $this->parserResult->addParameterMapping($expression->name, $this->sqlParamIndex++); + + return '?' . $comparison; } - $sql .= ' IS' . ($nullCompExpr->not ? ' NOT' : '') . ' NULL'; - - return $sql; + return $expression->dispatch($this) . $comparison; } /** diff --git a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php index ef52c4705..670a27fab 100644 --- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php @@ -1654,6 +1654,47 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase } + /** + * @group DDC-2234 + */ + public function testWhereFunctionIsNullComparisonExpression() + { + $this->assertSqlGeneration( + "SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE IDENTITY(u.email) IS NULL", + "SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ WHERE c0_.email_id IS NULL" + ); + + $this->assertSqlGeneration( + "SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE NULLIF(u.name, 'FabioBatSilva') IS NULL AND IDENTITY(u.email) IS NOT NULL", + "SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ WHERE NULLIF(c0_.name, 'FabioBatSilva') IS NULL AND c0_.email_id IS NOT NULL" + ); + + $this->assertSqlGeneration( + "SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE IDENTITY(u.email) IS NOT NULL", + "SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ WHERE c0_.email_id IS NOT NULL" + ); + + $this->assertSqlGeneration( + "SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE NULLIF(u.name, 'FabioBatSilva') IS NOT NULL", + "SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ WHERE NULLIF(c0_.name, 'FabioBatSilva') IS NOT NULL" + ); + + $this->assertSqlGeneration( + "SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE COALESCE(u.name, u.id) IS NOT NULL", + "SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ WHERE COALESCE(c0_.name, c0_.id) IS NOT NULL" + ); + + $this->assertSqlGeneration( + "SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE COALESCE(u.id, IDENTITY(u.email)) IS NOT NULL", + "SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ WHERE COALESCE(c0_.id, c0_.email_id) IS NOT NULL" + ); + + $this->assertSqlGeneration( + "SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE COALESCE(IDENTITY(u.email), NULLIF(u.name, 'FabioBatSilva')) IS NOT NULL", + "SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ WHERE COALESCE(c0_.email_id, NULLIF(c0_.name, 'FabioBatSilva')) IS NOT NULL" + ); + } + public function testCustomTypeValueSql() { if (DBALType::hasType('negative_to_positive')) { From 1d42a5385bc8fba378e0ac5ee19c6929cfdd77e4 Mon Sep 17 00:00:00 2001 From: "Fabio B. Silva" Date: Fri, 18 Jan 2013 23:47:31 -0200 Subject: [PATCH 2/3] test NOT EXISTS expression --- .../Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php index 670a27fab..b3543d329 100644 --- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php @@ -119,6 +119,14 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase ); } + public function testNotExistsExpression() + { + $this->assertSqlGeneration( + 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE NOT EXISTS (SELECT p.phonenumber FROM Doctrine\Tests\Models\CMS\CmsPhonenumber p WHERE p.phonenumber = 1234)', + 'SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ WHERE NOT EXISTS (SELECT c1_.phonenumber FROM cms_phonenumbers c1_ WHERE c1_.phonenumber = 1234)' + ); + } + public function testSupportsSelectForMultipleColumnsOfASingleComponent() { $this->assertSqlGeneration( From 93fba518a6dad66f212cf768fe793aa4463fb58b Mon Sep 17 00:00:00 2001 From: "Fabio B. Silva" Date: Sat, 19 Jan 2013 13:38:44 -0200 Subject: [PATCH 3/3] keep braces --- lib/Doctrine/ORM/Query/Parser.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Query/Parser.php b/lib/Doctrine/ORM/Query/Parser.php index d6c0077fe..460ebea46 100644 --- a/lib/Doctrine/ORM/Query/Parser.php +++ b/lib/Doctrine/ORM/Query/Parser.php @@ -3101,7 +3101,7 @@ class Parser } /** - * NullComparisonExpression ::= InputParameter | NullIfExpression | CoalesceExpression | SingleValuedPathExpression "IS" ["NOT"] "NULL" + * NullComparisonExpression ::= (InputParameter | NullIfExpression | CoalesceExpression | SingleValuedPathExpression) "IS" ["NOT"] "NULL" * * @return \Doctrine\ORM\Query\AST\NullComparisonExpression */