Skip to content

Commit b5d23af

Browse files
benkhalifebenkhalife
authored
Merge pull request #55 from webrium/fix/where-column-name-collision
Fix/where column name collision
2 parents eea0932 + bc7cc7e commit b5d23af

6 files changed

Lines changed: 111 additions & 13 deletions

File tree

‎src/Query/Builder.php‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,11 @@ public function where(
201201
string $boolean = 'AND',
202202
): static {
203203
// Nested group: ->where(function($q) { $q->where(...)->orWhere(...); })
204-
if (is_callable($column)) {
204+
// Only Closures count as nested groups. A plain string column name must
205+
// NEVER be treated as callable — column names like 'key', 'list', or
206+
// 'count' collide with built-in PHP function names and is_callable()
207+
// would wrongly return true for them.
208+
if ($column instanceof \Closure) {
205209
return $this->whereNested($column, $boolean);
206210
}
207211

@@ -535,7 +539,7 @@ public function whereNested(callable $callback, string $boolean = 'AND'): static
535539
*/
536540
public function whereExists(callable|Builder $subquery, string $boolean = 'AND', bool $not = false): static
537541
{
538-
if (is_callable($subquery)) {
542+
if ($subquery instanceof \Closure) {
539543
$sub = $this->newQuery();
540544
$subquery($sub);
541545
} else {
@@ -939,7 +943,10 @@ protected function addJoin(
939943
): static {
940944
$join = new JoinClause($type, $table);
941945

942-
if (is_callable($firstOrCallback)) {
946+
// Only a Closure is the advanced callback form. A plain string is the
947+
// first join column — is_callable() would wrongly match column names
948+
// like 'key' that collide with built-in PHP function names.
949+
if ($firstOrCallback instanceof \Closure) {
943950
$firstOrCallback($join);
944951
} else {
945952
$join->on($firstOrCallback, $operator ?? '=', $second ?? '');
@@ -1190,17 +1197,17 @@ public function get(): Collection
11901197
}
11911198

11921199
/**
1193-
* Execute the query and return the first row, or false if none.
1200+
* Execute the query and return the first row, or null if none.
11941201
* When a hydrator is set, the row is transformed into a model instance.
11951202
*
1196-
* @return object|false
1203+
* @return object|null
11971204
*/
1198-
public function first(): object|false
1205+
public function first(): ?object
11991206
{
12001207
$row = $this->limit(1)->connection->selectOne($this->toSql(), $this->getBindings());
12011208

1202-
if ($row === false) {
1203-
return false;
1209+
if ($row === false || $row === null) {
1210+
return null;
12041211
}
12051212

12061213
if ($this->hydrator !== null) {
@@ -1216,7 +1223,7 @@ public function first(): object|false
12161223
* @param int|string $id
12171224
* @return object|false
12181225
*/
1219-
public function find(int|string $id): object|false
1226+
public function find(int|string $id): ?object
12201227
{
12211228
return $this->where($this->primaryKey, '=', $id)->first();
12221229
}

‎src/Query/Grammars/Grammar.php‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,13 @@ protected function compileWhereRaw(array $where): string
328328
*/
329329
protected function compileWhereIn(array $where): string
330330
{
331-
$col = $this->wrapColumn($where['column']);
331+
// An empty IN list is invalid SQL in MySQL/PostgreSQL (`IN ()`).
332+
// Semantically `x IN (nothing)` is always false, so emit `1 = 0`.
333+
if (count($where['values']) === 0) {
334+
return '1 = 0';
335+
}
336+
337+
$col = $this->wrapColumn($where['column']);
332338
$placeholders = implode(', ', array_fill(0, count($where['values']), '?'));
333339

334340
return "{$col} IN ({$placeholders})";
@@ -340,6 +346,11 @@ protected function compileWhereIn(array $where): string
340346
*/
341347
protected function compileWhereNotIn(array $where): string
342348
{
349+
// An empty NOT IN list means "exclude nothing" → always true → `1 = 1`.
350+
if (count($where['values']) === 0) {
351+
return '1 = 1';
352+
}
353+
343354
$col = $this->wrapColumn($where['column']);
344355
$placeholders = implode(', ', array_fill(0, count($where['values']), '?'));
345356

‎src/Support/Collection.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,10 @@ public function isNotEmpty(): bool
347347
*/
348348
public function contains(callable|string $callbackOrColumn, mixed $value = null): bool
349349
{
350-
if (is_callable($callbackOrColumn)) {
350+
// Only a Closure is the callback form. A plain string is always a column
351+
// name — is_callable() would wrongly match column names like 'key' or
352+
// 'count' that collide with built-in PHP function names.
353+
if ($callbackOrColumn instanceof \Closure) {
351354
foreach ($this->items as $item) {
352355
if ($callbackOrColumn($item)) {
353356
return true;

‎tests/Integration/QueryTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ public function test_first_returns_single_row(): void
164164

165165
public function test_first_returns_null_when_empty(): void
166166
{
167-
$this->assertFalse($this->table()->first());
167+
$this->assertNull($this->table()->first());
168168
}
169169

170170
public function test_value_returns_single_column(): void

‎tests/Unit/BuilderTest.php‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,24 @@ public function test_where_not_in(): void
119119
$this->assertSql('SELECT * FROM `users` WHERE `status` NOT IN (?, ?)', $b);
120120
}
121121

122+
/**
123+
* Regression: an empty IN list produces invalid SQL (`IN ()`) on
124+
* MySQL/PostgreSQL. `x IN (nothing)` is always false, so it must
125+
* compile to `1 = 0` (and NOT IN to `1 = 1`).
126+
*/
127+
public function test_where_in_empty_array(): void
128+
{
129+
$b = $this->builder()->whereIn('id', []);
130+
$this->assertSql('SELECT * FROM `users` WHERE 1 = 0', $b);
131+
$this->assertSame([], $b->getBindings());
132+
}
133+
134+
public function test_where_not_in_empty_array(): void
135+
{
136+
$b = $this->builder()->whereNotIn('id', []);
137+
$this->assertSql('SELECT * FROM `users` WHERE 1 = 1', $b);
138+
}
139+
122140
public function test_where_null(): void
123141
{
124142
$this->assertSql(
@@ -160,6 +178,33 @@ public function test_where_nested_group(): void
160178
);
161179
}
162180

181+
/**
182+
* Regression: column names that collide with built-in PHP function names
183+
* (key, list, count, current, ...) were wrongly detected as callables by
184+
* is_callable(), routing them into whereNested() and triggering
185+
* "Calling key() on an object is deprecated". Only Closures are nested
186+
* groups now — a plain string is always a column name.
187+
*/
188+
public function test_where_with_php_function_named_column(): void
189+
{
190+
$b = $this->builder()->where('key', 'footer_settings');
191+
$this->assertSql('SELECT * FROM `users` WHERE `key` = ?', $b);
192+
$this->assertSame(['footer_settings'], $b->getBindings());
193+
}
194+
195+
public function test_where_with_count_column(): void
196+
{
197+
$b = $this->builder()->where('count', '>', 5);
198+
$this->assertSql('SELECT * FROM `users` WHERE `count` > ?', $b);
199+
}
200+
201+
public function test_nested_group_still_works_with_closure(): void
202+
{
203+
// A real Closure must still produce a nested group
204+
$b = $this->builder()->where(fn($q) => $q->where('a', 1)->orWhere('b', 2));
205+
$this->assertSql('SELECT * FROM `users` WHERE (`a` = ? OR `b` = ?)', $b);
206+
}
207+
163208
public function test_where_raw(): void
164209
{
165210
$b = $this->builder()->whereRaw('age > 18 AND active = 1');
@@ -297,4 +342,4 @@ public function test_true_false_shorthands(): void
297342
$this->builder()->false('active'),
298343
);
299344
}
300-
}
345+
}

‎tests/Unit/CollectionTest.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,38 @@ public function test_reduce(): void
160160
$this->assertSame(270.0, $total);
161161
}
162162

163+
// -----------------------------------------------------------------------
164+
// contains
165+
// -----------------------------------------------------------------------
166+
167+
public function test_contains_with_closure(): void
168+
{
169+
$this->assertTrue($this->makeItems()->contains(fn($u) => $u->role === 'admin'));
170+
$this->assertFalse($this->makeItems()->contains(fn($u) => $u->role === 'ghost'));
171+
}
172+
173+
public function test_contains_with_column_and_value(): void
174+
{
175+
$this->assertTrue($this->makeItems()->contains('name', 'Alice'));
176+
$this->assertFalse($this->makeItems()->contains('name', 'Nobody'));
177+
}
178+
179+
/**
180+
* Regression: contains('key', ...) must compare against the 'key' column,
181+
* not treat 'key' as a callable. is_callable('key') returns true because
182+
* key() is a built-in PHP function.
183+
*/
184+
public function test_contains_with_php_function_named_column(): void
185+
{
186+
$items = Collection::make([
187+
['id' => 1, 'key' => 'footer'],
188+
['id' => 2, 'key' => 'header'],
189+
]);
190+
191+
$this->assertTrue($items->contains('key', 'footer'));
192+
$this->assertFalse($items->contains('key', 'sidebar'));
193+
}
194+
163195
// -----------------------------------------------------------------------
164196
// Pluck / keyBy / groupBy
165197
// -----------------------------------------------------------------------

0 commit comments

Comments
 (0)