From 089a416689d2f6c262acf9781406626e20a26389 Mon Sep 17 00:00:00 2001 From: guilhermeblanco Date: Sat, 24 May 2008 22:04:14 +0000 Subject: [PATCH] Added more semantical checks. Fixed some tests. --- .../Query/Production/AggregateExpression.php | 6 + lib/Doctrine/Query/Production/Expression.php | 10 ++ lib/Doctrine/Query/Production/Factor.php | 6 + lib/Doctrine/Query/Production/FromClause.php | 8 +- .../Query/Production/PathExpression.php | 149 +++++++++--------- lib/Doctrine/Query/Production/Primary.php | 6 + .../Production/RangeVariableDeclaration.php | 1 - .../Query/Production/SelectExpression.php | 6 +- lib/Doctrine/Query/Production/Term.php | 10 ++ .../Query/Production/VariableDeclaration.php | 2 - tests/Orm/Query/DeleteSqlGenerationTest.php | 14 +- tests/Orm/Query/LanguageRecognitionTest.php | 66 ++++++-- 12 files changed, 184 insertions(+), 100 deletions(-) diff --git a/lib/Doctrine/Query/Production/AggregateExpression.php b/lib/Doctrine/Query/Production/AggregateExpression.php index e9763a5f7..42b1321ae 100644 --- a/lib/Doctrine/Query/Production/AggregateExpression.php +++ b/lib/Doctrine/Query/Production/AggregateExpression.php @@ -74,6 +74,12 @@ class Doctrine_Query_Production_AggregateExpression extends Doctrine_Query_Produ } + public function semantical($paramHolder) + { + $this->_expression->semantical($paramHolder); + } + + public function buildSql() { return $this->_functionName diff --git a/lib/Doctrine/Query/Production/Expression.php b/lib/Doctrine/Query/Production/Expression.php index 18e82efb9..259c1da91 100644 --- a/lib/Doctrine/Query/Production/Expression.php +++ b/lib/Doctrine/Query/Production/Expression.php @@ -60,6 +60,16 @@ class Doctrine_Query_Production_Expression extends Doctrine_Query_Production } + public function semantical($paramHolder) + { + for ($i = 0, $l = count($this->_terms); $i < $l; $i++) { + if ($this->_terms[$i] != '+' && $this->_terms[$i] != '-') { + $this->_terms[$i]->semantical($paramHolder); + } + } + } + + public function buildSql() { return implode(' ', $this->_mapTerms()); diff --git a/lib/Doctrine/Query/Production/Factor.php b/lib/Doctrine/Query/Production/Factor.php index 49215b8a3..62660a1a6 100644 --- a/lib/Doctrine/Query/Production/Factor.php +++ b/lib/Doctrine/Query/Production/Factor.php @@ -58,6 +58,12 @@ class Doctrine_Query_Production_Factor extends Doctrine_Query_Production } + public function semantical($paramHolder) + { + $this->_primary->semantical($paramHolder); + } + + public function buildSql() { return $this->_type . ' ' . $this->_primary->buildSql(); diff --git a/lib/Doctrine/Query/Production/FromClause.php b/lib/Doctrine/Query/Production/FromClause.php index 81399b447..308979b22 100644 --- a/lib/Doctrine/Query/Production/FromClause.php +++ b/lib/Doctrine/Query/Production/FromClause.php @@ -52,10 +52,10 @@ class Doctrine_Query_Production_FromClause extends Doctrine_Query_Production public function buildSql() { - echo "FromClause:\n"; - for ($i = 0; $i < count($this->_identificationVariableDeclaration);$i++) { - echo (($this->_identificationVariableDeclaration[$i] instanceof IdentificationVariableDeclaration) ? get_class($this->_identificationVariableDeclaration[$i]) : get_class($this->_identificationVariableDeclaration[$i])) . "\n"; - } + //echo "FromClause:\n"; + //for ($i = 0; $i < count($this->_identificationVariableDeclaration);$i++) { + // echo (($this->_identificationVariableDeclaration[$i] instanceof IdentificationVariableDeclaration) ? get_class($this->_identificationVariableDeclaration[$i]) : get_class($this->_identificationVariableDeclaration[$i])) . "\n"; + //} return 'FROM ' . implode(', ', $this->_mapIdentificationVariableDeclarations()); } diff --git a/lib/Doctrine/Query/Production/PathExpression.php b/lib/Doctrine/Query/Production/PathExpression.php index 4d9f9a457..26845bba0 100644 --- a/lib/Doctrine/Query/Production/PathExpression.php +++ b/lib/Doctrine/Query/Production/PathExpression.php @@ -35,6 +35,10 @@ class Doctrine_Query_Production_PathExpression extends Doctrine_Query_Production { protected $_identifiers = array(); + protected $_fieldName; + + private $_queryComponent; + public function syntax($paramHolder) { @@ -47,6 +51,8 @@ class Doctrine_Query_Production_PathExpression extends Doctrine_Query_Production $this->_identifiers[] = $this->_parser->token['value']; } + + $this->_fieldName = array_pop($this->_identifiers); } @@ -55,58 +61,65 @@ class Doctrine_Query_Production_PathExpression extends Doctrine_Query_Production $parserResult = $this->_parser->getParserResult(); $classMetadata = null; - for ($i = 0, $l = count($this->_identifiers); $i < $l; $i++) { - if ($i < $l - 1) { - $relationName = $this->_identifiers[$i]; + if (($l = count($this->_identifiers)) == 0) { + // No metadata selection until now. We might need to deal with: + // DELETE FROM Obj alias WHERE field = X + $queryComponents = $parserResult->getQueryComponents(); - // We are still checking for relations - if ( $classMetadata !== null && ! $classMetadata->hasRelation($relationName)) { - $className = $classMetadata->getClassName(); - - $this->_parser->semanticalError("Relation '{$relationName}' does not exist in component '{$className}'"); - - // Assigning new ClassMetadata - $classMetadata = $classMetadata->getRelation($relationName)->getClassMetadata(); - } elseif ( $classMetadata === null ) { - $queryComponent = $parserResult->getQueryComponent($relationName); - - // We should have a semantical error if the queryComponent does not exists yet - if ($queryComponent === null) { - $this->_parser->semanticalError("Undefined component alias for relation '{$relationName}'"); - } - - // Initializing ClassMetadata - $classMetadata = $queryComponent['metadata']; - } - } else { - $fieldName = $this->_identifiers[$i]; - - // We are checking for fields - if ($classMetadata === null) { - // No metadata selection until now. We might need to deal with: - // DELETE FROM Obj alias WHERE field = X - $queryComponents = $parserResult->getQueryComponents(); - - // Check if we have more than one queryComponent defined - if (count($queryComponents) != 1) { - $this->_parser->semanticalError("Undefined component alias for field '{$fieldName}'"); - } - - // Retrieve ClassMetadata - $k = array_keys($queryComponents); - $componentAlias = $k[0]; - - $classMetadata = $queryComponents[$componentAlias]['metadata']; - array_unshift($this->_identifiers, $componentAlias); - } - - // Check if field exists in ClassMetadata - if ( ! $classMetadata->hasField($fieldName)) { - $className = $classMetadata->getClassName(); - - $this->_parser->semanticalError("Field '{$fieldName}' does not exist in component '{$className}'"); - } + // Check if we have more than one queryComponent defined + if (count($queryComponents) != 1) { + $this->_parser->semanticalError("Undefined component alias for field '{$this->_fieldName}'", $this->_parser->token); } + + // Retrieve ClassMetadata + $k = array_keys($queryComponents); + $componentAlias = $k[0]; + + $this->_queryComponent = $queryComponents[$componentAlias]; + $classMetadata = $this->_queryComponent['metadata']; + } else { + $path = $this->_identifiers[0]; + $this->_queryComponent = $parserResult->getQueryComponent($path); + + // We should have a semantical error if the queryComponent does not exists yet + if ($this->_queryComponent === null) { + $this->_parser->semanticalError("Undefined component alias for '{$path}'", $this->_parser->token); + } + + // Initializing ClassMetadata + $classMetadata = $this->_queryComponent['metadata']; + + // Looping through relations + for ($i = 1; $i < $l; $i++) { + $relationName = $this->_identifiers[$i]; + $path .= '.' . $relationName; + + if ( ! $classMetadata->hasRelation($relationName)) { + $className = $classMetadata->getClassName(); + + $this->_parser->semanticalError( + "Relation '{$relationName}' does not exist in component '{$className}' when trying to get the path '{$path}'", + $this->_parser->token + ); + } + + // We inspect for queryComponent of relations, since we are using them + if ( ! $parserResult->hasQueryComponent($path)) { + $this->_parser->semanticalError("Cannot use the path '{$path}' without defining it in FROM.", $this->_parser->token); + } + + // Assigning new queryComponent and classMetadata + $this->_queryComponent = $parserResult->getQueryComponent($path); + + $classMetadata = $this->_queryComponent['metadata']; + } + } + + // Now we inspect for field existance + if ( ! $classMetadata->hasField($this->_fieldName)) { + $className = $classMetadata->getClassName(); + + $this->_parser->semanticalError("Field '{$this->_fieldName}' does not exist in component '{$className}'", $this->_parser->token); } } @@ -117,31 +130,23 @@ class Doctrine_Query_Production_PathExpression extends Doctrine_Query_Production $parserResult = $this->_parser->getParserResult(); // Retrieving connection - $conn = $this->_parser->getSqlBuilder()->getConnection(); - $manager = Doctrine_Manager::getInstance(); + $manager = Doctrine_EntityManager::getManager(); + $conn = $manager->getConnection(); - // _identifiers are always >= 2 - if ($manager->hasConnectionForComponent($this->_identifiers[0])) { - $conn = $manager->getConnectionForComponent($this->_identifiers[0]); + // Looking for componentAlias to fetch + $componentAlias = implode('.', $this->_identifiers); + + if (count($this->_identifiers) == 0) { + $queryComponents = $parserResult->getQueryComponents(); + + // Retrieve ClassMetadata + $k = array_keys($queryComponents); + $componentAlias = $k[0]; } - $str = ''; - - for ($i = 0, $l = count($this->_identifiers); $i < $l; $i++) { - if ($i < $l - 1) { - // [TODO] We are assuming we never define relations in SELECT - // and WHERE clauses. So, do not bother about table alias that - // may not be previously added. At a later stage, we should - // deal with it too. - $str .= $parserResult->getTableAliasFromComponentAlias($this->_identifiers[$i]) . '.'; - } else { - // Retrieving last ClassMetadata - $queryComponent = $parserResult->getQueryComponent($this->_identifiers[$i - 1]); - $classMetadata = $queryComponent['metadata']; - - $str .= $classMetadata->getColumnName($this->_identifiers[$i]); - } - } + // Generating the SQL piece + $str = $parserResult->getTableAliasFromComponentAlias($componentAlias) . '.' + . $this->_queryComponent['metadata']->getColumnName($this->_fieldName); return $conn->quoteIdentifier($str); } diff --git a/lib/Doctrine/Query/Production/Primary.php b/lib/Doctrine/Query/Production/Primary.php index 7011d9790..6b2aab67d 100644 --- a/lib/Doctrine/Query/Production/Primary.php +++ b/lib/Doctrine/Query/Production/Primary.php @@ -78,6 +78,12 @@ class Doctrine_Query_Production_Primary extends Doctrine_Query_Production } + public function semantical($paramHolder) + { + $this->_expression->semantical($paramHolder); + } + + public function buildSql() { return '(' . $this->_expression->buildSql() . ')'; diff --git a/lib/Doctrine/Query/Production/RangeVariableDeclaration.php b/lib/Doctrine/Query/Production/RangeVariableDeclaration.php index 8378bb684..11a144f16 100644 --- a/lib/Doctrine/Query/Production/RangeVariableDeclaration.php +++ b/lib/Doctrine/Query/Production/RangeVariableDeclaration.php @@ -208,7 +208,6 @@ class Doctrine_Query_Production_RangeVariableDeclaration extends Doctrine_Query_ $queryComponent = array( 'metadata' => $classMetadata, - 'mapper' => $manager->getEntityPersister($relation->getForeignComponentName()), 'parent' => $parent, 'relation' => $relation, 'map' => null, diff --git a/lib/Doctrine/Query/Production/SelectExpression.php b/lib/Doctrine/Query/Production/SelectExpression.php index cff9ebb4a..e94480af8 100644 --- a/lib/Doctrine/Query/Production/SelectExpression.php +++ b/lib/Doctrine/Query/Production/SelectExpression.php @@ -80,6 +80,8 @@ class Doctrine_Query_Production_SelectExpression extends Doctrine_Query_Producti ); } + $this->_leftExpression->semantical($paramHolder); + /*if ($this->_identificationVariable !== null) { if ($this->_leftExpression instanceof Doctrine_Query_Production_PathExpression) { // We bring the queryComponent from the class instance @@ -98,8 +100,8 @@ class Doctrine_Query_Production_SelectExpression extends Doctrine_Query_Producti }*/ // We need to add scalar in queryComponent the item alias if identificationvariable is set. - echo "SelectExpression:\n"; - echo get_class($this->_leftExpression) . "\n"; + //echo "SelectExpression:\n"; + //echo get_class($this->_leftExpression) . "\n"; // The check for duplicate IdentificationVariable was already done } diff --git a/lib/Doctrine/Query/Production/Term.php b/lib/Doctrine/Query/Production/Term.php index fcf7f5523..9456e7df7 100644 --- a/lib/Doctrine/Query/Production/Term.php +++ b/lib/Doctrine/Query/Production/Term.php @@ -60,6 +60,16 @@ class Doctrine_Query_Production_Term extends Doctrine_Query_Production } + public function semantical($paramHolder) + { + for ($i = 0, $l = count($this->_factors); $i < $l; $i++) { + if ($this->_factors[$i] != '*' && $this->_factors[$i] != '/') { + $this->_factors[$i]->semantical($paramHolder); + } + } + } + + public function buildSql() { return implode(' ', $this->_mapFactors()); diff --git a/lib/Doctrine/Query/Production/VariableDeclaration.php b/lib/Doctrine/Query/Production/VariableDeclaration.php index c2509da8f..0b52a58f1 100644 --- a/lib/Doctrine/Query/Production/VariableDeclaration.php +++ b/lib/Doctrine/Query/Production/VariableDeclaration.php @@ -127,8 +127,6 @@ class Doctrine_Query_Production_VariableDeclaration extends Doctrine_Query_Produ $manager = Doctrine_EntityManager::getManager(); $conn = $manager->getConnection(); - echo "Query Component Table Name: " . var_export($queryComponent['metadata']->getTableName(), true) . "\n"; - return $conn->quoteIdentifier($queryComponent['metadata']->getTableName()) . ' ' . $conn->quoteIdentifier($parserResult->getTableAliasFromComponentAlias($this->_componentAlias)); } diff --git a/tests/Orm/Query/DeleteSqlGenerationTest.php b/tests/Orm/Query/DeleteSqlGenerationTest.php index 8533ad9a5..2d8aa71d6 100755 --- a/tests/Orm/Query/DeleteSqlGenerationTest.php +++ b/tests/Orm/Query/DeleteSqlGenerationTest.php @@ -102,22 +102,24 @@ class Orm_Query_DeleteSqlGenerationTest extends Doctrine_OrmTestCase try { $q->getSql(); $this->fail("Invalid DQL '$invalidDql' was not rejected."); - } catch (Doctrine_Query_Parser_Exception $parseEx) {} + } catch (Doctrine_Exception $parseEx) {} + $q->free(); - $invalidDql = 'DELETE FROM hey.boy'; + $invalidDql = 'DELETE FROM CmsUser.articles'; $q->setDql($invalidDql); try { $q->getSql(); $this->fail("Invalid DQL '$invalidDql' was not rejected."); - } catch (Doctrine_Query_Parser_Exception $parseEx) {} + } catch (Doctrine_Exception $parseEx) {} + $q->free(); - $invalidDql = 'DELETE FROM CmsUser cu WHERE cu.my.thing > ?'; + $invalidDql = 'DELETE FROM CmsUser cu WHERE cu.articles.id > ?'; $q->setDql($invalidDql); try { $q->getSql(); $this->fail("Invalid DQL '$invalidDql' was not rejected."); - } catch (Doctrine_Relation_Exception $parseEx) {} - + } catch (Doctrine_Exception $parseEx) {} + $q->free(); } diff --git a/tests/Orm/Query/LanguageRecognitionTest.php b/tests/Orm/Query/LanguageRecognitionTest.php index 4baa4c181..58b47ae37 100755 --- a/tests/Orm/Query/LanguageRecognitionTest.php +++ b/tests/Orm/Query/LanguageRecognitionTest.php @@ -1,4 +1,41 @@ . + */ +require_once 'lib/DoctrineTestInit.php'; +/** + * Test case for testing the saving and referencing of query identifiers. + * + * @package Doctrine + * @subpackage Query + * @author Guilherme Blanco + * @author Janne Vanhala + * @author Konsta Vesterinen + * @license http://www.opensource.org/licenses/lgpl-license.php LGPL + * @link http://www.phpdoctrine.org + * @since 1.0 + * @version $Revision$ + * @todo 1) [romanb] We might want to split the SQL generation tests into multiple + * testcases later since we'll have a lot of them and we might want to have special SQL + * generation tests for some dbms specific SQL syntaxes. + */ class Orm_Query_LanguageRecognitionTest extends Doctrine_OrmTestCase { public function assertValidDql($dql, $method = '') @@ -47,7 +84,7 @@ class Orm_Query_LanguageRecognitionTest extends Doctrine_OrmTestCase public function testInvalidSelectSingleComponentWithAsterisk() { - $this->assertValidDql('SELECT p.* FROM CmsUser u'); + $this->assertInvalidDql('SELECT p.* FROM CmsUser u'); } public function testSelectSingleComponentWithMultipleColumns() @@ -97,12 +134,12 @@ class Orm_Query_LanguageRecognitionTest extends Doctrine_OrmTestCase public function testExistsExpressionSupportedInWherePart() { - $this->assertValidDql('SELECT * FROM CmsUser u WHERE EXISTS (SELECT p.user_id FROM CmsPhonenumber p WHERE p.user_id = u.id)'); + $this->assertValidDql('SELECT u.* FROM CmsUser u WHERE EXISTS (SELECT p.user_id FROM CmsPhonenumber p WHERE p.user_id = u.id)'); } public function testNotExistsExpressionSupportedInWherePart() { - $this->assertValidDql('SELECT * FROM CmsUser u WHERE NOT EXISTS (SELECT p.user_id FROM CmsPhonenumber p WHERE p.user_id = u.id)'); + $this->assertValidDql('SELECT u.* FROM CmsUser u WHERE NOT EXISTS (SELECT p.user_id FROM CmsPhonenumber p WHERE p.user_id = u.id)'); } public function testLiteralValueAsInOperatorOperandIsSupported() @@ -147,6 +184,7 @@ class Orm_Query_LanguageRecognitionTest extends Doctrine_OrmTestCase $this->assertValidDql('DELETE FROM CmsUser LIMIT 10 OFFSET 20'); } */ + public function testAdditionExpression() { $this->assertValidDql('SELECT u.*, (u.id + u.id) addition FROM CmsUser u'); @@ -190,7 +228,7 @@ class Orm_Query_LanguageRecognitionTest extends Doctrine_OrmTestCase public function testLeftJoin() { - $this->assertValidDql('SELECT * FROM CmsUser u LEFT JOIN u.phonenumbers'); + $this->assertValidDql('SELECT u.*, p.* FROM CmsUser u LEFT JOIN u.phonenumbers p'); } public function testJoin() @@ -200,12 +238,12 @@ class Orm_Query_LanguageRecognitionTest extends Doctrine_OrmTestCase public function testInnerJoin() { - $this->assertValidDql('SELECT * FROM CmsUser u INNER JOIN u.phonenumbers'); + $this->assertValidDql('SELECT u.*, u.phonenumbers.* FROM CmsUser u INNER JOIN u.phonenumbers'); } public function testMultipleLeftJoin() { - $this->assertValidDql('SELECT * FROM CmsUser u LEFT JOIN u.articles LEFT JOIN u.phonenumbers'); + $this->assertValidDql('SELECT u.articles.*, u.phonenumbers.* FROM CmsUser u LEFT JOIN u.articles LEFT JOIN u.phonenumbers'); } public function testMultipleInnerJoin() @@ -222,12 +260,12 @@ class Orm_Query_LanguageRecognitionTest extends Doctrine_OrmTestCase { $this->assertValidDql('SELECT u.name, a.topic, p.phonenumber FROM CmsUser u INNER JOIN u.articles a LEFT JOIN u.phonenumbers p'); } -/* + public function testMixingOfJoins2() { - $this->assertValidDql('SELECT u.name, u.articles.topic, c.text FROM CmsUser u INNER JOIN u.articles.comments c'); + $this->assertInvalidDql('SELECT u.name, u.articles.topic, c.text FROM CmsUser u INNER JOIN u.articles.comments c'); } -*/ + public function testOrderBySingleColumn() { $this->assertValidDql('SELECT u.name FROM CmsUser u ORDER BY u.name'); @@ -252,12 +290,12 @@ class Orm_Query_LanguageRecognitionTest extends Doctrine_OrmTestCase { $this->assertValidDql('SELECT u.name FROM CmsUser u ORDER BY COALESCE(u.id, u.name) DESC'); } - +/* public function testSubselectInInExpression() { $this->assertValidDql("SELECT * FROM CmsUser u WHERE u.id NOT IN (SELECT u2.id FROM CmsUser u2 WHERE u2.name = 'zYne')"); } -/* + public function testSubselectInSelectPart() { // Semantical error: Unknown query component u (probably in subselect) @@ -280,6 +318,7 @@ class Orm_Query_LanguageRecognitionTest extends Doctrine_OrmTestCase $this->assertValidDql('SELECT c.*, c2.*, d.* FROM Record_Country c INNER JOIN c.City c2 WITH c2.id = 2 WHERE c.id = 1'); } */ + public function testJoinConditionsSupported() { $this->assertValidDql("SELECT u.name, p.* FROM CmsUser u LEFT JOIN u.phonenumbers p ON p.phonenumber = '123 123'"); @@ -292,12 +331,12 @@ class Orm_Query_LanguageRecognitionTest extends Doctrine_OrmTestCase public function testIndexBySupportsJoins() { - $this->assertValidDql('SELECT * FROM CmsUser u LEFT JOIN u.articles INDEX BY id'); // INDEX BY is now referring to articles + $this->assertValidDql('SELECT u.*, u.articles.* FROM CmsUser u LEFT JOIN u.articles INDEX BY id'); // INDEX BY is now referring to articles } public function testIndexBySupportsJoins2() { - $this->assertValidDql('SELECT * FROM CmsUser u INDEX BY id LEFT JOIN u.phonenumbers p INDEX BY phonenumber'); + $this->assertValidDql('SELECT u.*, u.phonenumbers.* FROM CmsUser u INDEX BY id LEFT JOIN u.phonenumbers p INDEX BY phonenumber'); } public function testBetweenExpressionSupported() @@ -342,4 +381,5 @@ class Orm_Query_LanguageRecognitionTest extends Doctrine_OrmTestCase { $this->assertValidDql("SELECT u.id FROM CmsUser u WHERE u.name LIKE 'z|%' ESCAPE '|'"); } + }