SelectExpression rewrite to support dctrn queryComponent. Fixes in DQL. Added more test cases

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;
     }
 
 

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
      *

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 = $parserResult->getQueryField($this->_fieldAlias);
-            $fieldName = $queryComponent['fieldName'];
 
             $message  = "Cannot re-declare field alias '{$this->_fieldAlias}'"
-                      . "for '".$paramHolder->get('fieldName')."'. It was already declared for "
+                      . "for '".$paramHolder->get('fieldName')."'.";
-                      . "field '{$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;
     }
 }

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());
     }
 
 

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 = $queryComponents[$this->_componentAlias]['metadata'];
-            $classMetadata = $this->_queryComponent['metadata'];
         } else {
-            $path = $this->_identifiers[0];
+            $this->_componentAlias = $path = $this->_identifiers[0];
-            $this->_queryComponent = $parserResult->getQueryComponent($path);
+            $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
+                // Assigning new componentAlias, queryComponent and classMetadata
-                $this->_queryComponent = $parserResult->getQueryComponent($path);
+                $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
+        // Looking for queryComponent to fetch
-        $componentAlias = implode('.', $this->_identifiers);
+        $queryComponent = $parserResult->getQueryComponent($this->_componentAlias);
-
-        if (count($this->_identifiers) == 0) {
-            $queryComponents = $parserResult->getQueryComponents();
-
-            // Retrieve ClassMetadata
-            $k = array_keys($queryComponents);
-            $componentAlias = $k[1];
-        }
 
         // Generating the SQL piece
-        $str = $parserResult->getTableAliasFromComponentAlias($componentAlias) . '.'
+        $str = $parserResult->getTableAliasFromComponentAlias($this->_componentAlias) . '.'
-             . $this->_queryComponent['metadata']->getColumnName($this->_fieldName);
+             . $queryComponent['metadata']->getColumnName($this->_fieldName);
 
         return $conn->quoteIdentifier($str);
     }

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
+        // We cannot have aliases for foo.*
-        // left expression needs the identification variable. If yes, check
-        // its existance.
         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);
+    }
 }

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()

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 u.username, u.name 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 cu.id = ?'
+            '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)'
         );
     }