Skip to content

Commit 7efa677

Browse files
homersimpsonshomersimpsons
authored
Merge pull request #82 from homersimpsons/fix/expression-null-subtree
Expression: Handle null `$subTree` Between: Refactor bypass logic GitHub Actions: Wait for MySQL to start
2 parents bae0f92 + 9d90054 commit 7efa677

File tree

7 files changed

+61
-52
lines changed

7 files changed

+61
-52
lines changed

‎.github/workflows/continuous-integration.yml‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,12 @@ jobs:
8080
- name: "Install dependencies with composer"
8181
run: "composer update --no-interaction --no-progress --no-suggest --prefer-lowest"
8282

83+
- name: Wait for MySQL
84+
run: |
85+
while ! mysqladmin ping --host=127.0.0.1 --silent; do
86+
sleep 1
87+
done
88+
8389
- name: "Run PHPUnit"
8490
run: "vendor/bin/phpunit --no-coverage"
8591

@@ -124,6 +130,12 @@ jobs:
124130
- name: "Install dependencies with composer"
125131
run: "composer install --no-interaction --no-progress --no-suggest"
126132

133+
- name: Wait for MySQL
134+
run: |
135+
while ! mysqladmin ping --host=127.0.0.1 --silent; do
136+
sleep 1
137+
done
138+
127139
- name: "Run PHPUnit"
128140
run: "vendor/bin/phpunit"
129141

@@ -170,5 +182,11 @@ jobs:
170182
- name: "Install dependencies with composer"
171183
run: "composer install --no-interaction --no-progress --no-suggest"
172184

185+
- name: Wait for MySQL
186+
run: |
187+
while ! mysqladmin ping --host=127.0.0.1 --silent; do
188+
sleep 1
189+
done
190+
173191
- name: "Run PHPUnit"
174192
run: "vendor/bin/phpunit --no-coverage"

‎composer.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
}
1414
],
1515
"require": {
16-
"php": ">=7.4.0 || ^8.0",
16+
"php": "^7.4 || ^8.0",
1717
"mouf/utils.common.conditioninterface": "~2.0",
1818
"mouf/utils.value.value-interface": "~1.0",
1919
"mouf/utils.common.paginable-interface": "~1.0",

‎phpstan-baseline.neon‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,6 @@ parameters:
9595
count: 1
9696
path: src/SQLParser/Node/NodeFactory.php
9797

98-
-
99-
message: "#^Parameter \\#1 \\$var of function count expects array\\|Countable, array\\<SQLParser\\\\Node\\\\NodeInterface\\>\\|SQLParser\\\\Node\\\\NodeInterface given\\.$#"
100-
count: 1
101-
path: src/SQLParser/Node/NodeFactory.php
102-
10398
-
10499
message: "#^Parameter \\#2 \\.\\.\\.\\$args of function array_merge expects array, array\\<SQLParser\\\\Node\\\\NodeInterface\\>\\|SQLParser\\\\Node\\\\NodeInterface given\\.$#"
105100
count: 1

‎src/SQLParser/Node/Between.php‎

Lines changed: 35 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ public function toInstanceDescriptor(MoufManager $moufManager)
148148
}
149149

150150
/**
151-
* Renders the object as a SQL string.
151+
* Renders the object as an SQL string.
152152
*
153153
* @param array $parameters
154154
* @param AbstractPlatform $platform
@@ -160,52 +160,45 @@ public function toInstanceDescriptor(MoufManager $moufManager)
160160
*/
161161
public function toSql(array $parameters, AbstractPlatform $platform, int $indent = 0, $conditionsMode = self::CONDITION_APPLY, bool $extrapolateParameters = true): ?string
162162
{
163-
$minBypass = false;
164-
$maxBypass = false;
165-
166-
if ($conditionsMode == self::CONDITION_GUESS) {
167-
if ($this->minValueOperand instanceof Parameter) {
168-
if ($this->minValueOperand->isDiscardedOnNull() && !isset($parameters[$this->minValueOperand->getName()])) {
169-
$minBypass = true;
170-
}
171-
}
163+
switch ($conditionsMode) {
164+
case self::CONDITION_APPLY:
165+
$minBypass = $this->minValueCondition && !$this->minValueCondition->isOk($parameters);
166+
$maxBypass = $this->maxValueCondition && !$this->maxValueCondition->isOk($parameters);
167+
break;
168+
case self::CONDITION_GUESS:
169+
$minBypass = $this->minValueOperand instanceof Parameter && $this->minValueOperand->isDiscardedOnNull() && !isset($parameters[$this->minValueOperand->getName()]);
170+
$maxBypass = $this->maxValueOperand instanceof Parameter && $this->maxValueOperand->isDiscardedOnNull() && !isset($parameters[$this->maxValueOperand->getName()]);
171+
break;
172+
case self::CONDITION_IGNORE:
173+
$minBypass = false;
174+
$maxBypass = false;
175+
break;
176+
default:
177+
throw new \InvalidArgumentException('Invalid `$conditionsMode`: "' . $conditionsMode. '"');
178+
}
172179

173-
if ($this->maxValueOperand instanceof Parameter) {
174-
if ($this->maxValueOperand->isDiscardedOnNull() && !isset($parameters[$this->maxValueOperand->getName()])) {
175-
$maxBypass = true;
176-
}
177-
}
178-
} elseif ($conditionsMode == self::CONDITION_IGNORE) {
179-
$minBypass = false;
180-
$maxBypass = false;
181-
} else {
182-
if ($this->minValueCondition && !$this->minValueCondition->isOk($parameters)) {
183-
$minBypass = true;
184-
}
185-
if ($this->maxValueCondition && !$this->maxValueCondition->isOk($parameters)) {
186-
$maxBypass = true;
187-
}
180+
if ($maxBypass && $minBypass) {
181+
return null;
182+
}
183+
184+
if ($minBypass) {
185+
return sprintf('%s <= %s',
186+
NodeFactory::toSql($this->leftOperand, $platform, $parameters, ' ', false, $indent, $conditionsMode, $extrapolateParameters),
187+
NodeFactory::toSql($this->maxValueOperand, $platform, $parameters, ' ', false, $indent, $conditionsMode, $extrapolateParameters)
188+
);
188189
}
189190

190-
if (!$minBypass && !$maxBypass) {
191-
$sql = NodeFactory::toSql($this->leftOperand, $platform, $parameters, ' ', false, $indent, $conditionsMode, $extrapolateParameters);
192-
$sql .= ' BETWEEN ';
193-
$sql .= NodeFactory::toSql($this->minValueOperand, $platform, $parameters, ' ', false, $indent, $conditionsMode, $extrapolateParameters);
194-
$sql .= ' AND ';
195-
$sql .= NodeFactory::toSql($this->maxValueOperand, $platform, $parameters, ' ', false, $indent, $conditionsMode, $extrapolateParameters);
196-
} elseif (!$minBypass && $maxBypass) {
197-
$sql = NodeFactory::toSql($this->leftOperand, $platform, $parameters, ' ', false, $indent, $conditionsMode, $extrapolateParameters);
198-
$sql .= ' >= ';
199-
$sql .= NodeFactory::toSql($this->minValueOperand, $platform, $parameters, ' ', false, $indent, $conditionsMode, $extrapolateParameters);
200-
} elseif ($minBypass && !$maxBypass) {
201-
$sql = NodeFactory::toSql($this->leftOperand, $platform, $parameters, ' ', false, $indent, $conditionsMode, $extrapolateParameters);
202-
$sql .= ' <= ';
203-
$sql .= NodeFactory::toSql($this->maxValueOperand, $platform, $parameters, ' ', false, $indent, $conditionsMode, $extrapolateParameters);
204-
} else {
205-
$sql = null;
191+
if ($maxBypass) {
192+
return sprintf('%s >= %s',
193+
NodeFactory::toSql($this->leftOperand, $platform, $parameters, ' ', false, $indent, $conditionsMode, $extrapolateParameters),
194+
NodeFactory::toSql($this->minValueOperand, $platform, $parameters, ' ', false, $indent, $conditionsMode, $extrapolateParameters));
206195
}
207196

208-
return $sql;
197+
return sprintf('%s BETWEEN %s AND %s',
198+
NodeFactory::toSql($this->leftOperand, $platform, $parameters, ' ', false, $indent, $conditionsMode, $extrapolateParameters),
199+
NodeFactory::toSql($this->minValueOperand, $platform, $parameters, ' ', false, $indent, $conditionsMode, $extrapolateParameters),
200+
NodeFactory::toSql($this->maxValueOperand, $platform, $parameters, ' ', false, $indent, $conditionsMode, $extrapolateParameters)
201+
);
209202
}
210203

211204
/**

‎src/SQLParser/Node/Expression.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,11 @@ public function setBaseExpression($baseExpression): void
6969
$this->baseExpression = $baseExpression;
7070
}
7171

72-
/** @var NodeInterface[]|NodeInterface */
72+
/** @var NodeInterface[]|NodeInterface|null */
7373
private $subTree;
7474

7575
/**
76-
* @return NodeInterface|NodeInterface[]
76+
* @return NodeInterface|NodeInterface[]|null
7777
*/
7878
public function getSubTree()
7979
{
@@ -263,7 +263,7 @@ public function walk(VisitorInterface $visitor)
263263
$this->subTree[$key] = $result2;
264264
}
265265
}
266-
} else {
266+
} elseif ($this->subTree instanceof NodeInterface) {
267267
$result2 = $this->subTree->walk($visitor);
268268
if ($result2 === NodeTraverser::REMOVE_NODE) {
269269
$this->subTree = [];

‎src/SQLParser/Node/NodeFactory.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -652,7 +652,7 @@ public static function simplify($nodes)
652652
if ($operand instanceof Expression) {
653653
if (empty($operand->getBaseExpression())) {
654654
$subTree = $operand->getSubTree();
655-
if (count($subTree) === 1) {
655+
if (is_array($subTree) && count($subTree) === 1) {
656656
$newNodes = array_merge($newNodes, self::simplify($subTree));
657657
} else {
658658
$newNodes[] = $operand;

‎tests/Mouf/Database/MagicQueryTest.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,9 @@ public function testStandardSelect()
208208

209209
$sql = 'SELECT COUNT(*) AS cnt FROM (SELECT id FROM country) subquery';
210210
$this->assertEquals($sql, self::simplifySql($magicQuery->build($sql)));
211+
212+
$sql = "SELECT YEAR(register_date) AS year FROM users GROUP BY year";
213+
$this->assertEquals("SELECT YEAR(register_date) AS year FROM users GROUP BY year", self::simplifySql($magicQuery->build($sql)));
211214
}
212215

213216
public function testInNullException() {

0 commit comments

Comments
 (0)