Skip to content

Commit 1960c3c

Browse files
abnegateclaude
andcommitted
feat(sqlite): re-enable upsert / attribute-resize / PCRE flags + review fixes
Now that the cursor-leak cascade and ON CONFLICT column-order bugs are resolved, flip the parity flags back on: - getSupportForUpserts: true (matches MariaDB; ON CONFLICT path lands) - getSupportForAttributeResizing: true (TruncateException on resize-down) - getSupportForPCRERegex: true (PHP REGEXP UDF registered in __construct) Also addresses outstanding bot-review nits: - Move currentQueryCollection from Adapter base into SQL base — it's FTS5-specific plumbing only consumed by SQLite, no business on the Memory-shared abstract. - Escape `_` in the dbstat LIKE pattern so collection `users` doesn't pull in `userA`'s shadow tables. - Anchor the closing `)` in parseSqliteColumnType so `VARCHAR(36 evil)` no longer parses to size 36. - getFTS5Value: only treat balanced quote pairs as exact-phrase mode; a lone `"` and unbalanced strings now fall through to OR-prefix. - createAttributes: wrap the per-column loop in a transaction so a mid-batch failure doesn't leave half the columns committed. - createFulltextIndex: drop backticks from the `fts5(<cols>, ...)` declaration (FTS5 treats them inconsistently across builds); triggers + INSERT lists keep the SQL-standard backtick form. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 80c2906 commit 1960c3c

3 files changed

Lines changed: 97 additions & 37 deletions

File tree

src/Database/Adapter.php

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,6 @@ abstract class Adapter
3535

3636
protected bool $skipDuplicates = false;
3737

38-
/**
39-
* Filtered collection id of the query currently being built. Set by
40-
* find/count/sum (and similar) before delegating to getSQLConditions so
41-
* adapter overrides can resolve auxiliary tables (e.g. SQLite's FTS5
42-
* virtual tables) for the active collection.
43-
*/
44-
protected ?string $currentQueryCollection = null;
45-
4638
/**
4739
* @var array<string, mixed>
4840
*/

src/Database/Adapter/SQL.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,16 @@ 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+
3646
/**
3747
* Configure float precision for parameter binding/logging.
3848
*/

src/Database/Adapter/SQLite.php

Lines changed: 87 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,42 @@ class SQLite extends MariaDB
4545
private const MARIADB_MEDIUMTEXT_BYTES = '16777215';
4646
private const MARIADB_LONGTEXT_BYTES = '4294967295';
4747

48+
public function __construct(mixed $pdo)
49+
{
50+
parent::__construct($pdo);
51+
52+
$this->registerUserFunctions();
53+
}
54+
55+
/**
56+
* SQLite has no built-in regex operator — the inherited REGEXP path
57+
* relies on the connection supplying one. Register a PHP-implemented
58+
* REGEXP UDF that calls preg_match (PCRE), forwarding through the
59+
* Utopia PDO wrapper / pool proxies via __call. Failures are
60+
* swallowed so a non-SQLite PDO doesn't blow up adapter
61+
* construction.
62+
*/
63+
private function registerUserFunctions(): void
64+
{
65+
$pcre = static function (?string $pattern, ?string $value): int {
66+
if ($pattern === null || $value === null) {
67+
return 0;
68+
}
69+
$delimited = '/' . \str_replace('/', '\/', $pattern) . '/u';
70+
71+
return @\preg_match($delimited, $value) === 1 ? 1 : 0;
72+
};
73+
74+
try {
75+
$this->getPDO()->sqliteCreateFunction('REGEXP', $pcre, 2);
76+
} catch (\Throwable) {
77+
// Either the PDO isn't SQLite or the proxy doesn't expose
78+
// the create-function hook — REGEXP queries will simply
79+
// fail at SQL parse time, which is the same behaviour as
80+
// before this UDF existed.
81+
}
82+
}
83+
4884
/**
4985
* @inheritDoc
5086
*/
@@ -271,15 +307,21 @@ public function getSizeOfCollection(string $collection): int
271307
// so the result tracks bytes actually used inside each page rather
272308
// than full 4KB page allocations, which would let small inserts
273309
// appear as a no-op against the parent assertions.
310+
// `_` is a LIKE single-char wildcard, so the `_` between
311+
// collection and tenant in the prefix would happily match any
312+
// character — collection `users` and `userA` would both pull
313+
// each other's FTS shadow tables into the size. Escape `_`
314+
// with a sentinel and declare it via ESCAPE.
315+
$ftsPattern = \str_replace('_', '\_', $ftsPrefix) . '%\_fts%';
316+
274317
$stmt = $this->getPDO()->prepare("
275318
SELECT COALESCE(SUM(\"pgsize\" - \"unused\"), 0)
276319
FROM \"dbstat\"
277-
WHERE name = :name OR name = :perms OR name LIKE :fts_pattern;
320+
WHERE name = :name OR name = :perms OR name LIKE :fts_pattern ESCAPE '\\';
278321
");
279322

280323
$stmt->bindParam(':name', $name);
281324
$stmt->bindParam(':perms', $permissions);
282-
$ftsPattern = $ftsPrefix . '%_fts%';
283325
$stmt->bindParam(':fts_pattern', $ftsPattern);
284326

285327
try {
@@ -556,6 +598,12 @@ protected function createFulltextIndex(string $collection, string $id, array $at
556598
}
557599

558600
$columns = \array_map(fn (string $attr) => $this->filter($attr), $attributes);
601+
// FTS5's `fts5(<cols>, ...)` declaration treats backticks as an
602+
// identifier quote in some builds and as part of the column
603+
// name in others; stick to bare identifiers there. Backticks are
604+
// still fine for the parent table references in triggers and
605+
// for the INSERT column list since those are regular SQL.
606+
$ftsColumnList = \implode(', ', $columns);
559607
$columnList = \implode(', ', \array_map(fn (string $c) => "`{$c}`", $columns));
560608
$newColumnList = \implode(', ', \array_map(fn (string $c) => "NEW.`{$c}`", $columns));
561609
$oldColumnList = \implode(', ', \array_map(fn (string $c) => "OLD.`{$c}`", $columns));
@@ -565,7 +613,7 @@ protected function createFulltextIndex(string $collection, string $id, array $at
565613
// index — the next document write would silently desync.
566614
$this->startTransaction();
567615
try {
568-
$createSql = "CREATE VIRTUAL TABLE `{$ftsTable}` USING fts5({$columnList}, content=`{$parentTable}`, content_rowid=`_id`)";
616+
$createSql = "CREATE VIRTUAL TABLE `{$ftsTable}` USING fts5({$ftsColumnList}, content=`{$parentTable}`, content_rowid=`_id`)";
569617
$createSql = $this->trigger(Database::EVENT_INDEX_CREATE, $createSql);
570618
$this->getPDO()->prepare($createSql)->execute();
571619

@@ -1167,12 +1215,11 @@ public function getSupportForUpdateLock(): bool
11671215
*/
11681216
public function getSupportForAttributeResizing(): bool
11691217
{
1170-
// SQLite is dynamically typed and has no MODIFY COLUMN. The
1171-
// resize-down guard in updateAttribute matches MariaDB's
1172-
// contract, but flipping this on activates parent test suites
1173-
// that currently cascade off other adapter behaviour. Re-enable
1174-
// alongside the upsert path once the cascade is understood.
1175-
return false;
1218+
// SQLite is dynamically typed with no MODIFY COLUMN, but
1219+
// updateAttribute scans the column on resize-down and raises
1220+
// TruncateException when any row exceeds the new size — same
1221+
// contract MariaDB enforces via the engine.
1222+
return true;
11761223
}
11771224

11781225
/**
@@ -1207,10 +1254,7 @@ public function getSupportForSchemaIndexes(): bool
12071254
*/
12081255
public function getSupportForUpserts(): bool
12091256
{
1210-
// Upsert support gates several scoped tests. Re-enable after
1211-
// the parent suite stops cascading failures off the existing
1212-
// ON CONFLICT path.
1213-
return false;
1257+
return true;
12141258
}
12151259

12161260
/**
@@ -2153,10 +2197,7 @@ public function getSupportNonUtfCharacters(): bool
21532197
*/
21542198
public function getSupportForPCRERegex(): bool
21552199
{
2156-
// SQLite has no built-in REGEXP. Re-enable once we figure out a
2157-
// safe place to register the PHP UDF that doesn't trip the test
2158-
// harness's connection lifecycle.
2159-
return false;
2200+
return true;
21602201
}
21612202

21622203
/**
@@ -2190,16 +2231,27 @@ protected function getInsertKeyword(): string
21902231
*/
21912232
public function createAttributes(string $collection, array $attributes): bool
21922233
{
2193-
foreach ($attributes as $attribute) {
2194-
$this->createAttribute(
2195-
$collection,
2196-
$attribute['$id'],
2197-
$attribute['type'],
2198-
$attribute['size'] ?? 0,
2199-
$attribute['signed'] ?? true,
2200-
$attribute['array'] ?? false,
2201-
$attribute['required'] ?? false,
2202-
);
2234+
// The flag advertises atomic batch creation. SQLite has no
2235+
// multi-column ADD, but DDL inside a transaction is still
2236+
// rolled back on failure, so a mid-batch error doesn't leave
2237+
// the table half-extended.
2238+
$this->startTransaction();
2239+
try {
2240+
foreach ($attributes as $attribute) {
2241+
$this->createAttribute(
2242+
$collection,
2243+
$attribute['$id'],
2244+
$attribute['type'],
2245+
$attribute['size'] ?? 0,
2246+
$attribute['signed'] ?? true,
2247+
$attribute['array'] ?? false,
2248+
$attribute['required'] ?? false,
2249+
);
2250+
}
2251+
$this->commitTransaction();
2252+
} catch (\Throwable $e) {
2253+
$this->rollbackTransaction();
2254+
throw $e;
22032255
}
22042256

22052257
return true;
@@ -2520,7 +2572,7 @@ private function parseSqliteColumnType(string $declaration): array
25202572
$base = $declaration;
25212573
$argument = null;
25222574
$secondArgument = null;
2523-
if (\preg_match('/^([A-Za-z]+)\s*\((\d+)(?:\s*,\s*(\d+))?/', $declaration, $matches) === 1) {
2575+
if (\preg_match('/^([A-Za-z]+)\s*\((\d+)(?:\s*,\s*(\d+))?\s*\)/', $declaration, $matches) === 1) {
25242576
$base = $matches[1];
25252577
$argument = (int) $matches[2];
25262578
if (isset($matches[3]) && $matches[3] !== '') {
@@ -2771,7 +2823,13 @@ protected function getLikeCondition(Query $query, array &$binds): string
27712823
*/
27722824
protected function getFTS5Value(string $value): string
27732825
{
2774-
$exact = \str_starts_with($value, '"') && \str_ends_with($value, '"');
2826+
// A balanced pair of quotes triggers exact-phrase mode. A lone
2827+
// quote (the same `"` matching both ends of a single-character
2828+
// string) and unbalanced phrases like `"foo"bar"` should not.
2829+
$exact = \strlen($value) >= 2
2830+
&& \str_starts_with($value, '"')
2831+
&& \str_ends_with($value, '"')
2832+
&& \substr_count($value, '"') === 2;
27752833

27762834
// FTS5 reserves a number of characters as syntax. Replacing them
27772835
// with whitespace lets multi-word search terms still split into

0 commit comments

Comments
 (0)