Skip to content

Commit aad4c9d

Browse files
memleakdmemleakd
authored
refactor: Query Builder WHERE/HAVING condition handling (#10217)
1 parent 64c1b38 commit aad4c9d

3 files changed

Lines changed: 170 additions & 104 deletions

File tree

‎system/Database/BaseBuilder.php‎

Lines changed: 122 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -855,16 +855,14 @@ protected function whereColumnHaving(string $qbKey, string $first, string $secon
855855

856856
$escape ??= $this->db->protectIdentifiers;
857857

858-
$prefix = $this->{$qbKey} === [] ? $this->groupGetType('') : $this->groupGetType($type);
859-
860-
$this->{$qbKey}[] = [
858+
$this->addWhereHavingCondition($qbKey, [
861859
'columnComparison' => true,
862-
'condition' => $prefix,
860+
'condition' => '',
863861
'escape' => $escape,
864862
'first' => $first,
865863
'operator' => $operator,
866864
'second' => $second,
867-
];
865+
], $type);
868866

869867
return $this;
870868
}
@@ -1041,17 +1039,15 @@ private function whereBetweenHaving(string $qbKey, ?string $key = null, $values
10411039
$lowerBind = $this->setBind($key, $values[0], $escape);
10421040
$upperBind = $this->setBind($key, $values[1], $escape);
10431041
$not = $not ? ' NOT' : '';
1044-
$prefix = $this->{$qbKey} === [] ? $this->groupGetType('') : $this->groupGetType($type);
1045-
1046-
$this->{$qbKey}[] = [
1042+
$this->addWhereHavingCondition($qbKey, [
10471043
'betweenComparison' => true,
1048-
'condition' => $prefix,
1044+
'condition' => '',
10491045
'escape' => $escape,
10501046
'key' => $key,
10511047
'lowerBind' => $lowerBind,
10521048
'not' => $not,
10531049
'upperBind' => $upperBind,
1054-
];
1050+
], $type);
10551051

10561052
return $this;
10571053
}
@@ -1074,13 +1070,12 @@ protected function whereExistsSubquery($subquery, bool $not = false, string $typ
10741070
throw new InvalidArgumentException(sprintf('%s() expects $subquery to be of type BaseBuilder or closure', debug_backtrace(0, 2)[1]['function']));
10751071
}
10761072

1077-
$prefix = $this->QBWhere === [] ? $this->groupGetType('') : $this->groupGetType($type);
10781073
$operator = $not ? 'NOT EXISTS' : 'EXISTS';
10791074

1080-
$this->QBWhere[] = [
1081-
'condition' => "{$prefix}{$operator} {$this->buildSubquery($subquery, true)}",
1075+
$this->addWhereHavingCondition('QBWhere', [
1076+
'condition' => "{$operator} {$this->buildSubquery($subquery, true)}",
10821077
'escape' => false,
1083-
];
1078+
], $type);
10841079

10851080
return $this;
10861081
}
@@ -1119,8 +1114,6 @@ protected function whereHaving(string $qbKey, $key, $value = null, string $type
11191114
}
11201115

11211116
foreach ($keyValue as $k => $v) {
1122-
$prefix = empty($this->{$qbKey}) ? $this->groupGetType('') : $this->groupGetType($type);
1123-
11241117
if ($rawSqlOnly) {
11251118
$k = '';
11261119
$op = '';
@@ -1168,22 +1161,46 @@ protected function whereHaving(string $qbKey, $key, $value = null, string $type
11681161
$op = '';
11691162
}
11701163

1164+
$condition = $k . $op . $v;
1165+
11711166
if ($v instanceof RawSql) {
11721167
$this->{$qbKey}[] = [
1173-
'condition' => $v->with($prefix . $k . $op . $v),
1174-
'escape' => $escape,
1175-
];
1176-
} else {
1177-
$this->{$qbKey}[] = [
1178-
'condition' => $prefix . $k . $op . $v,
1168+
'condition' => $v->with($this->getWhereHavingPrefix($qbKey, $type) . $condition),
11791169
'escape' => $escape,
11801170
];
1171+
1172+
continue;
11811173
}
1174+
1175+
$this->addWhereHavingCondition($qbKey, [
1176+
'condition' => $condition,
1177+
'escape' => $escape,
1178+
], $type);
11821179
}
11831180

11841181
return $this;
11851182
}
11861183

1184+
/**
1185+
* @param 'QBHaving'|'QBWhere' $clause
1186+
* @param array<string, mixed> $condition
1187+
* @param non-empty-string $type
1188+
*/
1189+
private function addWhereHavingCondition(string $clause, array $condition, string $type): void
1190+
{
1191+
$condition['condition'] = $this->getWhereHavingPrefix($clause, $type) . $condition['condition'];
1192+
1193+
$this->{$clause}[] = $condition;
1194+
}
1195+
1196+
/**
1197+
* @param 'QBHaving'|'QBWhere' $clause
1198+
*/
1199+
private function getWhereHavingPrefix(string $clause, string $type): string
1200+
{
1201+
return $this->{$clause} === [] ? $this->groupGetType('') : $this->groupGetType($type);
1202+
}
1203+
11871204
/**
11881205
* Generates a WHERE field IN('item', 'item') SQL query,
11891206
* joined with 'AND' if appropriate.
@@ -1332,14 +1349,10 @@ protected function _whereIn(?string $key = null, $values = null, bool $not = fal
13321349

13331350
$ok = $this->setBind($ok, $whereIn, $escape);
13341351

1335-
$prefix = empty($this->{$clause}) ? $this->groupGetType('') : $this->groupGetType($type);
1336-
1337-
$whereIn = [
1338-
'condition' => "{$prefix}{$key}{$not} IN :{$ok}:",
1352+
$this->addWhereHavingCondition($clause, [
1353+
'condition' => "{$key}{$not} IN :{$ok}:",
13391354
'escape' => false,
1340-
];
1341-
1342-
$this->{$clause}[] = $whereIn;
1355+
], $type);
13431356

13441357
return $this;
13451358
}
@@ -1472,7 +1485,7 @@ protected function _like($field, string $match = '', string $type = 'AND ', stri
14721485
$v = $match;
14731486
$insensitiveSearch = false;
14741487

1475-
$prefix = empty($this->{$clause}) ? $this->groupGetType('') : $this->groupGetType($type);
1488+
$prefix = $this->getWhereHavingPrefix($clause, $type);
14761489

14771490
if ($side === 'none') {
14781491
$bind = $this->setBind($field->getBindingKey(), $v, $escape);
@@ -1506,7 +1519,7 @@ protected function _like($field, string $match = '', string $type = 'AND ', stri
15061519
$v = mb_strtolower($v, 'UTF-8');
15071520
}
15081521

1509-
$prefix = empty($this->{$clause}) ? $this->groupGetType('') : $this->groupGetType($type);
1522+
$prefix = $this->getWhereHavingPrefix($clause, $type);
15101523

15111524
if ($side === 'none') {
15121525
$bind = $this->setBind($k, $v, $escape);
@@ -3642,98 +3655,104 @@ protected function compileWhereHaving(string $qbKey): string
36423655
{
36433656
if (! empty($this->{$qbKey})) {
36443657
foreach ($this->{$qbKey} as &$qbkey) {
3645-
// Is this condition already compiled?
3646-
if (is_string($qbkey)) {
3647-
continue;
3648-
}
3658+
$qbkey = $this->compileWhereHavingCondition($qbkey);
3659+
}
36493660

3650-
if ($qbkey instanceof RawSql) {
3651-
continue;
3652-
}
3661+
return ($qbKey === 'QBHaving' ? "\nHAVING " : "\nWHERE ")
3662+
. implode("\n", $this->{$qbKey});
3663+
}
36533664

3654-
if ($qbkey['condition'] instanceof RawSql) {
3655-
$qbkey = $qbkey['condition'];
3665+
return '';
3666+
}
36563667

3657-
continue;
3658-
}
3668+
/**
3669+
* @used-by compileWhereHaving()
3670+
*
3671+
* @param array<string, mixed>|RawSql|string $condition
3672+
*/
3673+
private function compileWhereHavingCondition(array|RawSql|string $condition): RawSql|string
3674+
{
3675+
// Is this condition already compiled?
3676+
if (is_string($condition) || $condition instanceof RawSql) {
3677+
return $condition;
3678+
}
36593679

3660-
if (($qbkey['columnComparison'] ?? false) === true) {
3661-
$qbkey = $this->compileColumnComparison($qbkey);
3680+
if ($condition['condition'] instanceof RawSql) {
3681+
return $condition['condition'];
3682+
}
36623683

3663-
continue;
3664-
}
3684+
if (($condition['columnComparison'] ?? false) === true) {
3685+
return $this->compileColumnComparison($condition);
3686+
}
36653687

3666-
if (($qbkey['betweenComparison'] ?? false) === true) {
3667-
$qbkey = $this->compileBetweenComparison($qbkey);
3688+
if (($condition['betweenComparison'] ?? false) === true) {
3689+
return $this->compileBetweenComparison($condition);
3690+
}
36683691

3669-
continue;
3670-
}
3692+
if ($condition['escape'] === false) {
3693+
return $condition['condition'];
3694+
}
36713695

3672-
if ($qbkey['escape'] === false) {
3673-
$qbkey = $qbkey['condition'];
3696+
return $this->compileEscapedCondition($condition['condition']);
3697+
}
36743698

3675-
continue;
3676-
}
3699+
/**
3700+
* @used-by compileWhereHavingCondition()
3701+
*/
3702+
private function compileEscapedCondition(string $condition): string
3703+
{
3704+
// Split multiple conditions
3705+
$conditions = preg_split(
3706+
'/((?:^|\s+)AND\s+|(?:^|\s+)OR\s+)/i',
3707+
$condition,
3708+
-1,
3709+
PREG_SPLIT_DELIM_CAPTURE | PREG_SPLIT_NO_EMPTY,
3710+
);
36773711

3678-
// Split multiple conditions
3679-
$conditions = preg_split(
3680-
'/((?:^|\s+)AND\s+|(?:^|\s+)OR\s+)/i',
3681-
$qbkey['condition'],
3682-
-1,
3683-
PREG_SPLIT_DELIM_CAPTURE | PREG_SPLIT_NO_EMPTY,
3684-
);
3685-
3686-
foreach ($conditions as &$condition) {
3687-
$op = $this->getOperator($condition);
3688-
if (
3689-
$op === false
3690-
|| preg_match(
3691-
'/^(\(?)(.*)(' . preg_quote($op, '/') . ')\s*(.*(?<!\)))?(\)?)$/i',
3692-
$condition,
3693-
$matches,
3694-
) !== 1
3695-
) {
3696-
continue;
3697-
}
3712+
foreach ($conditions as &$condition) {
3713+
$op = $this->getOperator($condition);
3714+
if (
3715+
$op === false
3716+
|| preg_match(
3717+
'/^(\(?)(.*)(' . preg_quote($op, '/') . ')\s*(.*(?<!\)))?(\)?)$/i',
3718+
$condition,
3719+
$matches,
3720+
) !== 1
3721+
) {
3722+
continue;
3723+
}
36983724

3699-
// $matches = [
3700-
// 0 => '(test <= foo)', /* the whole thing */
3701-
// 1 => '(', /* optional */
3702-
// 2 => 'test', /* the field name */
3703-
// 3 => ' <= ', /* $op */
3704-
// 4 => 'foo', /* optional, if $op is e.g. 'IS NULL' */
3705-
// 5 => ')' /* optional */
3706-
// ];
3707-
3708-
if ($matches[4] !== '') {
3709-
$protectIdentifiers = false;
3710-
if (str_contains($matches[4], '.')) {
3711-
$protectIdentifiers = true;
3712-
}
3713-
3714-
if (! str_contains($matches[4], ':')) {
3715-
$matches[4] = $this->db->protectIdentifiers(trim($matches[4]), false, $protectIdentifiers);
3716-
}
3717-
3718-
$matches[4] = ' ' . $matches[4];
3719-
}
3725+
// $matches = [
3726+
// 0 => '(test <= foo)', /* the whole thing */
3727+
// 1 => '(', /* optional */
3728+
// 2 => 'test', /* the field name */
3729+
// 3 => ' <= ', /* $op */
3730+
// 4 => 'foo', /* optional, if $op is e.g. 'IS NULL' */
3731+
// 5 => ')' /* optional */
3732+
// ];
3733+
3734+
if ($matches[4] !== '') {
3735+
$protectIdentifiers = false;
3736+
if (str_contains($matches[4], '.')) {
3737+
$protectIdentifiers = true;
3738+
}
37203739

3721-
$condition = $matches[1] . $this->db->protectIdentifiers(trim($matches[2]))
3722-
. ' ' . trim($matches[3]) . $matches[4] . $matches[5];
3740+
if (! str_contains($matches[4], ':')) {
3741+
$matches[4] = $this->db->protectIdentifiers(trim($matches[4]), false, $protectIdentifiers);
37233742
}
37243743

3725-
$qbkey = implode('', $conditions);
3744+
$matches[4] = ' ' . $matches[4];
37263745
}
37273746

3728-
return ($qbKey === 'QBHaving' ? "\nHAVING " : "\nWHERE ")
3729-
. implode("\n", $this->{$qbKey});
3747+
$condition = $matches[1] . $this->db->protectIdentifiers(trim($matches[2]))
3748+
. ' ' . trim($matches[3]) . $matches[4] . $matches[5];
37303749
}
37313750

3732-
return '';
3751+
return implode('', $conditions);
37333752
}
37343753

37353754
/**
3736-
* @used-by compileWhereHaving()
3755+
* @used-by compileWhereHavingCondition()
37373756
*
37383757
* @param array{columnComparison: true, condition: string, escape: bool, first: string, operator: string, second: string} $condition
37393758
*/
@@ -3748,7 +3767,7 @@ private function compileColumnComparison(array $condition): string
37483767
}
37493768

37503769
/**
3751-
* @used-by compileWhereHaving()
3770+
* @used-by compileWhereHavingCondition()
37523771
*
37533772
* @param array{betweenComparison: true, condition: string, escape: bool, key: string, lowerBind: string, not: string, upperBind: string} $condition
37543773
*/

‎tests/system/Database/Builder/WhereTest.php‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,53 @@ public function testWhereLikeInAssociateArray(): void
151151
$this->assertSame($expectedSQL, str_replace("\n", ' ', $builder->getCompiledSelect()));
152152
}
153153

154+
/**
155+
* @param mixed $value
156+
* @param array<string, array{mixed, bool}> $expectedBinds
157+
*/
158+
#[DataProvider('provideWhereOperatorRegressionCases')]
159+
public function testWhereOperatorRegressionCases(string $key, $value, string $expectedSQL, array $expectedBinds): void
160+
{
161+
$builder = $this->db->table('jobs job');
162+
163+
$builder->where($key, $value);
164+
165+
$this->assertSame($expectedSQL, str_replace("\n", ' ', $builder->getCompiledSelect()));
166+
$this->assertSame($expectedBinds, $builder->getBinds());
167+
}
168+
169+
/**
170+
* @return iterable<string, array{string, mixed, string, array<string, array{mixed, bool}>}>
171+
*/
172+
public static function provideWhereOperatorRegressionCases(): iterable
173+
{
174+
return [
175+
'like operator with value' => [
176+
'job.status LIKE',
177+
'p%',
178+
'SELECT * FROM "jobs" "job" WHERE "job"."status" LIKE \'p%\'',
179+
[
180+
'job.status' => [
181+
'p%',
182+
true,
183+
],
184+
],
185+
],
186+
'equals operator with null' => [
187+
'job.deleted_at =',
188+
null,
189+
'SELECT * FROM "jobs" "job" WHERE "job"."deleted_at" IS NULL',
190+
[],
191+
],
192+
'not equals operator with null' => [
193+
'job.deleted_at !=',
194+
null,
195+
'SELECT * FROM "jobs" "job" WHERE "job"."deleted_at" IS NOT NULL',
196+
[],
197+
],
198+
];
199+
}
200+
154201
public function testWhereCustomString(): void
155202
{
156203
$builder = $this->db->table('jobs');

0 commit comments

Comments
 (0)