Improving IN expression handling

src/SQLParser/Node/AbstractTwoOperandsOperator.php

@@ -100,15 +100,11 @@ public function toSql(array $parameters = array(), Connection $dbConnection = nu
     {
         if ($conditionsMode == self::CONDITION_GUESS) {
             $bypass = false;
-            if ($this->leftOperand instanceof Parameter) {
-                if ($this->leftOperand->isDiscardedOnNull() && !isset($parameters[$this->leftOperand->getName()])) {
-                    $bypass = true;
-                }
+            if ($this->leftOperand instanceof BypassableInterface && $this->leftOperand->canBeBypassed($parameters)) {
+                $bypass = true;
             }
-            if ($this->rightOperand instanceof Parameter) {
-                if ($this->rightOperand->isDiscardedOnNull() && !isset($parameters[$this->rightOperand->getName()])) {
-                    $bypass = true;
-                }
+            if ($this->rightOperand instanceof BypassableInterface && $this->rightOperand->canBeBypassed($parameters)) {
+                $bypass = true;
             }
             if ($bypass === true) {
                 return;

src/SQLParser/Node/BypassableInterface.php

@@ -0,0 +1,12 @@
+<?php
+
+namespace SQLParser\Node;
+
+
+interface BypassableInterface
+{
+    /**
+     * Returns if this node should be removed from the tree.
+     */
+    public function canBeBypassed(array $parameters): bool;
+}

src/SQLParser/Node/Expression.php

@@ -43,7 +43,7 @@
  *
  * @author David Négrier <[email protected]>
  */
-class Expression implements NodeInterface
+class Expression implements NodeInterface, BypassableInterface
 {
     private $baseExpression;
 
@@ -257,4 +257,17 @@ public function walk(VisitorInterface $visitor)
 
         return $visitor->leaveNode($node);
     }
+
+    /**
+     * Returns if this node should be removed from the tree.
+     */
+    public function canBeBypassed(array $parameters): bool
+    {
+        foreach ($this->subTree as $node) {
+            if (!$node instanceof BypassableInterface || !$node->canBeBypassed($parameters)) {
+                return false;
+            }
+        }
+        return true;
+    }
 }

src/SQLParser/Node/In.php

@@ -24,15 +24,48 @@ protected function getOperatorSymbol()
     protected function getSql(array $parameters = array(), Connection $dbConnection = null, $indent = 0, $conditionsMode = self::CONDITION_APPLY, bool $extrapolateParameters = true)
     {
         $rightOperand = $this->getRightOperand();
-        if ($rightOperand instanceof Parameter) {
-            if (!isset($parameters[$rightOperand->getName()])) {
-                throw new MagicQueryException("Missing parameter '" . $rightOperand->getName() . "' for 'IN' operand.");
+
+        $rightOperand = $this->refactorParameterToExpression($rightOperand);
+
+        $this->setRightOperand($rightOperand);
+
+        $parameterNode = $this->getParameter($rightOperand);
+
+        if ($parameterNode !== null) {
+            if (!isset($parameters[$parameterNode->getName()])) {
+                throw new MagicQueryException("Missing parameter '" . $parameterNode->getName() . "' for 'IN' operand.");
             }
-            if ($parameters[$rightOperand->getName()] === []) {
+            if ($parameters[$parameterNode->getName()] === []) {
                 return "FALSE";
             }
         }
 
         return parent::getSql($parameters, $dbConnection, $indent, $conditionsMode, $extrapolateParameters);
     }
+
+    protected function refactorParameterToExpression(NodeInterface $rightOperand): NodeInterface
+    {
+        if ($rightOperand instanceof Parameter) {
+            $expression = new Expression();
+            $expression->setSubTree([$rightOperand]);
+            $expression->setBrackets(true);
+            return $expression;
+        }
+        return $rightOperand;
+    }
+
+    protected function getParameter(NodeInterface $operand): ?Parameter
+    {
+        if (!$operand instanceof Expression) {
+            return null;
+        }
+        $subtree = $operand->getSubTree();
+        if (!isset($subtree[0])) {
+            return null;
+        }
+        if ($subtree[0] instanceof Parameter) {
+            return $subtree[0];
+        }
+        return null;
+    }
 }

src/SQLParser/Node/Parameter.php

@@ -42,7 +42,7 @@
  *
  * @author David Négrier <[email protected]>
  */
-class Parameter implements NodeInterface
+class Parameter implements NodeInterface, BypassableInterface
 {
     protected $name;
     protected $discardedOnNull = true;
@@ -175,9 +175,9 @@ public function toSql(array $parameters = array(), Connection $dbConnection = nu
                     return 'null';
                 } else {
                     if (is_array($parameters[$this->name])) {
-                        return '('.implode(',', array_map(function ($item) {
+                        return implode(',', array_map(function ($item) {
                             return "'".addslashes($this->autoPrepend.$item.$this->autoAppend)."'";
-                        }, $parameters[$this->name])).')';
+                        }, $parameters[$this->name]));
                     } else {
                         return "'".addslashes($this->autoPrepend.$parameters[$this->name].$this->autoAppend)."'";
                     }
@@ -217,4 +217,12 @@ public function isDiscardedOnNull()
     {
         return $this->discardedOnNull;
     }
+
+    /**
+     * Returns if this node should be removed from the tree.
+     */
+    public function canBeBypassed(array $parameters): bool
+    {
+        return $this->isDiscardedOnNull() && !isset($parameters[$this->getName()]);
+    }
 }

tests/Mouf/Database/MagicQueryTest.php

@@ -72,6 +72,12 @@ public function testStandardSelect()
         $sql = 'SELECT * FROM users WHERE status in :status';
         $this->assertEquals("SELECT * FROM users WHERE status IN ('2','4')", self::simplifySql($magicQuery->build($sql, ['status' => [2, 4]])));
 
+        $sql = 'SELECT * FROM users WHERE status in (:status)';
+        $this->assertEquals("SELECT * FROM users WHERE status IN ('2','4')", self::simplifySql($magicQuery->build($sql, ['status' => [2, 4]])));
+
+        $sql = 'SELECT * FROM users WHERE status IN :statuses';
+        $this->assertEquals('SELECT * FROM users WHERE status IN (\'1\',\'2\')', self::simplifySql($magicQuery->build($sql, ['statuses' => [1, 2]])));
+
         $sql = 'SELECT * FROM myTable where someField BETWEEN :value1 AND :value2';
         $this->assertEquals("SELECT * FROM myTable WHERE someField BETWEEN '2' AND '4'", self::simplifySql($magicQuery->build($sql, ['value1' => 2, 'value2' => 4])));
         $this->assertEquals("SELECT * FROM myTable WHERE someField >= '2'", self::simplifySql($magicQuery->build($sql, ['value1' => 2])));
@@ -413,5 +419,13 @@ public function testBuildPreparedStatement()
         // Test cache
         $this->assertEquals("SELECT id FROM users WHERE name LIKE :name LIMIT :offset, 2", self::simplifySql($magicQuery->buildPreparedStatement($sql, ['name' => 'bar', 'offset' => 10])));
         $this->assertEquals("SELECT id FROM users WHERE name LIKE :name LIMIT 2", self::simplifySql($magicQuery->buildPreparedStatement($sql, ['name' => 'bar'])));
+
+        $sql = 'SELECT id FROM users WHERE status IN (:status)';
+        $this->assertEquals("SELECT id FROM users WHERE status IN (:status)", self::simplifySql($magicQuery->buildPreparedStatement($sql, ['status' => [1,2]])));
+        $this->assertEquals("SELECT id FROM users", self::simplifySql($magicQuery->buildPreparedStatement($sql, ['status' => null])));
+
+        // Let's check that MagicQuery is cleverly adding parenthesis if the user forgot those in the "IN" statement.
+        $sql = 'SELECT id FROM users WHERE status IN :status';
+        $this->assertEquals("SELECT id FROM users WHERE status IN (:status)", self::simplifySql($magicQuery->buildPreparedStatement($sql, ['status' => [1,2]])));
     }
 }