Skip to content

Commit 7452ce9

Browse files
abnegateclaude
andcommitted
fix(sqlite): address review feedback on FTS5, PCRE, relationships
- Cast json_encode() to string in MariaDB CONTAINS array path so the string|false return type doesn't trip phpstan and an encoding failure binds as an empty pattern instead of `false`. - Track REGEXP UDF registration in $pcreRegistered so getSupportForPCRERegex reflects whether sqliteCreateFunction actually succeeded; pool/proxy PDOs that swallow registration won't advertise PCRE support and emit queries that parse-error at runtime. - Quote FTS5 reserved tokens (AND/OR/NOT/NEAR) in getFTS5Value so a user term like `or cats` doesn't parse as an FTS5 boolean operator and crash the search; skip the trailing-prefix wildcard on a quoted reserved token since `"or"*` is invalid FTS5 syntax. - Route CONTAINS/CONTAINS_ANY/NOT_CONTAINS through SQLite's LIKE override so the bound pattern carries an explicit ESCAPE '\\' clause. The inherited MariaDB path emits plain LIKE; under SQLite that treats literal `%`/`_` in the search term as wildcards. - Guard ALTER COLUMN renames in updateRelationship's ONE_TO_ONE, ONE_TO_MANY, and MANY_TO_ONE branches with !is_null($newKey) / !is_null($newTwoWayKey). MANY_TO_MANY already had the check; the others would emit `RENAME COLUMN x TO ` and SQL-error when the caller passed null to update only one side. - Hash the sorted attribute set in getFulltextTableName so attributes containing underscores can't alias onto the same FTS5 table — e.g. ['ab', 'cd_ef'] and ['ab_cd', 'ef'] both joined to `ab_cd_ef`, silently colliding two indexes onto one virtual table. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 00d502f commit 7452ce9

2 files changed

Lines changed: 65 additions & 22 deletions

File tree

src/Database/Adapter/MariaDB.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1649,8 +1649,8 @@ protected function getSQLCondition(Query $query, array &$binds): string
16491649
Query::TYPE_NOT_STARTS_WITH => $this->escapeWildcards($value) . '%',
16501650
Query::TYPE_ENDS_WITH => '%' . $this->escapeWildcards($value),
16511651
Query::TYPE_NOT_ENDS_WITH => '%' . $this->escapeWildcards($value),
1652-
Query::TYPE_CONTAINS, Query::TYPE_CONTAINS_ANY => ($query->onArray()) ? '%' . $this->escapeWildcards(\json_encode($value)) . '%' : '%' . $this->escapeWildcards($value) . '%',
1653-
Query::TYPE_NOT_CONTAINS => ($query->onArray()) ? '%' . $this->escapeWildcards(\json_encode($value)) . '%' : '%' . $this->escapeWildcards($value) . '%',
1652+
Query::TYPE_CONTAINS, Query::TYPE_CONTAINS_ANY => ($query->onArray()) ? '%' . $this->escapeWildcards((string) \json_encode($value)) . '%' : '%' . $this->escapeWildcards($value) . '%',
1653+
Query::TYPE_NOT_CONTAINS => ($query->onArray()) ? '%' . $this->escapeWildcards((string) \json_encode($value)) . '%' : '%' . $this->escapeWildcards($value) . '%',
16541654
default => $value
16551655
};
16561656

src/Database/Adapter/SQLite.php

Lines changed: 63 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,16 @@ class SQLite extends MariaDB
7777
*/
7878
protected bool $emulateMySQL = false;
7979

80+
/**
81+
* Set to true once the REGEXP UDF has been successfully registered on
82+
* the underlying PDO. Used by getSupportForPCRERegex so the capability
83+
* flag reflects reality — pool/proxy PDOs that don't expose
84+
* sqliteCreateFunction will silently fail registration, and the planner
85+
* shouldn't emit REGEXP queries against a connection that can't run
86+
* them.
87+
*/
88+
private bool $pcreRegistered = false;
89+
8090
public function __construct(mixed $pdo)
8191
{
8292
parent::__construct($pdo);
@@ -125,6 +135,7 @@ private function registerUserFunctions(): void
125135

126136
try {
127137
$this->getPDO()->sqliteCreateFunction('REGEXP', $pcre, 2);
138+
$this->pcreRegistered = true;
128139
} catch (\Throwable) {
129140
// Either the PDO isn't SQLite or the proxy doesn't expose
130141
// the create-function hook — REGEXP queries will simply
@@ -829,9 +840,11 @@ protected function createFulltextIndex(string $collection, string $id, array $at
829840
}
830841

831842
/**
832-
* Stable, per-collection-and-attribute FTS5 table name. Uses attribute
833-
* names rather than the index id so getSQLCondition can derive it from
834-
* the search query without consulting the index map.
843+
* Stable, per-collection-and-attribute FTS5 table name. The attribute
844+
* set is hashed rather than joined with `_` because attribute names may
845+
* themselves contain underscores after `filter()` — the joins
846+
* `['ab', 'cd_ef']` and `['ab_cd', 'ef']` both produce `ab_cd_ef`,
847+
* which would silently alias unrelated indexes onto the same table.
835848
*
836849
* @param array<string>|string $attributes
837850
*/
@@ -840,7 +853,10 @@ protected function getFulltextTableName(string $collection, array|string $attrib
840853
$attrs = \is_array($attributes) ? $attributes : [$attributes];
841854
$attrs = \array_map(fn (string $attr) => $this->filter($attr), $attrs);
842855
\sort($attrs);
843-
$key = \implode('_', $attrs);
856+
// NUL is filtered out of attribute names, so it's safe as a
857+
// sentinel separator before hashing — no input can produce a
858+
// colliding pre-image.
859+
$key = \substr(\hash('sha1', \implode("\0", $attrs)), 0, 16);
844860

845861
return $this->getFulltextTablePrefix($collection) . $key . self::FTS_TABLE_SUFFIX;
846862
}
@@ -2455,10 +2471,10 @@ public function getSupportNonUtfCharacters(): bool
24552471
*/
24562472
public function getSupportForPCRERegex(): bool
24572473
{
2458-
// The REGEXP UDF is registered unconditionally in the
2459-
// constructor and runs preg_match (PCRE) — same surface as
2460-
// MariaDB's native operator from the caller's perspective.
2461-
return true;
2474+
// Registration runs in the constructor but is best-effort — proxy
2475+
// PDOs (pool, mirror) may not expose sqliteCreateFunction. Only
2476+
// advertise PCRE support when the UDF actually wired up.
2477+
return $this->pcreRegistered;
24622478
}
24632479

24642480
/**
@@ -2594,31 +2610,31 @@ public function updateRelationship(
25942610

25952611
switch ($type) {
25962612
case Database::RELATION_ONE_TO_ONE:
2597-
if ($key !== $newKey) {
2613+
if (!\is_null($newKey) && $key !== $newKey) {
25982614
$statements[] = "ALTER TABLE {$table} RENAME COLUMN `{$key}` TO `{$newKey}`";
25992615
}
2600-
if ($twoWay && $twoWayKey !== $newTwoWayKey) {
2616+
if ($twoWay && !\is_null($newTwoWayKey) && $twoWayKey !== $newTwoWayKey) {
26012617
$statements[] = "ALTER TABLE {$relatedTable} RENAME COLUMN `{$twoWayKey}` TO `{$newTwoWayKey}`";
26022618
}
26032619
break;
26042620
case Database::RELATION_ONE_TO_MANY:
26052621
if ($side === Database::RELATION_SIDE_PARENT) {
2606-
if ($twoWayKey !== $newTwoWayKey) {
2622+
if (!\is_null($newTwoWayKey) && $twoWayKey !== $newTwoWayKey) {
26072623
$statements[] = "ALTER TABLE {$relatedTable} RENAME COLUMN `{$twoWayKey}` TO `{$newTwoWayKey}`";
26082624
}
26092625
} else {
2610-
if ($key !== $newKey) {
2626+
if (!\is_null($newKey) && $key !== $newKey) {
26112627
$statements[] = "ALTER TABLE {$table} RENAME COLUMN `{$key}` TO `{$newKey}`";
26122628
}
26132629
}
26142630
break;
26152631
case Database::RELATION_MANY_TO_ONE:
26162632
if ($side === Database::RELATION_SIDE_CHILD) {
2617-
if ($twoWayKey !== $newTwoWayKey) {
2633+
if (!\is_null($newTwoWayKey) && $twoWayKey !== $newTwoWayKey) {
26182634
$statements[] = "ALTER TABLE {$relatedTable} RENAME COLUMN `{$twoWayKey}` TO `{$newTwoWayKey}`";
26192635
}
26202636
} else {
2621-
if ($key !== $newKey) {
2637+
if (!\is_null($newKey) && $key !== $newKey) {
26222638
$statements[] = "ALTER TABLE {$table} RENAME COLUMN `{$key}` TO `{$newKey}`";
26232639
}
26242640
}
@@ -2966,6 +2982,9 @@ protected function getSQLCondition(Query $query, array &$binds): string
29662982
Query::TYPE_NOT_STARTS_WITH,
29672983
Query::TYPE_ENDS_WITH,
29682984
Query::TYPE_NOT_ENDS_WITH,
2985+
Query::TYPE_CONTAINS,
2986+
Query::TYPE_CONTAINS_ANY,
2987+
Query::TYPE_NOT_CONTAINS,
29692988
];
29702989

29712990
if (\in_array($method, $likeMethods, true)) {
@@ -3069,11 +3088,13 @@ protected function findFulltextTableForAttribute(string $collection, string $att
30693088
}
30703089

30713090
/**
3072-
* Compile STARTS_WITH / ENDS_WITH (and their NOT variants) into LIKE
3073-
* with an explicit ESCAPE '\' clause. Inherited escapeWildcards()
3074-
* backslash-escapes every wildcard before binding; SQLite needs the
3075-
* ESCAPE clause to honour those backslashes the way MariaDB does
3076-
* implicitly.
3091+
* Compile STARTS_WITH / ENDS_WITH / CONTAINS (and their NOT variants)
3092+
* into LIKE with an explicit ESCAPE '\' clause. Inherited
3093+
* escapeWildcards() backslash-escapes every wildcard before binding;
3094+
* SQLite needs the ESCAPE clause to honour those backslashes the way
3095+
* MariaDB does implicitly. The MariaDB parent path issues plain LIKE,
3096+
* which under SQLite would treat literal `%`/`_` in the search term
3097+
* as wildcards.
30773098
*
30783099
* @param array<string,mixed> $binds
30793100
*/
@@ -3089,13 +3110,19 @@ protected function getLikeCondition(Query $query, array &$binds): string
30893110
$isNotQuery = \in_array($method, [
30903111
Query::TYPE_NOT_STARTS_WITH,
30913112
Query::TYPE_NOT_ENDS_WITH,
3113+
Query::TYPE_NOT_CONTAINS,
30923114
], true);
30933115

3116+
$onArray = $query->onArray();
3117+
30943118
$conditions = [];
30953119
foreach ($query->getValues() as $key => $value) {
30963120
$bound = match ($method) {
30973121
Query::TYPE_STARTS_WITH, Query::TYPE_NOT_STARTS_WITH => $this->escapeWildcards($value) . '%',
30983122
Query::TYPE_ENDS_WITH, Query::TYPE_NOT_ENDS_WITH => '%' . $this->escapeWildcards($value),
3123+
Query::TYPE_CONTAINS, Query::TYPE_CONTAINS_ANY, Query::TYPE_NOT_CONTAINS => $onArray
3124+
? '%' . $this->escapeWildcards((string) \json_encode($value)) . '%'
3125+
: '%' . $this->escapeWildcards($value) . '%',
30993126
default => $value,
31003127
};
31013128

@@ -3145,7 +3172,23 @@ protected function getFTS5Value(string $value): string
31453172
}
31463173

31473174
$tokens = \explode(' ', $sanitized);
3148-
$last = \array_pop($tokens) . '*';
3175+
// FTS5 treats AND/OR/NOT/NEAR (case-insensitive) as boolean operators
3176+
// when they appear as bare tokens. A user term like "or cats" would
3177+
// become "or OR cats*", which FTS5 parses as a syntax error. Wrap
3178+
// any reserved-word token in double quotes so it's matched as a
3179+
// literal term instead.
3180+
$tokens = \array_map(static function (string $token): string {
3181+
if (\preg_match('/^(AND|OR|NOT|NEAR)$/i', $token) === 1) {
3182+
return '"' . $token . '"';
3183+
}
3184+
return $token;
3185+
}, $tokens);
3186+
$last = \array_pop($tokens);
3187+
// Don't append the prefix wildcard to a quoted reserved-word token —
3188+
// FTS5 doesn't support `"or"*` as a prefix-match phrase.
3189+
if ($last !== null && !\str_starts_with($last, '"')) {
3190+
$last .= '*';
3191+
}
31493192
$tokens[] = $last;
31503193

31513194
return \implode(' OR ', $tokens);

0 commit comments

Comments
 (0)