Skip to content

Commit b284fdd

Browse files
moufmoufmoufmouf
committed
Improving IN expression handling
A IN expression with a parameter (like `status IN (:statuses)`) can now accept parameters... or not. - `status IN (:statuses)` - `status IN :statuses` are now both valid. Previously, the first was only valid in $magicQuery->buildPreparedStatement and the later was only valid in $magicQuery->build, which caused confusion.
1 parent d5fcdd8 commit b284fdd

File tree

6 files changed

+92
-16
lines changed

6 files changed

+92
-16
lines changed

‎src/SQLParser/Node/AbstractTwoOperandsOperator.php‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -100,15 +100,11 @@ public function toSql(array $parameters = array(), Connection $dbConnection = nu
100100
{
101101
if ($conditionsMode == self::CONDITION_GUESS) {
102102
$bypass = false;
103-
if ($this->leftOperand instanceof Parameter) {
104-
if ($this->leftOperand->isDiscardedOnNull() && !isset($parameters[$this->leftOperand->getName()])) {
105-
$bypass = true;
106-
}
103+
if ($this->leftOperand instanceof BypassableInterface && $this->leftOperand->canBeBypassed($parameters)) {
104+
$bypass = true;
107105
}
108-
if ($this->rightOperand instanceof Parameter) {
109-
if ($this->rightOperand->isDiscardedOnNull() && !isset($parameters[$this->rightOperand->getName()])) {
110-
$bypass = true;
111-
}
106+
if ($this->rightOperand instanceof BypassableInterface && $this->rightOperand->canBeBypassed($parameters)) {
107+
$bypass = true;
112108
}
113109
if ($bypass === true) {
114110
return;

‎src/SQLParser/Node/BypassableInterface.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?php
2+
3+
namespace SQLParser\Node;
4+
5+
6+
interface BypassableInterface
7+
{
8+
/**
9+
* Returns if this node should be removed from the tree.
10+
*/
11+
public function canBeBypassed(array $parameters): bool;
12+
}

‎src/SQLParser/Node/Expression.php‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
*
4444
* @author David Négrier <[email protected]>
4545
*/
46-
class Expression implements NodeInterface
46+
class Expression implements NodeInterface, BypassableInterface
4747
{
4848
private $baseExpression;
4949

@@ -257,4 +257,17 @@ public function walk(VisitorInterface $visitor)
257257

258258
return $visitor->leaveNode($node);
259259
}
260+
261+
/**
262+
* Returns if this node should be removed from the tree.
263+
*/
264+
public function canBeBypassed(array $parameters): bool
265+
{
266+
foreach ($this->subTree as $node) {
267+
if (!$node instanceof BypassableInterface || !$node->canBeBypassed($parameters)) {
268+
return false;
269+
}
270+
}
271+
return true;
272+
}
260273
}

‎src/SQLParser/Node/In.php‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,48 @@ protected function getOperatorSymbol()
2424
protected function getSql(array $parameters = array(), Connection $dbConnection = null, $indent = 0, $conditionsMode = self::CONDITION_APPLY, bool $extrapolateParameters = true)
2525
{
2626
$rightOperand = $this->getRightOperand();
27-
if ($rightOperand instanceof Parameter) {
28-
if (!isset($parameters[$rightOperand->getName()])) {
29-
throw new MagicQueryException("Missing parameter '" . $rightOperand->getName() . "' for 'IN' operand.");
27+
28+
$rightOperand = $this->refactorParameterToExpression($rightOperand);
29+
30+
$this->setRightOperand($rightOperand);
31+
32+
$parameterNode = $this->getParameter($rightOperand);
33+
34+
if ($parameterNode !== null) {
35+
if (!isset($parameters[$parameterNode->getName()])) {
36+
throw new MagicQueryException("Missing parameter '" . $parameterNode->getName() . "' for 'IN' operand.");
3037
}
31-
if ($parameters[$rightOperand->getName()] === []) {
38+
if ($parameters[$parameterNode->getName()] === []) {
3239
return "FALSE";
3340
}
3441
}
3542

3643
return parent::getSql($parameters, $dbConnection, $indent, $conditionsMode, $extrapolateParameters);
3744
}
45+
46+
protected function refactorParameterToExpression(NodeInterface $rightOperand): NodeInterface
47+
{
48+
if ($rightOperand instanceof Parameter) {
49+
$expression = new Expression();
50+
$expression->setSubTree([$rightOperand]);
51+
$expression->setBrackets(true);
52+
return $expression;
53+
}
54+
return $rightOperand;
55+
}
56+
57+
protected function getParameter(NodeInterface $operand): ?Parameter
58+
{
59+
if (!$operand instanceof Expression) {
60+
return null;
61+
}
62+
$subtree = $operand->getSubTree();
63+
if (!isset($subtree[0])) {
64+
return null;
65+
}
66+
if ($subtree[0] instanceof Parameter) {
67+
return $subtree[0];
68+
}
69+
return null;
70+
}
3871
}

‎src/SQLParser/Node/Parameter.php‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
*
4343
* @author David Négrier <[email protected]>
4444
*/
45-
class Parameter implements NodeInterface
45+
class Parameter implements NodeInterface, BypassableInterface
4646
{
4747
protected $name;
4848
protected $discardedOnNull = true;
@@ -175,9 +175,9 @@ public function toSql(array $parameters = array(), Connection $dbConnection = nu
175175
return 'null';
176176
} else {
177177
if (is_array($parameters[$this->name])) {
178-
return '('.implode(',', array_map(function ($item) {
178+
return implode(',', array_map(function ($item) {
179179
return "'".addslashes($this->autoPrepend.$item.$this->autoAppend)."'";
180-
}, $parameters[$this->name])).')';
180+
}, $parameters[$this->name]));
181181
} else {
182182
return "'".addslashes($this->autoPrepend.$parameters[$this->name].$this->autoAppend)."'";
183183
}
@@ -217,4 +217,12 @@ public function isDiscardedOnNull()
217217
{
218218
return $this->discardedOnNull;
219219
}
220+
221+
/**
222+
* Returns if this node should be removed from the tree.
223+
*/
224+
public function canBeBypassed(array $parameters): bool
225+
{
226+
return $this->isDiscardedOnNull() && !isset($parameters[$this->getName()]);
227+
}
220228
}

‎tests/Mouf/Database/MagicQueryTest.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@ public function testStandardSelect()
7272
$sql = 'SELECT * FROM users WHERE status in :status';
7373
$this->assertEquals("SELECT * FROM users WHERE status IN ('2','4')", self::simplifySql($magicQuery->build($sql, ['status' => [2, 4]])));
7474

75+
$sql = 'SELECT * FROM users WHERE status in (:status)';
76+
$this->assertEquals("SELECT * FROM users WHERE status IN ('2','4')", self::simplifySql($magicQuery->build($sql, ['status' => [2, 4]])));
77+
78+
$sql = 'SELECT * FROM users WHERE status IN :statuses';
79+
$this->assertEquals('SELECT * FROM users WHERE status IN (\'1\',\'2\')', self::simplifySql($magicQuery->build($sql, ['statuses' => [1, 2]])));
80+
7581
$sql = 'SELECT * FROM myTable where someField BETWEEN :value1 AND :value2';
7682
$this->assertEquals("SELECT * FROM myTable WHERE someField BETWEEN '2' AND '4'", self::simplifySql($magicQuery->build($sql, ['value1' => 2, 'value2' => 4])));
7783
$this->assertEquals("SELECT * FROM myTable WHERE someField >= '2'", self::simplifySql($magicQuery->build($sql, ['value1' => 2])));
@@ -413,5 +419,13 @@ public function testBuildPreparedStatement()
413419
// Test cache
414420
$this->assertEquals("SELECT id FROM users WHERE name LIKE :name LIMIT :offset, 2", self::simplifySql($magicQuery->buildPreparedStatement($sql, ['name' => 'bar', 'offset' => 10])));
415421
$this->assertEquals("SELECT id FROM users WHERE name LIKE :name LIMIT 2", self::simplifySql($magicQuery->buildPreparedStatement($sql, ['name' => 'bar'])));
422+
423+
$sql = 'SELECT id FROM users WHERE status IN (:status)';
424+
$this->assertEquals("SELECT id FROM users WHERE status IN (:status)", self::simplifySql($magicQuery->buildPreparedStatement($sql, ['status' => [1,2]])));
425+
$this->assertEquals("SELECT id FROM users", self::simplifySql($magicQuery->buildPreparedStatement($sql, ['status' => null])));
426+
427+
// Let's check that MagicQuery is cleverly adding parenthesis if the user forgot those in the "IN" statement.
428+
$sql = 'SELECT id FROM users WHERE status IN :status';
429+
$this->assertEquals("SELECT id FROM users WHERE status IN (:status)", self::simplifySql($magicQuery->buildPreparedStatement($sql, ['status' => [1,2]])));
416430
}
417431
}

0 commit comments

Comments
 (0)