fix issue greenlion#299: arithmetic operations in brackets are comma …

…separated (greenlion#306)

for example the query:

```sql
SELECT (1 + 2) AS THREE, (1 + 3) AS FOUR
```

would become:

```sql
SELECT (1,+,2) AS THREE, (1 + 3) AS FOUR
```

This is a regression caused by commit 3a9d906, that meant to add missing
commas in expressions like `CAST(... AS DECIMAL(16, 2))`,
but added too many commas visibly.

=> revert the parts of that commit that changed the code (leave the
added test assertion)

=> change the `ExpressionListProcessor` code that handles
unspecified nodes.

That code was added in commit 63d59d7, apparently to solve issue 51:
https://code.google.com/archive/p/php-sql-parser/issues/51

It seems to have copied and pasted the code that handles function
arguments, just above.

But that code does not keep commas,
and while the `FunctionBuilder` adds them back,
the `SelectBracketExpressionBuilder` does not.

The issue51Test test seems to cover the bug described in issue 51,
and it still passes.

So maybe we do not need the same handling as for function arguments in
unspecified nodes?

Hopefully someone with a better understanding of this code can have a look.

src/PHPSQLParser/builders/SelectBracketExpressionBuilder.php

@@ -66,12 +66,7 @@ public function build(array $parsed) {
         if ($parsed['expr_type'] !== ExpressionType::BRACKET_EXPRESSION) {
             return "";
         }
-        return '('
-            . $this->buildSubTree(
-                $parsed,
-                !empty($parsed['delim']) ? $parsed['delim'] : ' '
-            )
-            . ')'
+        return '(' . $this->buildSubTree($parsed, ' ') . ')'
             . $this->buildAlias($parsed);
     }
 }

src/PHPSQLParser/processors/ExpressionListProcessor.php

@@ -217,45 +217,7 @@ public function process($tokens) {
 
                 // we have parenthesis, but it seems to be an expression
                 if ($curr->isUnspecified()) {
-
-                    $localExpr = new ExpressionToken();
-                    $tmpExprList = array();
-
-                    foreach ($localTokenList as $k => $v) {
-                        $tmpToken = new ExpressionToken($k, $v);
-                        if (!$tmpToken->isCommaToken()) {
-                            $localExpr->addToken($v);
-                            $tmpExprList[] = $v;
-                        } else {
-                            // an expression could have multiple parts split by operands
-                            // if we have a comma, it is a split-point for expressions
-                            $tmpExprList = array_values($tmpExprList);
-                            $localExprList = $this->process($tmpExprList);
-
-                            if (count($localExprList) > 1) {
-                                $localExpr->setSubTree($localExprList);
-                                $localExpr->setTokenType(ExpressionType::EXPRESSION);
-                                $localExprList = $localExpr->toArray();
-                                $localExprList['alias'] = false;
-                                $localExprList = array($localExprList);
-                            }
-
-                            if (!$curr->getSubTree()) {
-                                if (!empty($localExprList)) {
-                                    $curr->setSubTree($localExprList);
-                                }
-                            } else {
-                                $tmpExprList = $curr->getSubTree();
-                                $curr->setSubTree(array_merge($tmpExprList, $localExprList));
-                            }
-                            $curr->setDelim(',');
-
-                            $tmpExprList = array();
-                            $localExpr = new ExpressionToken();
-                        }
-                    }
-
-                    $tmpExprList = array_values($tmpExprList);
+                    $tmpExprList = array_values($localTokenList);
                     $localExprList = $this->process($tmpExprList);
 
                     $curr->setTokenType(ExpressionType::BRACKET_EXPRESSION);

src/PHPSQLParser/utils/ExpressionToken.php

@@ -15,7 +15,6 @@ class ExpressionToken {
     private $trim;
     private $upper;
     private $noQuotes;
-    private $delim;
 
     public function __construct($key = "", $token = "") {
         $this->subTree = false;
@@ -65,10 +64,6 @@ public function setTokenType($type) {
         $this->tokenType = $type;
     }
 
-    public function setDelim($delim) {
-        $this->delim = $delim;
-    }
-
     public function endsWith($needle) {
         $length = strlen($needle);
         if ($length == 0) {
@@ -160,9 +155,6 @@ public function toArray() {
             $result['no_quotes'] = $this->noQuotes;
         }
         $result['sub_tree'] = $this->subTree;
-        if ($this->delim) {
-            $result['delim'] = $this->delim;
-        }
         return $result;
     }
 }

tests/cases/parser/issue299Test.php

@@ -0,0 +1,58 @@
+<?php
+/**
+ * issue299.php
+ *
+ * Test case for PHPSQLParser.
+ *
+ * PHP version 5
+ *
+ * LICENSE:
+ * Copyright (c) 2010-2014 Justin Swanhart and André Rothe
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. The name of the author may not be used to endorse or promote products
+ *    derived from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``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 AUTHOR 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.
+ *
+ * @author    André Rothe <andre.rothe@phosco.info>
+ * @copyright 2010-2014 Justin Swanhart and André Rothe
+ * @license   http://www.debian.org/misc/bsd.license  BSD License (3 Clause)
+ * @version   SVN: $Id$
+ *
+ */
+namespace PHPSQLParser\Test\Parser;
+use PHPSQLParser\PHPSQLParser;
+use PHPSQLParser\PHPSQLCreator;
+
+class issue299Test extends \PHPUnit_Framework_TestCase
+{
+    public function testIssue299()
+    {
+        $parser = new PHPSQLParser();
+        $sql = 'SELECT (1 + 2) AS THREE, (1 + 3) AS FOUR';
+        $parser->parse($sql, true);
+        $creator = new PHPSQLCreator($parser->parsed);
+        $this->assertEquals(
+            'SELECT (1 + 2) AS THREE, (1 + 3) AS FOUR',
+            $creator->created
+        );
+    }
+}

tests/cases/parser/issue51Test.php

@@ -56,7 +56,7 @@ public function testIssue51() {
         $this->assertEquals($expected, $p, 'should not die if query contains cast expression');
 
         $creator = new PHPSQLCreator($p);
-        $this->assertEquals('SELECT CAST(12 AS decimal (9,3))', $creator->created);
+        $this->assertEquals('SELECT CAST(12 AS decimal (9 , 3))', $creator->created);
     }
 }
 

tests/expected/parser/issue51.serialized

@@ -1 +1 @@
-a:1:{s:6:"SELECT";a:1:{i:0;a:6:{s:9:"expr_type";s:8:"function";s:5:"alias";b:0;s:9:"base_expr";s:4:"CAST";s:8:"sub_tree";a:1:{i:0;a:5:{s:9:"expr_type";s:10:"expression";s:9:"base_expr";s:21:"12 AS decimal( 9, 3 )";s:8:"sub_tree";a:4:{i:0;a:4:{s:9:"expr_type";s:5:"const";s:9:"base_expr";s:2:"12";s:8:"sub_tree";b:0;s:8:"position";i:13;}i:1;a:4:{s:9:"expr_type";s:8:"reserved";s:9:"base_expr";s:2:"AS";s:8:"sub_tree";b:0;s:8:"position";i:16;}i:2;a:4:{s:9:"expr_type";s:8:"reserved";s:9:"base_expr";s:7:"decimal";s:8:"sub_tree";b:0;s:8:"position";i:19;}i:3;a:5:{s:9:"expr_type";s:18:"bracket_expression";s:9:"base_expr";s:8:"( 9, 3 )";s:8:"sub_tree";a:2:{i:0;a:4:{s:9:"expr_type";s:5:"const";s:9:"base_expr";s:1:"9";s:8:"sub_tree";b:0;s:8:"position";i:28;}i:1;a:4:{s:9:"expr_type";s:5:"const";s:9:"base_expr";s:1:"3";s:8:"sub_tree";b:0;s:8:"position";i:31;}}s:5:"delim";s:1:",";s:8:"position";i:26;}}s:5:"alias";b:0;s:8:"position";i:13;}}s:5:"delim";b:0;s:8:"position";i:7;}}}
+a:1:{s:6:"SELECT";a:1:{i:0;a:6:{s:9:"expr_type";s:8:"function";s:5:"alias";b:0;s:9:"base_expr";s:4:"CAST";s:8:"sub_tree";a:1:{i:0;a:5:{s:9:"expr_type";s:10:"expression";s:9:"base_expr";s:21:"12 AS decimal( 9, 3 )";s:8:"sub_tree";a:4:{i:0;a:4:{s:9:"expr_type";s:5:"const";s:9:"base_expr";s:2:"12";s:8:"sub_tree";b:0;s:8:"position";i:13;}i:1;a:4:{s:9:"expr_type";s:8:"reserved";s:9:"base_expr";s:2:"AS";s:8:"sub_tree";b:0;s:8:"position";i:16;}i:2;a:4:{s:9:"expr_type";s:8:"reserved";s:9:"base_expr";s:7:"decimal";s:8:"sub_tree";b:0;s:8:"position";i:19;}i:3;a:4:{s:9:"expr_type";s:18:"bracket_expression";s:9:"base_expr";s:8:"( 9, 3 )";s:8:"sub_tree";a:3:{i:0;a:4:{s:9:"expr_type";s:5:"const";s:9:"base_expr";s:1:"9";s:8:"sub_tree";b:0;s:8:"position";i:28;}i:1;a:5:{s:9:"expr_type";s:6:"colref";s:9:"base_expr";s:1:",";s:9:"no_quotes";a:2:{s:5:"delim";b:0;s:5:"parts";a:1:{i:0;s:1:",";}}s:8:"sub_tree";b:0;s:8:"position";i:29;}i:2;a:4:{s:9:"expr_type";s:5:"const";s:9:"base_expr";s:1:"3";s:8:"sub_tree";b:0;s:8:"position";i:31;}}s:8:"position";i:26;}}s:5:"alias";b:0;s:8:"position";i:13;}}s:5:"delim";b:0;s:8:"position";i:7;}}}