Skip to content

Commit b97e5c4

Browse files
abnegateclaude
andcommitted
fix(sql): thread query collection through getSQLCondition as parameter
currentQueryCollection was per-instance mutable state, set by getSQLConditionsForCollection and read inside SQLite's getSQLCondition before calling findFulltextTableForAttribute. Under Swoole, the PRAGMA executed by the lookup yields the coroutine — if the same statement compiles a second SEARCH after the yield, its read of currentQueryCollection can land on a value written by a sibling coroutine. The try/finally save/restore only protected against sequential leakage, not coroutine interleaving, so the race could route one coroutine's SEARCH to another's FTS5 table. Drop the property and add an explicit ?string forCollection parameter to getSQLCondition / getSQLConditions on the SQL base, plumb it through MariaDB and Postgres' overrides (and their TYPE_OR / TYPE_AND recursive calls), and have SQLite read it from the parameter. A local parameter cannot race — each coroutine's call frame holds its own copy across yields. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent d971937 commit b97e5c4

4 files changed

Lines changed: 25 additions & 34 deletions

File tree

src/Database/Adapter/MariaDB.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1565,7 +1565,7 @@ protected function handleSpatialQueries(Query $query, array &$binds, string $att
15651565
* @return string
15661566
* @throws Exception
15671567
*/
1568-
protected function getSQLCondition(Query $query, array &$binds): string
1568+
protected function getSQLCondition(Query $query, array &$binds, ?string $forCollection = null): string
15691569
{
15701570
$query->setAttribute($this->getInternalKeyForAttribute($query->getAttribute()));
15711571

@@ -1585,7 +1585,7 @@ protected function getSQLCondition(Query $query, array &$binds): string
15851585
$conditions = [];
15861586
/* @var $q Query */
15871587
foreach ($query->getValue() as $q) {
1588-
$conditions[] = $this->getSQLCondition($q, $binds);
1588+
$conditions[] = $this->getSQLCondition($q, $binds, $forCollection);
15891589
}
15901590

15911591
$method = strtoupper($query->getMethod());

src/Database/Adapter/Postgres.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1763,7 +1763,7 @@ protected function handleObjectQueries(Query $query, array &$binds, string $attr
17631763
* @return string
17641764
* @throws Exception
17651765
*/
1766-
protected function getSQLCondition(Query $query, array &$binds): string
1766+
protected function getSQLCondition(Query $query, array &$binds, ?string $forCollection = null): string
17671767
{
17681768
$query->setAttribute($this->getInternalKeyForAttribute($query->getAttribute()));
17691769
$isNestedObjectAttribute = $query->isObjectAttribute() && \str_contains($query->getAttribute(), '.');
@@ -1793,7 +1793,7 @@ protected function getSQLCondition(Query $query, array &$binds): string
17931793
$conditions = [];
17941794
/* @var $q Query */
17951795
foreach ($query->getValue() as $q) {
1796-
$conditions[] = $this->getSQLCondition($q, $binds);
1796+
$conditions[] = $this->getSQLCondition($q, $binds, $forCollection);
17971797
}
17981798

17991799
$method = strtoupper($query->getMethod());

src/Database/Adapter/SQL.php

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -33,16 +33,6 @@ abstract class SQL extends Adapter
3333
*/
3434
protected int $floatPrecision = 17;
3535

36-
/**
37-
* Filtered collection id of the query currently being built. Set by
38-
* find/count/sum (and similar) before delegating to getSQLConditions
39-
* so SQL adapter overrides can resolve auxiliary tables (e.g.
40-
* SQLite's FTS5 virtual tables) for the active collection.
41-
* Lives on the SQL base because Memory has no concept of collection
42-
* scoping at this layer.
43-
*/
44-
protected ?string $currentQueryCollection = null;
45-
4636
/**
4737
* Configure float precision for parameter binding/logging.
4838
*/
@@ -60,22 +50,19 @@ protected function getFloatPrecision(float $value): string
6050
}
6151

6252
/**
63-
* Build SQL conditions for a query while exposing the active collection
64-
* via $currentQueryCollection. The state is always reset, even when
65-
* getSQLConditions throws — adapter overrides that read it (e.g.
66-
* SQLite's FTS5 routing) rely on it being absent outside this scope.
53+
* Build SQL conditions for `$name` and thread the collection name down
54+
* to the per-query builders. Adapter overrides that need the active
55+
* collection (e.g. SQLite's FTS5 routing) read it from this parameter
56+
* instead of per-instance state — under Swoole coroutines, instance
57+
* state would race across yields if the same statement compiled
58+
* multiple SEARCH conditions.
6759
*
6860
* @param array<Query> $queries
6961
* @param array<string,mixed> $binds
7062
*/
7163
protected function getSQLConditionsForCollection(string $name, array $queries, array &$binds, string $separator = 'AND'): string
7264
{
73-
$this->currentQueryCollection = $name;
74-
try {
75-
return $this->getSQLConditions($queries, $binds, $separator);
76-
} finally {
77-
$this->currentQueryCollection = null;
78-
}
65+
return $this->getSQLConditions($queries, $binds, $separator, $name);
7966
}
8067

8168
/**
@@ -2341,19 +2328,25 @@ public function getMaxUIDLength(): int
23412328
/**
23422329
* @param Query $query
23432330
* @param array<string, mixed> $binds
2331+
* @param ?string $forCollection Filtered collection id of the query
2332+
* currently being built, threaded through so adapter overrides
2333+
* can resolve auxiliary tables (e.g. SQLite's FTS5 virtual
2334+
* tables) for the active collection without touching shared
2335+
* state that would race across Swoole coroutine yields.
23442336
* @return string
23452337
* @throws Exception
23462338
*/
2347-
abstract protected function getSQLCondition(Query $query, array &$binds): string;
2339+
abstract protected function getSQLCondition(Query $query, array &$binds, ?string $forCollection = null): string;
23482340

23492341
/**
23502342
* @param array<Query> $queries
23512343
* @param array<string, mixed> $binds
23522344
* @param string $separator
2345+
* @param ?string $forCollection See {@see getSQLCondition}.
23532346
* @return string
23542347
* @throws Exception
23552348
*/
2356-
public function getSQLConditions(array $queries, array &$binds, string $separator = 'AND'): string
2349+
public function getSQLConditions(array $queries, array &$binds, string $separator = 'AND', ?string $forCollection = null): string
23572350
{
23582351
$conditions = [];
23592352
foreach ($queries as $query) {
@@ -2362,9 +2355,9 @@ public function getSQLConditions(array $queries, array &$binds, string $separato
23622355
}
23632356

23642357
if ($query->isNested()) {
2365-
$conditions[] = $this->getSQLConditions($query->getValues(), $binds, $query->getMethod());
2358+
$conditions[] = $this->getSQLConditions($query->getValues(), $binds, $query->getMethod(), $forCollection);
23662359
} else {
2367-
$conditions[] = $this->getSQLCondition($query, $binds);
2360+
$conditions[] = $this->getSQLCondition($query, $binds, $forCollection);
23682361
}
23692362
}
23702363

src/Database/Adapter/SQLite.php

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3131,7 +3131,7 @@ private function parseSqliteColumnType(string $declaration): array
31313131
* escapeWildcards() emits backslash escapes on every wildcard.
31323132
* Everything else falls through to the MariaDB implementation.
31333133
*/
3134-
protected function getSQLCondition(Query $query, array &$binds): string
3134+
protected function getSQLCondition(Query $query, array &$binds, ?string $forCollection = null): string
31353135
{
31363136
$method = $query->getMethod();
31373137

@@ -3150,7 +3150,7 @@ protected function getSQLCondition(Query $query, array &$binds): string
31503150
}
31513151

31523152
if ($method !== Query::TYPE_SEARCH && $method !== Query::TYPE_NOT_SEARCH) {
3153-
return parent::getSQLCondition($query, $binds);
3153+
return parent::getSQLCondition($query, $binds, $forCollection);
31543154
}
31553155

31563156
$query->setAttribute($this->getInternalKeyForAttribute($query->getAttribute()));
@@ -3168,15 +3168,13 @@ protected function getSQLCondition(Query $query, array &$binds): string
31683168
return $method === Query::TYPE_SEARCH ? '1 = 0' : '1 = 1';
31693169
}
31703170

3171-
$collection = $this->currentQueryCollection;
3172-
31733171
// Look the FTS5 table up by attribute rather than computing the
31743172
// name from the attribute alone — multi-column fulltext indexes
31753173
// encode their full sorted attribute set in the table name, so
31763174
// single-attribute searches against them would otherwise miss.
3177-
$ftsTable = $collection === null
3175+
$ftsTable = $forCollection === null
31783176
? null
3179-
: $this->findFulltextTableForAttribute($collection, $attribute);
3177+
: $this->findFulltextTableForAttribute($forCollection, $attribute);
31803178

31813179
if ($ftsTable === null) {
31823180
// No collection context or no matching FTS5 table — fall back

0 commit comments

Comments
 (0)