diff --git a/lib/Doctrine/Query/ParserResult.php b/lib/Doctrine/Query/ParserResult.php index d91293a7c..6f0ffcf78 100755 --- a/lib/Doctrine/Query/ParserResult.php +++ b/lib/Doctrine/Query/ParserResult.php @@ -86,7 +86,7 @@ class Doctrine_Query_ParserResult extends Doctrine_Query_AbstractResult * @param string $fieldAlias The field alias to set the declaration to. * @param string $queryField Alias declaration. */ - public function setQueryField($fieldAlias, array $queryField) + public function setQueryField($fieldAlias, $queryField) { $this->_queryFields[$fieldAlias] = $queryField; } @@ -99,7 +99,7 @@ class Doctrine_Query_ParserResult extends Doctrine_Query_AbstractResult */ public function getQueryFields() { - return $this->_queryComponents; + return $this->_queryFields; } diff --git a/lib/Doctrine/Query/Production.php b/lib/Doctrine/Query/Production.php index 5f7f20590..150159168 100644 --- a/lib/Doctrine/Query/Production.php +++ b/lib/Doctrine/Query/Production.php @@ -35,6 +35,18 @@ */ abstract class Doctrine_Query_Production { + /** + * @nodoc + */ + const SQLALIAS_SEPARATOR = '__'; + + + /** + * @nodoc + */ + const DEFAULT_QUERYCOMPONENT = 'dctrn'; + + /** * Parser object * diff --git a/lib/Doctrine/Query/Production/FieldIdentificationVariable.php b/lib/Doctrine/Query/Production/FieldIdentificationVariable.php index e7184005b..521aa7b0e 100755 --- a/lib/Doctrine/Query/Production/FieldIdentificationVariable.php +++ b/lib/Doctrine/Query/Production/FieldIdentificationVariable.php @@ -32,7 +32,9 @@ */ class Doctrine_Query_Production_FieldIdentificationVariable extends Doctrine_Query_Production { - protected $_componentAlias; + protected $_fieldAlias; + + protected $_columnAlias; public function syntax($paramHolder) @@ -49,14 +51,27 @@ class Doctrine_Query_Production_FieldIdentificationVariable extends Doctrine_Que if ($parserResult->hasQueryField($this->_fieldAlias)) { // We should throw semantical error if there's already a component for this alias - $queryComponent = $parserResult->getQueryField($this->_fieldAlias); - $fieldName = $queryComponent['fieldName']; + $fieldName = $parserResult->getQueryField($this->_fieldAlias); $message = "Cannot re-declare field alias '{$this->_fieldAlias}'" - . "for '".$paramHolder->get('fieldName')."'. It was already declared for " - . "field '{$fieldName}'."; + . "for '".$paramHolder->get('fieldName')."'."; $this->_parser->semanticalError($message); } + + // Now we map it in queryComponent + $componentAlias = Doctrine_Query_Production::DEFAULT_QUERYCOMPONENT; + $queryComponent = $parserResult->getQueryComponent($componentAlias); + + $idx = count($queryComponent['scalar']); + $queryComponent['scalar'][$idx] = $this->_fieldAlias; + $parserResult->setQueryComponent($componentAlias, $queryComponent); + + // And also in field aliases + $parserResult->setQueryField($queryComponent['scalar'][$idx], $idx); + + // Build the column alias + $this->_columnAlias = $parserResult->getTableAliasFromComponentAlias($componentAlias) + . Doctrine_Query_Production::SQLALIAS_SEPARATOR . $idx; } } diff --git a/lib/Doctrine/Query/Production/GroupByClause.php b/lib/Doctrine/Query/Production/GroupByClause.php index d648de144..df28312f8 100644 --- a/lib/Doctrine/Query/Production/GroupByClause.php +++ b/lib/Doctrine/Query/Production/GroupByClause.php @@ -52,7 +52,7 @@ class Doctrine_Query_Production_GroupByClause extends Doctrine_Query_Production public function buildSql() { - return 'GROUP BY ' . implode(', ', $this->_mapGroupByItems()) . ')'; + return 'GROUP BY ' . implode(', ', $this->_mapGroupByItems()); } diff --git a/lib/Doctrine/Query/Production/PathExpression.php b/lib/Doctrine/Query/Production/PathExpression.php index 6684995b4..4fb9a5dc7 100644 --- a/lib/Doctrine/Query/Production/PathExpression.php +++ b/lib/Doctrine/Query/Production/PathExpression.php @@ -37,7 +37,7 @@ class Doctrine_Query_Production_PathExpression extends Doctrine_Query_Production protected $_fieldName; - protected $_queryComponent; + protected $_componentAlias; public function syntax($paramHolder) @@ -73,21 +73,20 @@ class Doctrine_Query_Production_PathExpression extends Doctrine_Query_Production // Retrieve ClassMetadata $k = array_keys($queryComponents); - $componentAlias = $k[1]; + $this->_componentAlias = $k[1]; - $this->_queryComponent = $queryComponents[$componentAlias]; - $classMetadata = $this->_queryComponent['metadata']; + $classMetadata = $queryComponents[$this->_componentAlias]['metadata']; } else { - $path = $this->_identifiers[0]; - $this->_queryComponent = $parserResult->getQueryComponent($path); + $this->_componentAlias = $path = $this->_identifiers[0]; + $queryComponent = $parserResult->getQueryComponent($path); // We should have a semantical error if the queryComponent does not exists yet - if ($this->_queryComponent === null) { + if ($queryComponent === null) { $this->_parser->semanticalError("Undefined component alias for '{$path}'", $this->_parser->token); } // Initializing ClassMetadata - $classMetadata = $this->_queryComponent['metadata']; + $classMetadata = $queryComponent['metadata']; // Looping through relations for ($i = 1; $i < $l; $i++) { @@ -108,10 +107,11 @@ class Doctrine_Query_Production_PathExpression extends Doctrine_Query_Production $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); + // Assigning new componentAlias, queryComponent and classMetadata + $this->_componentAlias = $path; - $classMetadata = $this->_queryComponent['metadata']; + $queryComponent = $parserResult->getQueryComponent($path); + $classMetadata = $queryComponent['metadata']; } } @@ -133,20 +133,12 @@ class Doctrine_Query_Production_PathExpression extends Doctrine_Query_Production $manager = Doctrine_EntityManager::getManager(); $conn = $manager->getConnection(); - // 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[1]; - } + // Looking for queryComponent to fetch + $queryComponent = $parserResult->getQueryComponent($this->_componentAlias); // Generating the SQL piece - $str = $parserResult->getTableAliasFromComponentAlias($componentAlias) . '.' - . $this->_queryComponent['metadata']->getColumnName($this->_fieldName); + $str = $parserResult->getTableAliasFromComponentAlias($this->_componentAlias) . '.' + . $queryComponent['metadata']->getColumnName($this->_fieldName); return $conn->quoteIdentifier($str); } diff --git a/lib/Doctrine/Query/Production/SelectExpression.php b/lib/Doctrine/Query/Production/SelectExpression.php index be88c0cc7..bf6d47c7d 100644 --- a/lib/Doctrine/Query/Production/SelectExpression.php +++ b/lib/Doctrine/Query/Production/SelectExpression.php @@ -86,15 +86,21 @@ class Doctrine_Query_Production_SelectExpression extends Doctrine_Query_Producti { $parserResult = $this->_parser->getParserResult(); - // Here we inspect for duplicate IdentificationVariable, and if the - // left expression needs the identification variable. If yes, check - // its existance. + // We cannot have aliases for foo.* if ($this->_leftExpression instanceof Doctrine_Query_Production_PathExpressionEndingWithAsterisk && $this->_fieldIdentificationVariable !== null) { $this->_parser->semanticalError( - "Cannot assign an identification variable to a path expression with asterisk (ie. foo.bar.* AS foobaz)." + "Cannot assign an identification variable to a path expression ending with asterisk (ie. foo.bar.* AS foobaz)." ); } + // Also, we cannot have aliases for path expressions: foo.bar + if ($this->_leftExpression instanceof Doctrine_Query_Production_PathExpressionEndingWithAsterisk && $this->_fieldIdentificationVariable !== null) { + $this->_parser->semanticalError( + "Cannot assign an identification variable to a path expression (ie. foo.bar AS foobaz)." + ); + } + + // Make semantical checks $this->_leftExpression->semantical($paramHolder); if($this->_fieldIdentificationVariable !== null) { @@ -127,6 +133,10 @@ class Doctrine_Query_Production_SelectExpression extends Doctrine_Query_Producti // Retrieving parser result $parserResult = $this->_parser->getParserResult(); + // Retrieving connection + $manager = Doctrine_EntityManager::getManager(); + $conn = $manager->getConnection(); + switch (get_class($this->_leftExpression)) { case 'Doctrine_Query_Production_PathExpressionEndingWithAsterisk': return ''; @@ -134,40 +144,49 @@ class Doctrine_Query_Production_SelectExpression extends Doctrine_Query_Producti case 'Doctrine_Query_Production_PathExpression': // We bring the queryComponent from the class instance - $queryComponent = $this->_leftExpression->getQueryComponent(); + $componentAlias = $this->_leftExpression->getComponentAlias(); + $queryComponent = $parserResult->getQueryComponent($componentAlias); + $fieldName = $this->_leftExpression->getFieldName(); + + // Build the column alias now + $columnAlias = $parserResult->getTableAliasFromComponentAlias($componentAlias) + . Doctrine_Query_Production::SQLALIAS_SEPARATOR + . $queryComponent['metadata']->getColumnName($fieldName); break; default: // We bring the default queryComponent - $queryComponent = $parserResult->getQueryComponent('dctrn'); + $componentAlias = Doctrine_Query_Production::DEFAULT_QUERYCOMPONENT; + $queryComponent = $parserResult->getQueryComponent($componentAlias); + + // If we have FieldIdentificationVariable, we have to use the scalar map of it + if ($this->_fieldIdentificationVariable !== null) { + $columnAlias = $this->_fieldIdentificationVariable->getColumnAlias(); + } else { + // We have to include the map now, since we don't have the scalar mapped + $queryFields = $parserResult->getQueryFields(); + $itemIndex = 'item' . count(array_filter($queryFields, array($this, "_nonIdentifiedVariable"))); + $idx = count($queryFields); + + $queryComponent['scalar'][$idx] = $itemIndex; + $parserResult->setQueryComponent($componentAlias, $queryComponent); + + // And also in field aliases + $parserResult->setQueryField($itemIndex, $idx); + + // Build the column alias + $columnAlias = $parserResult->getTableAliasFromComponentAlias($componentAlias) + . Doctrine_Query_Production::SQLALIAS_SEPARATOR . $idx; + } break; } - // Retrieving connection - $manager = Doctrine_EntityManager::getManager(); - $conn = $manager->getConnection(); - - $componentAlias = $parserResult->getComponentAlias($queryComponent); - - if ($this->_fieldIdentificationVariable !== null) { - // We need to add scalar map in queryComponent if iidentificationvariable is set. - $idx = count($queryComponent['scalar']); - $queryComponent['scalar'][$idx] = $this->_fieldIdentificationVariable; - $parserResult->setQueryComponent($componentAlias, $queryComponent); - - $columnAlias = $parserResult->getTableAliasFromComponentAlias($componentAlias) . '__' . $idx; - } elseif ($this->_leftExpression instanceof Doctrine_Query_Production_PathExpression) { - // We need to build the column alias based on column name - $columnAlias = $parserResult->getTableAliasFromComponentAlias($componentAlias) - . '__' . $queryComponent['metadata']->getColumnName($this->_leftExpression->getFieldName()); - } else { - // The right thing should be return the index alone... but we need a better solution. - // Possible we can search the index for mapped values and add our item there. - - // [TODO] Find another return value. We cant return 0 here. - $columnAlias = 'idx__' . 0; - } - return ' AS ' . $conn->quoteIdentifier($columnAlias); } + + + protected function _nonIdentifiedVariable($value) + { + return ! is_string($value); + } } diff --git a/tests/Orm/Query/LanguageRecognitionTest.php b/tests/Orm/Query/LanguageRecognitionTest.php index f9b078fdc..cb0d8422c 100755 --- a/tests/Orm/Query/LanguageRecognitionTest.php +++ b/tests/Orm/Query/LanguageRecognitionTest.php @@ -71,11 +71,13 @@ class Orm_Query_LanguageRecognitionTest extends Doctrine_OrmTestCase public function testPlainFromClauseWithoutAlias() { $this->assertValidDql('SELECT * FROM CmsUser'); + + $this->assertValidDql('SELECT id FROM CmsUser'); } public function testPlainFromClauseWithAlias() { - $this->assertValidDql('SELECT u.* FROM CmsUser u'); + $this->assertValidDql('SELECT u.id FROM CmsUser u'); } public function testSelectSingleComponentWithAsterisk() diff --git a/tests/Orm/Query/SelectSqlGenerationTest.php b/tests/Orm/Query/SelectSqlGenerationTest.php index a5e5aa138..687f4cbb6 100755 --- a/tests/Orm/Query/SelectSqlGenerationTest.php +++ b/tests/Orm/Query/SelectSqlGenerationTest.php @@ -42,6 +42,7 @@ class Orm_Query_SelectSqlGenerationTest extends Doctrine_OrmTestCase try { $entityManager = Doctrine_EntityManager::getManager(); $query = $entityManager->createQuery($dqlToBeTested); + //echo print_r($query->parse()->getQueryFields(), true) . "\n"; parent::assertEquals($sqlToBeConfirmed, $query->getSql()); @@ -52,14 +53,31 @@ class Orm_Query_SelectSqlGenerationTest extends Doctrine_OrmTestCase } - public function testWithoutWhere() + public function testPlainFromClauseWithoutAlias() + { + $this->assertSqlGeneration( + 'SELECT * FROM CmsUser', + 'SELECT cu.id AS cu__id, cu.status AS cu__status, cu.username AS cu__username, cu.name AS cu__name FROM cms_user cu WHERE 1 = 1' + ); + + $this->assertSqlGeneration( + 'SELECT id FROM CmsUser', + 'SELECT cu.id AS cu__id FROM cms_user cu WHERE 1 = 1' + ); + } + + + public function testPlainFromClauseWithAlias() { - // NO WhereClause $this->assertSqlGeneration( 'SELECT u.id FROM CmsUser u', 'SELECT cu.id AS cu__id FROM cms_user cu WHERE 1 = 1' ); + } + + public function testSelectSingleComponentWithAsterisk() + { $this->assertSqlGeneration( 'SELECT u.* FROM CmsUser u', 'SELECT cu.id AS cu__id, cu.status AS cu__status, cu.username AS cu__username, cu.name AS cu__name FROM cms_user cu WHERE 1 = 1' @@ -67,21 +85,84 @@ class Orm_Query_SelectSqlGenerationTest extends Doctrine_OrmTestCase } - public function testWithWhere() + public function testSelectSingleComponentWithMultipleColumns() { - // "WHERE" ConditionalExpression - // ConditionalExpression = ConditionalTerm {"OR" ConditionalTerm} - // ConditionalTerm = ConditionalFactor {"AND" ConditionalFactor} - // ConditionalFactor = ["NOT"] ConditionalPrimary - // ConditionalPrimary = SimpleConditionalExpression | "(" ConditionalExpression ")" - // SimpleConditionalExpression - // = Expression (ComparisonExpression | BetweenExpression | LikeExpression - // | InExpression | NullComparisonExpression) | ExistsExpression - - // If this one test fail, all others will fail too. That's the simplest case possible $this->assertSqlGeneration( - 'SELECT u.* FROM CmsUser u WHERE id = ?', - 'SELECT cu.id AS cu__id, cu.status AS cu__status, cu.username AS cu__username, cu.name AS cu__name FROM cms_user cu WHERE cu.id = ?' + 'SELECT u.username, u.name FROM CmsUser u', + 'SELECT cu.username AS cu__username, cu.name AS cu__name FROM cms_user cu WHERE 1 = 1' + ); + } + +/* + Not supported yet! + + public function testSelectMultipleComponentsWithAsterisk() + { + $this->assertSqlGeneration( + 'SELECT u.*, p.* FROM CmsUser u, u.phonenumbers p', + 'SELECT cu.id AS cu__id, cu.status AS cu__status, cu.username AS cu__username, cu.name AS cu__name FROM cms_user cu WHERE 1 = 1' + ); + } +*/ + + public function testSelectDistinctIsSupported() + { + $this->assertSqlGeneration( + 'SELECT DISTINCT u.name FROM CmsUser u', + 'SELECT DISTINCT cu.name AS cu__name FROM cms_user cu WHERE 1 = 1' + ); + } + + + public function testAggregateFunctionInSelect() + { + $this->assertSqlGeneration( + 'SELECT COUNT(u.id) FROM CmsUser u GROUP BY u.id', + 'SELECT COUNT(cu.id) AS dctrn__0 FROM cms_user cu WHERE 1 = 1 GROUP BY cu.id' + ); + } + + public function testAggregateFunctionWithDistinctInSelect() + { + $this->assertSqlGeneration( + 'SELECT COUNT(DISTINCT u.name) FROM CmsUser u', + 'SELECT COUNT(DISTINCT cu.name) AS dctrn__0 FROM cms_user cu WHERE 1 = 1' + ); + } + + + public function testFunctionalExpressionsSupportedInWherePart() + { + $this->assertSqlGeneration( + "SELECT u.name FROM CmsUser u WHERE TRIM(u.name) = 'someone'", + "SELECT cu.name AS cu__name FROM cms_user cu WHERE TRIM(cu.name) = ''someone''" // SQLite double slashes for strings + ); + } + + + public function testArithmeticExpressionsSupportedInWherePart() + { + $this->assertSqlGeneration( + 'SELECT u.* FROM CmsUser u WHERE ((u.id + 5000) * u.id + 3) < 10000000', + 'SELECT cu.id AS cu__id, cu.status AS cu__status, cu.username AS cu__username, cu.name AS cu__name FROM cms_user cu WHERE ((cu.id + 5000) * cu.id + 3) < 10000000' + ); + } + + + public function testInExpressionSupportedInWherePart() + { + $this->assertSqlGeneration( + 'SELECT * FROM CmsUser WHERE CmsUser.id IN (1, 2)', + 'SELECT cu.id AS cu__id, cu.status AS cu__status, cu.username AS cu__username, cu.name AS cu__name FROM cms_user cu WHERE cu.id IN (1, 2)' + ); + } + + + public function testNotInExpressionSupportedInWherePart() + { + $this->assertSqlGeneration( + 'SELECT * FROM CmsUser WHERE CmsUser.id NOT IN (1)', + 'SELECT cu.id AS cu__id, cu.status AS cu__status, cu.username AS cu__username, cu.name AS cu__name FROM cms_user cu WHERE cu.id NOT IN (1)' ); }