fixing limit issue with different ways of writing a limit and adding some new unit test case

Author: David Maechler

src/SQLParser/Node/NodeFactory.php

@@ -713,12 +713,12 @@ public static function toSql($nodes, Connection $dbConnection = null, array $par
             $item = $nodes;
             if ($item instanceof SqlRenderInterface) {
                 $itemSql = $item->toSql($parameters, $dbConnection, $indent, $conditionsMode);
-                if ($itemSql == null) {
+                if ($itemSql === null || $itemSql === '') {
                     return;
                 }
                 $sql = str_repeat(' ', $indent).$itemSql;
             } else {
-                if ($item == null) {
+                if ($item === null || $item === '') {
                     return;
                 }
                 $sql = str_repeat(' ', $indent).$item;

src/SQLParser/Node/UnquotedParameter.php

@@ -58,6 +58,6 @@ public function toSql(array $parameters = array(), Connection $dbConnection = nu
     {
         $name = parent::toSql($parameters, $dbConnection, $indent, $conditionsMode);
         $name = str_replace("'", "", $name);
-        return is_numeric($name) ? (int)$name : $name;
+        return $name;
     }
 }

src/SQLParser/Query/Select.php

@@ -232,19 +232,37 @@ public function setOptions($options)
     private $limit;
 
     /**
-     * @return NodeInterface[]|NodeInterface $limit
+     * @return NodeInterface
      */
     public function getLimit() {
         return $this->limit;
     }
 
     /**
-     * @param NodeInterface[]|NodeInterface $limit
+     * @param NodeInterface $limit
      */
     public function setLimit($limit) {
         $this->limit = $limit;
     }
 
+    private $offset;
+
+    /**
+     * @return NodeInterface
+     */
+    public function getOffset()
+    {
+        return $this->offset;
+    }
+
+    /**
+     * @param NodeInterface $offset
+     */
+    public function setOffset($offset)
+    {
+        $this->offset = $offset;
+    }
+
     /**
      * @param MoufManager $moufManager
      *
@@ -260,6 +278,7 @@ public function toInstanceDescriptor(MoufManager $moufManager)
         $instanceDescriptor->getProperty('group')->setValue(NodeFactory::nodeToInstanceDescriptor($this->group, $moufManager));
         $instanceDescriptor->getProperty('having')->setValue(NodeFactory::nodeToInstanceDescriptor($this->having, $moufManager));
         $instanceDescriptor->getProperty('order')->setValue(NodeFactory::nodeToInstanceDescriptor($this->order, $moufManager));
+        $instanceDescriptor->getProperty('offset')->setValue(NodeFactory::nodeToInstanceDescriptor($this->offset, $moufManager));
         $instanceDescriptor->getProperty('limit')->setValue(NodeFactory::nodeToInstanceDescriptor($this->limit, $moufManager));
         $instanceDescriptor->getProperty('options')->setValue($this->options);
 
@@ -284,6 +303,7 @@ public function overwriteInstanceDescriptor($name, MoufManager $moufManager)
         $instanceDescriptor->getProperty('group')->setValue(NodeFactory::nodeToInstanceDescriptor($this->group, $moufManager));
         $instanceDescriptor->getProperty('having')->setValue(NodeFactory::nodeToInstanceDescriptor($this->having, $moufManager));
         $instanceDescriptor->getProperty('order')->setValue(NodeFactory::nodeToInstanceDescriptor($this->order, $moufManager));
+        $instanceDescriptor->getProperty('offset')->setValue(NodeFactory::nodeToInstanceDescriptor($this->offset, $moufManager));
         $instanceDescriptor->getProperty('limit')->setValue(NodeFactory::nodeToInstanceDescriptor($this->limit, $moufManager));
         $instanceDescriptor->getProperty('options')->setValue($this->options);
 
@@ -348,11 +368,31 @@ public function toSql(array $parameters = array(), Connection $dbConnection = nu
             }
         }
 
-        if (!empty($this->limit)) {
+        if (!empty($this->offset) && empty($this->limit)) {
+            throw new \Exception('There is no offset if no limit is provided. An error may have occurred during SQLParsing.');
+        } else if (!empty($this->limit)) {
             $limit = NodeFactory::toSql($this->limit, $dbConnection, $parameters, ',', false, $indent + 2, $conditionsMode);
-            if ($limit) {
-                $sql .= "\nLIMIT ".$limit;
+            if ($limit === '') {
+                $limit = null;
+            }
+            if (!empty($this->offset)) {
+                $offset = NodeFactory::toSql($this->offset, $dbConnection, $parameters, ',', false, $indent + 2, $conditionsMode);
+                if ($offset === '') {
+                    $offset = null;
+                }
+            } else {
+                $offset = null;
+            }
+
+
+            if ($limit === null && $offset !== null) {
+                throw new \Exception('There is no offset if no limit is provided. An error may have occurred during SQLParsing.');
+            }
+            $sql .= "\nLIMIT ";
+            if ($offset !== null) {
+                $sql .= $offset.', ';
             }
+            $sql .= $limit;
         }
 
         return $sql;

src/SQLParser/Query/StatementFactory.php

@@ -96,17 +96,34 @@ public static function toObject(array $desc)
             }
 
             if (isset($desc['LIMIT'])) {
-                $limit = self::mapArrayToNodeObjectList($desc['LIMIT']);
-                $limit = NodeFactory::simplify($limit);
+                $descLimit = self::checkLimitDesc($desc['LIMIT']);
+
+                $limit = NodeFactory::toObject($descLimit['limit']);
+                //$limit = NodeFactory::simplify($limit);
                 $select->setLimit($limit);
-            }
 
+                $offset = NodeFactory::toObject($descLimit['offset']);
+                //$offset = NodeFactory::simplify($offset);
+                $select->setOffset($offset);
+            }
             return $select;
         } else {
             throw new \BadMethodCallException('Unknown query');
         }
     }
 
+    /**
+     * @param array $descLimit
+     * @return array
+     * @throws \Exception
+     */
+    private static function checkLimitDesc(array $descLimit) {
+        if(count($descLimit) > 2) {
+            throw new \Exception('The limit returned by the SQLParser contains more than 2 items, something might went wrong.');
+        }
+        return ['offset' => $descLimit[0], 'limit' => $descLimit[1]];
+    }
+
     /**
      * @param array $items An array of objects represented as SQLParser arrays.
      */

tests/Mouf/Database/MagicQueryTest.php

@@ -12,6 +12,12 @@ public function testStandardSelect()
     {
         $magicQuery = new MagicQuery();
 
+        $sql = "SELECT id FROM users LIMIT 10";
+        $this->assertEquals("SELECT id FROM users LIMIT 10", self::simplifySql($magicQuery->build($sql)));
+
+        $sql = "SELECT id FROM users LIMIT 10 OFFSET 20";
+        $this->assertEquals("SELECT id FROM users LIMIT 20, 10", self::simplifySql($magicQuery->build($sql)));
+
         $sql = "SELECT id FROM users WHERE name LIKE :name LIMIT :offset, :limit";
         $this->assertEquals("SELECT id FROM users WHERE name LIKE 'foo' LIMIT 0, 20", self::simplifySql($magicQuery->build($sql, ['name' => 'foo', 'offset' => 0, 'limit' => 20])));