[2.0] More work on TODO items. Fixed grammar rule that was incorrect.

lib/Doctrine/ORM/Query/AST/AggregateExpression.php

@@ -1,7 +1,22 @@
 <?php
-/* 
+/*
- * To change this template, choose Tools | Templates
+ *  $Id$
- * and open the template in the editor.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * This software consists of voluntary contributions made by many individuals
+ * and is licensed under the LGPL. For more information, see
+ * <http://www.doctrine-project.org>.
  */
 
 namespace Doctrine\ORM\Query\AST;

lib/Doctrine/ORM/Query/AST/OrderByItem.php

@@ -1,30 +1,49 @@
 <?php
-/* 
+/*
- * To change this template, choose Tools | Templates
+ *  $Id$
- * and open the template in the editor.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * This software consists of voluntary contributions made by many individuals
+ * and is licensed under the LGPL. For more information, see
+ * <http://www.doctrine-project.org>.
  */
 
 namespace Doctrine\ORM\Query\AST;
 
 /**
- * OrderByItem ::= StateFieldPathExpression ["ASC" | "DESC"]
+ * AST node for the following grammar rule:
  *
- * @author robo
+ * OrderByItem ::= (ResultVariable | StateFieldPathExpression) ["ASC" | "DESC"]
+ *
+ * @author Guilherme Blanco <guilhermeblanco@hotmail.com>
+ * @author Roman Borschel <roman@code-factory.org>
+ * @since 2.0
  */
 class OrderByItem extends Node
 {
-    private $_pathExpr;
+    private $_expr;
     private $_asc;
     private $_desc;
 
-    public function __construct($pathExpr)
+    public function __construct($expr)
     {
-        $this->_pathExpr = $pathExpr;
+        $this->_expr = $expr;
     }
 
-    public function getStateFieldPathExpression()
+    public function getExpression()
     {
-        return $this->_pathExpr;
+        return $this->_expr;
     }
 
     public function setAsc($bool)

lib/Doctrine/ORM/Query/Parser.php

@@ -627,8 +627,7 @@ class Parser
     /**
      * SelectExpression ::=
      *      IdentificationVariable | StateFieldPathExpression |
-     *      (AggregateExpression | "(" Subselect ")") [["AS"] FieldAliasIdentificationVariable] |
+     *      (AggregateExpression | "(" Subselect ")" | Function) [["AS"] FieldAliasIdentificationVariable]
-     *      Function
      */
     public function SelectExpression()
     {
@@ -657,8 +656,7 @@ class Parser
             }
 
             if ($this->_lexer->isNextToken(Lexer::T_IDENTIFIER)) {
-                $this->match(Lexer::T_IDENTIFIER);
+                $fieldIdentificationVariable = $this->ResultVariable();
-                $fieldIdentificationVariable = $this->_lexer->token['value'];
             }
         } else {
             // Deny hydration of partial objects if doctrine.forcePartialLoad query hint not defined 
@@ -675,6 +673,16 @@ class Parser
 
         return new AST\SelectExpression($expression, $fieldIdentificationVariable);
     }
+    
+    /**
+     * ResultVariable ::= identifier
+     */
+    public function ResultVariable()
+    {
+        $this->match(Lexer::T_IDENTIFIER);
+    
+        return $this->_lexer->token['value'];
+    }
 
     /**
      * IdentificationVariable ::= identifier
@@ -1251,24 +1259,38 @@ class Parser
     }
 
     /**
-     * OrderByItem ::= ResultVariable | StateFieldPathExpression ["ASC" | "DESC"]
+     * OrderByItem ::= (ResultVariable | StateFieldPathExpression) ["ASC" | "DESC"]
      *
-     * @todo Implementation incomplete for OrderByItem.
+     * @todo Support general SingleValuedPathExpression instead of only StateFieldPathExpression.
      */
     public function OrderByItem()
     {
-        $item = new AST\OrderByItem($this->StateFieldPathExpression());
+        // We need to check if we are in a ResultVariable or StateFieldPathExpression
+        $glimpse = $this->_lexer->glimpse();
+        
+        if ($glimpse['value'] != '.') {
+            $expr = $this->ResultVariable();
+            
+            // @todo Check if ResultVariable is defined somewhere
+        } else {
+            $expr = $this->StateFieldPathExpression();
+        }
+    
+        $item = new AST\OrderByItem($expr);
 
         if ($this->_lexer->isNextToken(Lexer::T_ASC)) {
             $this->match(Lexer::T_ASC);
             $item->setAsc(true);
-        } else if ($this->_lexer->isNextToken(Lexer::T_DESC)) {
+            
-            $this->match(Lexer::T_DESC);
+            return $item;
-            $item->setDesc(true);
-        } else {
-            $item->setDesc(true);
         }
-
+        
+        if ($this->_lexer->isNextToken(Lexer::T_DESC)) {
+            $this->match(Lexer::T_DESC);
+        }
+        
+        $item->setDesc(true);
+        
         return $item;
     }
 
@@ -1338,37 +1360,38 @@ class Parser
         $condPrimary = new AST\ConditionalPrimary;
 
         if ($this->_lexer->isNextToken('(')) {
-            // Peek beyond the matching closing paranthesis ')'
-            $numUnmatched = 1;
             $peek = $this->_lexer->peek();
-
+            
-            while ($numUnmatched > 0) {
+            // We need to inner inspect for a subselect (ArithmeticExpression)
-                if ($peek['value'] == ')') {
+            if ($peek['type'] != Lexer::T_SELECT) {
-                    --$numUnmatched;
+                // Peek beyond and not until matching closing parenthesis
-                } else if ($peek['value'] == '(') {
+                $arithmeticOps = array("+",  "-", "*", "/");
-                    ++$numUnmatched;
+                $numUnmatched = 1;
+            
+                // While not found a closing matched parenthesis and a matched arithmetic operator 
+                while ($numUnmatched > 0 && ! in_array($peek['value'], $arithmeticOps)) {
+                    if ($peek['value'] == ')') {
+                        --$numUnmatched;
+                    } else if ($peek['value'] == '(') {
+                        ++$numUnmatched;
+                    }
+                
+                    $peek = $this->_lexer->peek();
                 }
-
-                $peek = $this->_lexer->peek();
             }
-
+            
-            $this->_lexer->resetPeek();
+            // Check if unmatched parenthesis is > 0, then we found a matching arithmetic operator
-
+            if ($numUnmatched > 0) {
-            //TODO: This is not complete, what about LIKE/BETWEEN/...etc?
-            $comparisonOps = array("=",  "<", "<=", "<>", ">", ">=", "!=");
-
-            if (in_array($peek['value'], $comparisonOps)) {
                 $condPrimary->setSimpleConditionalExpression($this->SimpleConditionalExpression());
             } else {
                 $this->match('(');
-                $conditionalExpression = $this->ConditionalExpression();
+                $condPrimary->setConditionalExpression($this->ConditionalExpression());
                 $this->match(')');
-                $condPrimary->setConditionalExpression($conditionalExpression);
             }
         } else {
             $condPrimary->setSimpleConditionalExpression($this->SimpleConditionalExpression());
         }
-
+        
         return $condPrimary;
     }
 

lib/Doctrine/ORM/Query/SqlWalker.php

@@ -240,9 +240,9 @@ class SqlWalker implements TreeWalker
     public function walkOrderByItem($orderByItem)
     {
         //TODO: support general SingleValuedPathExpression, not just state field
-        $pathExpr = $orderByItem->getStateFieldPathExpression();
+        $expr = $orderByItem->getExpression();
-        $parts = $pathExpr->getParts();
+        $parts = $expr->getParts();
-        $dqlAlias = $pathExpr->getIdentificationVariable();
+        $dqlAlias = $expr->getIdentificationVariable();
         $qComp = $this->_queryComponents[$dqlAlias];
         $columnName = $qComp['metadata']->getColumnName($parts[0]);
         $sql = $this->getSqlTableAlias($qComp['metadata']->getTableName() . $dqlAlias) . '.' . $columnName;