From 4a2715cad4d7f03b905537667f399d6934f9add3 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Tue, 21 Apr 2026 10:48:17 +0100 Subject: [PATCH 01/10] Replace skipDuplicates with OnDuplicate enum + withOnDuplicate scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add Utopia\Database\OnDuplicate enum with Fail/Skip/Upsert cases - Replace bool $skipDuplicates flag with OnDuplicate $onDuplicate on Database and Adapter, scoped via new withOnDuplicate(mode, callable) - Remove skipDuplicates(callable) — callers use withOnDuplicate(Skip) - Adapters (MariaDB via SQL, SQLite, Postgres, Mongo) dispatch on enum via match; INSERT IGNORE / OR IGNORE / ON CONFLICT DO NOTHING for Skip; REPLACE / OR REPLACE for Upsert where natively supported - Pool + Mirror updated to propagate the enum state - Tests updated to call withOnDuplicate(OnDuplicate::Skip, $cb) Prepares the library for Upsert row semantics and schema-level skip/upsert handling in a follow-up commit. --- src/Database/Adapter.php | 16 ++++++------- src/Database/Adapter/Mongo.php | 6 +++-- src/Database/Adapter/Pool.php | 16 ++++++++----- src/Database/Adapter/Postgres.php | 7 ++++-- src/Database/Adapter/SQL.php | 12 +++++++--- src/Database/Adapter/SQLite.php | 7 +++++- src/Database/Database.php | 22 ++++++++++++------ src/Database/Mirror.php | 10 ++++---- src/Database/OnDuplicate.php | 27 ++++++++++++++++++++++ tests/e2e/Adapter/MirrorTest.php | 3 ++- tests/e2e/Adapter/Scopes/DocumentTests.php | 19 +++++++-------- 11 files changed, 102 insertions(+), 43 deletions(-) create mode 100644 src/Database/OnDuplicate.php diff --git a/src/Database/Adapter.php b/src/Database/Adapter.php index 1678024ee..079418a54 100644 --- a/src/Database/Adapter.php +++ b/src/Database/Adapter.php @@ -33,7 +33,7 @@ abstract class Adapter protected bool $alterLocks = false; - protected bool $skipDuplicates = false; + protected OnDuplicate $onDuplicate = OnDuplicate::Fail; /** * @var array @@ -395,23 +395,23 @@ public function inTransaction(): bool } /** - * Run a callback with skipDuplicates enabled. - * Duplicate key errors during createDocuments() will be silently skipped - * instead of thrown. Nestable — saves and restores previous state. + * Run a callback scoped to a specific OnDuplicate mode. Create-style + * operations (createDocument, createCollection, createAttribute, createIndex) + * dispatch on this mode. Nestable — saves and restores previous state. * * @template T * @param callable(): T $callback * @return T */ - public function skipDuplicates(callable $callback): mixed + public function withOnDuplicate(OnDuplicate $mode, callable $callback): mixed { - $previous = $this->skipDuplicates; - $this->skipDuplicates = true; + $previous = $this->onDuplicate; + $this->onDuplicate = $mode; try { return $callback(); } finally { - $this->skipDuplicates = $previous; + $this->onDuplicate = $previous; } } diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index 311b60476..72cd524c7 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -17,6 +17,7 @@ use Utopia\Database\Exception\Timeout as TimeoutException; use Utopia\Database\Exception\Transaction as TransactionException; use Utopia\Database\Exception\Type as TypeException; +use Utopia\Database\OnDuplicate; use Utopia\Database\Query; use Utopia\Database\Validator\Authorization; use Utopia\Mongo\Client; @@ -123,7 +124,8 @@ public function withTransaction(callable $callback): mixed } // upsert + $setOnInsert hits WriteConflict (E112) under txn snapshot isolation. - if ($this->skipDuplicates) { + // Both Skip and Upsert modes use the no-transaction path. + if ($this->onDuplicate !== OnDuplicate::Fail) { return $callback(); } @@ -1498,7 +1500,7 @@ public function createDocuments(Document $collection, array $documents): array } // insertMany aborts the txn on any duplicate; upsert + $setOnInsert no-ops instead. - if ($this->skipDuplicates) { + if ($this->onDuplicate === OnDuplicate::Skip) { if (empty($records)) { return []; } diff --git a/src/Database/Adapter/Pool.php b/src/Database/Adapter/Pool.php index 7bbfb98f2..e23ffdb4d 100644 --- a/src/Database/Adapter/Pool.php +++ b/src/Database/Adapter/Pool.php @@ -6,6 +6,7 @@ use Utopia\Database\Database; use Utopia\Database\Document; use Utopia\Database\Exception as DatabaseException; +use Utopia\Database\OnDuplicate; use Utopia\Database\Validator\Authorization; use Utopia\Pools\Pool as UtopiaPool; @@ -43,8 +44,9 @@ public function __construct(UtopiaPool $pool) public function delegate(string $method, array $args): mixed { if ($this->pinnedAdapter !== null) { - if ($this->skipDuplicates) { - return $this->pinnedAdapter->skipDuplicates( + if ($this->onDuplicate !== OnDuplicate::Fail) { + return $this->pinnedAdapter->withOnDuplicate( + $this->onDuplicate, fn () => $this->pinnedAdapter->{$method}(...$args) ); } @@ -71,8 +73,9 @@ public function delegate(string $method, array $args): mixed $adapter->setMetadata($key, $value); } - if ($this->skipDuplicates) { - return $adapter->skipDuplicates( + if ($this->onDuplicate !== OnDuplicate::Fail) { + return $adapter->withOnDuplicate( + $this->onDuplicate, fn () => $adapter->{$method}(...$args) ); } @@ -156,8 +159,9 @@ public function withTransaction(callable $callback): mixed $this->pinnedAdapter = $adapter; try { - if ($this->skipDuplicates) { - return $adapter->skipDuplicates( + if ($this->onDuplicate !== OnDuplicate::Fail) { + return $adapter->withOnDuplicate( + $this->onDuplicate, fn () => $adapter->withTransaction($callback) ); } diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 2c27e08e7..d705ca9b7 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -17,6 +17,7 @@ use Utopia\Database\Exception\Transaction as TransactionException; use Utopia\Database\Exception\Truncate as TruncateException; use Utopia\Database\Helpers\ID; +use Utopia\Database\OnDuplicate; use Utopia\Database\Operator; use Utopia\Database\Query; @@ -2357,18 +2358,20 @@ protected function getInsertKeyword(): string protected function getInsertSuffix(string $table): string { - if (!$this->skipDuplicates) { + if ($this->onDuplicate === OnDuplicate::Fail) { return ''; } $conflictTarget = $this->sharedTables ? '("_uid", "_tenant")' : '("_uid")'; + // Upsert on Postgres uses ON CONFLICT DO UPDATE; for now Skip and Upsert + // both DO NOTHING until upsert SET clause generation is implemented. return "ON CONFLICT {$conflictTarget} DO NOTHING"; } protected function getInsertPermissionsSuffix(): string { - if (!$this->skipDuplicates) { + if ($this->onDuplicate === OnDuplicate::Fail) { return ''; } diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index 3fe2696db..e734b9304 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -15,6 +15,7 @@ use Utopia\Database\Exception\NotFound as NotFoundException; use Utopia\Database\Exception\Timeout as TimeoutException; use Utopia\Database\Exception\Transaction as TransactionException; +use Utopia\Database\OnDuplicate; use Utopia\Database\Operator; use Utopia\Database\Query; @@ -1030,12 +1031,17 @@ public function getSupportForHostname(): bool } /** - * Returns the INSERT keyword, optionally with IGNORE for duplicate handling. - * Override in adapter subclasses for DB-specific syntax. + * Returns the INSERT keyword, varying by the active OnDuplicate mode. + * Override in adapter subclasses for DB-specific syntax (e.g. Postgres uses + * suffix ON CONFLICT instead). */ protected function getInsertKeyword(): string { - return $this->skipDuplicates ? 'INSERT IGNORE INTO' : 'INSERT INTO'; + return match ($this->onDuplicate) { + OnDuplicate::Skip => 'INSERT IGNORE INTO', + OnDuplicate::Upsert => 'INSERT INTO', // Upsert is realized by the ON DUPLICATE KEY UPDATE suffix on MySQL/MariaDB — handled in getInsertSuffix. + OnDuplicate::Fail => 'INSERT INTO', + }; } /** diff --git a/src/Database/Adapter/SQLite.php b/src/Database/Adapter/SQLite.php index 33f370775..fdf5a4d02 100644 --- a/src/Database/Adapter/SQLite.php +++ b/src/Database/Adapter/SQLite.php @@ -16,6 +16,7 @@ use Utopia\Database\Exception\Timeout as TimeoutException; use Utopia\Database\Exception\Transaction as TransactionException; use Utopia\Database\Helpers\ID; +use Utopia\Database\OnDuplicate; use Utopia\Database\Operator; /** @@ -1939,6 +1940,10 @@ public function getSupportForTTLIndexes(): bool protected function getInsertKeyword(): string { - return $this->skipDuplicates ? 'INSERT OR IGNORE INTO' : 'INSERT INTO'; + return match ($this->onDuplicate) { + OnDuplicate::Skip => 'INSERT OR IGNORE INTO', + OnDuplicate::Upsert => 'INSERT OR REPLACE INTO', + OnDuplicate::Fail => 'INSERT INTO', + }; } } diff --git a/src/Database/Database.php b/src/Database/Database.php index 136baad9b..0ea5e655c 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -417,7 +417,7 @@ class Database protected bool $preserveDates = false; - protected bool $skipDuplicates = false; + protected OnDuplicate $onDuplicate = OnDuplicate::Fail; protected bool $preserveSequence = false; @@ -844,15 +844,23 @@ public function skipRelationshipsExistCheck(callable $callback): mixed } } - public function skipDuplicates(callable $callback): mixed + /** + * Run $callback within a scope where create-style operations apply the + * given OnDuplicate mode. Nestable — previous mode is restored on return. + * + * @template T + * @param callable(): T $callback + * @return T + */ + public function withOnDuplicate(OnDuplicate $mode, callable $callback): mixed { - $previous = $this->skipDuplicates; - $this->skipDuplicates = true; + $previous = $this->onDuplicate; + $this->onDuplicate = $mode; try { return $callback(); } finally { - $this->skipDuplicates = $previous; + $this->onDuplicate = $previous; } } @@ -5727,8 +5735,8 @@ public function createDocuments( foreach (\array_chunk($documents, $batchSize) as $chunk) { $insert = fn () => $this->withTransaction(fn () => $this->adapter->createDocuments($collection, $chunk)); // Set adapter flag before withTransaction so Mongo can opt out of a real txn. - $batch = $this->skipDuplicates - ? $this->adapter->skipDuplicates($insert) + $batch = $this->onDuplicate !== OnDuplicate::Fail + ? $this->adapter->withOnDuplicate($this->onDuplicate, $insert) : $insert(); $batch = $this->adapter->getSequences($collection->getId(), $batch); diff --git a/src/Database/Mirror.php b/src/Database/Mirror.php index 86beb5d0a..cbc099a9c 100644 --- a/src/Database/Mirror.php +++ b/src/Database/Mirror.php @@ -601,8 +601,9 @@ public function createDocuments( ?callable $onNext = null, ?callable $onError = null, ): int { - $modified = $this->skipDuplicates - ? $this->source->skipDuplicates( + $modified = $this->onDuplicate !== OnDuplicate::Fail + ? $this->source->withOnDuplicate( + $this->onDuplicate, fn () => $this->source->createDocuments($collection, $documents, $batchSize, $onNext, $onError) ) : $this->source->createDocuments($collection, $documents, $batchSize, $onNext, $onError); @@ -638,8 +639,9 @@ public function createDocuments( $clones[] = $clone; } - if ($this->skipDuplicates) { - $this->destination->skipDuplicates( + if ($this->onDuplicate !== OnDuplicate::Fail) { + $this->destination->withOnDuplicate( + $this->onDuplicate, fn () => $this->destination->withPreserveDates( fn () => $this->destination->createDocuments($collection, $clones, $batchSize) ) diff --git a/src/Database/OnDuplicate.php b/src/Database/OnDuplicate.php new file mode 100644 index 000000000..21984f705 --- /dev/null +++ b/src/Database/OnDuplicate.php @@ -0,0 +1,27 @@ + + */ + public static function values(): array + { + return \array_map(fn (self $case) => $case->value, self::cases()); + } +} diff --git a/tests/e2e/Adapter/MirrorTest.php b/tests/e2e/Adapter/MirrorTest.php index de73d7be8..4e0a1a3f5 100644 --- a/tests/e2e/Adapter/MirrorTest.php +++ b/tests/e2e/Adapter/MirrorTest.php @@ -13,6 +13,7 @@ use Utopia\Database\Exception\Conflict; use Utopia\Database\Exception\Duplicate; use Utopia\Database\Exception\Limit; +use Utopia\Database\OnDuplicate; use Utopia\Database\Exception\Structure; use Utopia\Database\Helpers\Permission; use Utopia\Database\Helpers\Role; @@ -351,7 +352,7 @@ public function testCreateDocumentsSkipDuplicatesBackfillsDestination(): void $database->getDestination()->getDocument($collection, 'dup')->isEmpty() ); - $database->skipDuplicates(fn () => $database->createDocuments($collection, [ + $database->withOnDuplicate(OnDuplicate::Skip, fn () => $database->createDocuments($collection, [ new Document([ '$id' => 'dup', 'name' => 'WouldBe', diff --git a/tests/e2e/Adapter/Scopes/DocumentTests.php b/tests/e2e/Adapter/Scopes/DocumentTests.php index c45610c28..8f870827f 100644 --- a/tests/e2e/Adapter/Scopes/DocumentTests.php +++ b/tests/e2e/Adapter/Scopes/DocumentTests.php @@ -20,6 +20,7 @@ use Utopia\Database\Helpers\ID; use Utopia\Database\Helpers\Permission; use Utopia\Database\Helpers\Role; +use Utopia\Database\OnDuplicate; use Utopia\Database\Query; trait DocumentTests @@ -7904,7 +7905,7 @@ public function testCreateDocumentsIgnoreDuplicates(): void // With skipDuplicates, duplicates should be silently skipped $emittedIds = []; $collection = __FUNCTION__; - $count = $database->skipDuplicates(function () use ($database, $collection, &$emittedIds) { + $count = $database->withOnDuplicate(OnDuplicate::Skip,function () use ($database, $collection, &$emittedIds) { return $database->createDocuments($collection, [ new Document([ '$id' => 'doc1', @@ -7966,7 +7967,7 @@ public function testCreateDocumentsIgnoreAllDuplicates(): void // With skipDuplicates, inserting only duplicates should succeed with no new rows $emittedIds = []; $collection = __FUNCTION__; - $count = $database->skipDuplicates(function () use ($database, $collection, &$emittedIds) { + $count = $database->withOnDuplicate(OnDuplicate::Skip,function () use ($database, $collection, &$emittedIds) { return $database->createDocuments($collection, [ new Document([ '$id' => 'existing', @@ -8000,7 +8001,7 @@ public function testCreateDocumentsSkipDuplicatesEmptyBatch(): void $database->createCollection($collection); $database->createAttribute($collection, 'name', Database::VAR_STRING, 128, true); - $count = $database->skipDuplicates(fn () => $database->createDocuments($collection, [])); + $count = $database->withOnDuplicate(OnDuplicate::Skip,fn () => $database->createDocuments($collection, [])); $this->assertSame(0, $count); $this->assertCount(0, $database->find($collection)); @@ -8029,9 +8030,9 @@ public function testCreateDocumentsSkipDuplicatesNestedScope(): void // Nested scope — inner scope runs inside outer scope. // After inner exits, outer state should still be "skip enabled". // After outer exits, state should restore to "skip disabled". - $countOuter = $database->skipDuplicates(function () use ($database, $collection, $makeDoc) { + $countOuter = $database->withOnDuplicate(OnDuplicate::Skip,function () use ($database, $collection, $makeDoc) { // Inner scope: add dup + new - $countInner = $database->skipDuplicates(function () use ($database, $collection, $makeDoc) { + $countInner = $database->withOnDuplicate(OnDuplicate::Skip,function () use ($database, $collection, $makeDoc) { return $database->createDocuments($collection, [ $makeDoc('seed', 'Dup'), $makeDoc('innerNew', 'InnerNew'), @@ -8101,7 +8102,7 @@ public function testCreateDocumentsSkipDuplicatesLargeBatch(): void } $emittedIds = []; - $count = $database->skipDuplicates(function () use ($database, $collection, $batch, &$emittedIds) { + $count = $database->withOnDuplicate(OnDuplicate::Skip,function () use ($database, $collection, $batch, &$emittedIds) { return $database->createDocuments($collection, $batch, onNext: function (Document $doc) use (&$emittedIds) { $emittedIds[] = $doc->getId(); }); @@ -8141,13 +8142,13 @@ public function testCreateDocumentsSkipDuplicatesSecondCallSkipsAll(): void ); // First call — all new - $firstCount = $database->skipDuplicates( + $firstCount = $database->withOnDuplicate(OnDuplicate::Skip, fn () => $database->createDocuments($collection, $makeBatch('First')) ); $this->assertSame(3, $firstCount); $emittedIds = []; - $secondCount = $database->skipDuplicates(function () use ($database, $collection, $makeBatch, &$emittedIds) { + $secondCount = $database->withOnDuplicate(OnDuplicate::Skip,function () use ($database, $collection, $makeBatch, &$emittedIds) { return $database->createDocuments($collection, $makeBatch('Second'), onNext: function (Document $doc) use (&$emittedIds) { $emittedIds[] = $doc->getId(); }); @@ -8237,7 +8238,7 @@ public function testCreateDocumentsSkipDuplicatesRelationships(): void ]), ]; - $database->skipDuplicates(fn () => $database->createDocuments($parent, $batch)); + $database->withOnDuplicate(OnDuplicate::Skip,fn () => $database->createDocuments($parent, $batch)); $existing = $database->getDocument($parent, 'existingParent'); $this->assertFalse($existing->isEmpty()); From 306cd1e7949f0c34e2c4616495179c5b82b1a570 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Tue, 21 Apr 2026 11:00:46 +0100 Subject: [PATCH 02/10] Adapter row Upsert: emit ON DUPLICATE KEY UPDATE / ON CONFLICT DO UPDATE / bulkWrite $set - SQL.getInsertSuffix now accepts column list so Upsert can generate SET clause - MariaDB override: 'ON DUPLICATE KEY UPDATE col = VALUES(col), ...' (skipping _uid/_id keys) - Postgres override: extends ON CONFLICT branch to DO UPDATE SET using EXCLUDED.col (skipping _uid/_id/_tenant conflict keys) - SQLite: already correct via INSERT OR REPLACE keyword (no suffix change) - Mongo bulk createDocuments: Upsert uses $set operator instead of $setOnInsert, making matched documents reflect incoming values; Skip keeps $setOnInsert semantics (insert-only no-op) Single-row createDocument upsert path + schema-level (createCollection / createAttribute / createIndex) dispatch remain for the next commit. --- src/Database/Adapter/MariaDB.php | 30 ++++++++++++++++++++++++++++++ src/Database/Adapter/Mongo.php | 17 ++++++++++------- src/Database/Adapter/Postgres.php | 26 ++++++++++++++++++++++---- src/Database/Adapter/SQL.php | 14 +++++++++----- 4 files changed, 71 insertions(+), 16 deletions(-) diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index 223f91e71..e881dcb4e 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -16,6 +16,7 @@ use Utopia\Database\Exception\Timeout as TimeoutException; use Utopia\Database\Exception\Truncate as TruncateException; use Utopia\Database\Helpers\ID; +use Utopia\Database\OnDuplicate; use Utopia\Database\Operator; use Utopia\Database\Query; @@ -2376,4 +2377,33 @@ public function getSupportForTTLIndexes(): bool { return false; } + + /** + * MariaDB/MySQL Upsert: append `ON DUPLICATE KEY UPDATE col = VALUES(col), ...` + * so the INSERT replaces the matched row's columns instead of throwing. + * + * @param array $columns + */ + protected function getInsertSuffix(string $table, array $columns = []): string + { + if ($this->onDuplicate !== OnDuplicate::Upsert || empty($columns)) { + return ''; + } + + $assignments = []; + foreach ($columns as $col) { + // Skip the primary unique key (_uid) — no need to "update" what matched. + // Safe to skip _id too since it's auto-increment in INSERT and untouched on update. + if (\in_array($col, ['`_uid`', '`_id`'], true)) { + continue; + } + $assignments[] = "{$col} = VALUES({$col})"; + } + + if (empty($assignments)) { + return ''; + } + + return 'ON DUPLICATE KEY UPDATE ' . \implode(', ', $assignments); + } } diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index 72cd524c7..9a36e3695 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -1499,12 +1499,15 @@ public function createDocuments(Document $collection, array $documents): array $records[] = $record; } - // insertMany aborts the txn on any duplicate; upsert + $setOnInsert no-ops instead. - if ($this->onDuplicate === OnDuplicate::Skip) { + // insertMany aborts the txn on any duplicate; Mongo's upsert path handles + // both Skip ($setOnInsert: insert-only no-op) and Upsert ($set: overwrite). + if ($this->onDuplicate !== OnDuplicate::Fail) { if (empty($records)) { return []; } + $operator = $this->onDuplicate === OnDuplicate::Upsert ? '$set' : '$setOnInsert'; + $operations = []; foreach ($records as $record) { $filter = ['_uid' => $record['_uid'] ?? '']; @@ -1512,17 +1515,17 @@ public function createDocuments(Document $collection, array $documents): array $filter['_tenant'] = $record['_tenant'] ?? $this->getTenant(); } - // Filter fields can't reappear in $setOnInsert (mongo path-conflict error). - $setOnInsert = $record; - unset($setOnInsert['_uid'], $setOnInsert['_tenant']); + // Filter fields can't reappear in $setOnInsert/$set (mongo path-conflict error). + $payload = $record; + unset($payload['_uid'], $payload['_tenant']); - if (empty($setOnInsert)) { + if (empty($payload)) { continue; } $operations[] = [ 'filter' => $filter, - 'update' => ['$setOnInsert' => $setOnInsert], + 'update' => [$operator => $payload], ]; } diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index d705ca9b7..65b1d2866 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -2356,7 +2356,10 @@ protected function getInsertKeyword(): string return 'INSERT INTO'; } - protected function getInsertSuffix(string $table): string + /** + * @param array $columns + */ + protected function getInsertSuffix(string $table, array $columns = []): string { if ($this->onDuplicate === OnDuplicate::Fail) { return ''; @@ -2364,9 +2367,24 @@ protected function getInsertSuffix(string $table): string $conflictTarget = $this->sharedTables ? '("_uid", "_tenant")' : '("_uid")'; - // Upsert on Postgres uses ON CONFLICT DO UPDATE; for now Skip and Upsert - // both DO NOTHING until upsert SET clause generation is implemented. - return "ON CONFLICT {$conflictTarget} DO NOTHING"; + if ($this->onDuplicate === OnDuplicate::Skip) { + return "ON CONFLICT {$conflictTarget} DO NOTHING"; + } + + // Upsert: DO UPDATE SET col = EXCLUDED.col for every column except the conflict key. + $assignments = []; + foreach ($columns as $col) { + if (\in_array($col, ['"_uid"', '"_id"', '"_tenant"'], true)) { + continue; + } + $assignments[] = "{$col} = EXCLUDED.{$col}"; + } + + if (empty($assignments)) { + return "ON CONFLICT {$conflictTarget} DO NOTHING"; + } + + return "ON CONFLICT {$conflictTarget} DO UPDATE SET " . \implode(', ', $assignments); } protected function getInsertPermissionsSuffix(): string diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index e734b9304..ae50c6a60 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -1047,8 +1047,12 @@ protected function getInsertKeyword(): string /** * Returns a suffix appended after VALUES clause for duplicate handling. * Override in adapter subclasses (e.g., Postgres uses ON CONFLICT DO NOTHING). + * + * @param string $table table name (without namespace prefix) + * @param array $columns quoted column names present in the INSERT — needed + * to emit ON DUPLICATE KEY UPDATE / ON CONFLICT DO UPDATE SET clauses */ - protected function getInsertSuffix(string $table): string + protected function getInsertSuffix(string $table, array $columns = []): string { return ''; } @@ -2539,12 +2543,12 @@ public function createDocuments(Document $collection, array $documents): array $attributeKeys[] = '_tenant'; } - $columns = []; + $columnList = []; foreach ($attributeKeys as $key => $attribute) { - $columns[$key] = $this->quote($this->filter($attribute)); + $columnList[$key] = $this->quote($this->filter($attribute)); } - $columns = '(' . \implode(', ', $columns) . ')'; + $columns = '(' . \implode(', ', $columnList) . ')'; $bindIndex = 0; $batchKeys = []; @@ -2609,7 +2613,7 @@ public function createDocuments(Document $collection, array $documents): array $stmt = $this->getPDO()->prepare(" {$this->getInsertKeyword()} {$this->getSQLTable($name)} {$columns} VALUES {$batchKeys} - {$this->getInsertSuffix($name)} + {$this->getInsertSuffix($name, $columnList)} "); foreach ($bindValues as $key => $value) { From 7d2458f2a3cb59e04dd95997c0a692952fc9d655 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Tue, 21 Apr 2026 11:36:56 +0100 Subject: [PATCH 03/10] Adapter schema-level dispatch: IF NOT EXISTS / tolerate-duplicate on Skip/Upsert MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - createCollection: * MariaDB/Postgres/SQLite: emit CREATE TABLE IF NOT EXISTS (and matching CREATE INDEX IF NOT EXISTS for Postgres internal indexes) when onDuplicate is Skip or Upsert; keep plain CREATE TABLE for Fail * Mongo: extend the existing shared-tables/metadata 'skip if exists' branch to also apply when onDuplicate is Skip or Upsert - createAttribute (SQL base): catch DuplicateException, return true in Skip/Upsert. Type/size changes go through updateAttribute — auto-applying them during re-migration is unsafe. - createIndex: * MariaDB: catch DuplicateException in Skip/Upsert (MySQL doesn't support IF NOT EXISTS on ALTER TABLE ADD INDEX) * Postgres: emit CREATE INDEX IF NOT EXISTS natively * SQLite: already has sqlite_master pre-check - Upsert on schema resources behaves the same as Skip (don't auto-modify existing schema). Upsert semantics apply to rows only. --- src/Database/Adapter/MariaDB.php | 17 +++++++++--- src/Database/Adapter/Mongo.php | 25 ++++++++++++------ src/Database/Adapter/Postgres.php | 43 +++++++++++++++++++++---------- src/Database/Adapter/SQL.php | 8 +++++- src/Database/Adapter/SQLite.php | 8 ++++-- 5 files changed, 73 insertions(+), 28 deletions(-) diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index e881dcb4e..dbb0ffda6 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -158,8 +158,12 @@ public function createCollection(string $name, array $attributes = [], array $in $indexStrings[$key] = "{$indexType} `{$indexId}` ({$indexAttributes}),"; } + $createKeyword = $this->onDuplicate !== OnDuplicate::Fail + ? 'CREATE TABLE IF NOT EXISTS' + : 'CREATE TABLE'; + $collection = " - CREATE TABLE {$this->getSQLTable($id)} ( + {$createKeyword} {$this->getSQLTable($id)} ( _id BIGINT UNSIGNED NOT NULL AUTO_INCREMENT, _uid VARCHAR(255) NOT NULL, _createdAt DATETIME(3) DEFAULT NULL, @@ -190,7 +194,7 @@ public function createCollection(string $name, array $attributes = [], array $in $collection = $this->trigger(Database::EVENT_COLLECTION_CREATE, $collection); $permissions = " - CREATE TABLE {$this->getSQLTable($id . '_perms')} ( + {$createKeyword} {$this->getSQLTable($id . '_perms')} ( _id BIGINT UNSIGNED NOT NULL AUTO_INCREMENT, _type VARCHAR(12) NOT NULL, _permission VARCHAR(255) NOT NULL, @@ -778,7 +782,14 @@ public function createIndex(string $collection, string $id, string $type, array ->prepare($sql) ->execute(); } catch (PDOException $e) { - throw $this->processException($e); + $err = $this->processException($e); + // Skip/Upsert: tolerate an existing index. Spec changes (attributes, type, + // uniqueness) must go through deleteIndex+createIndex — auto-modifying + // indexes risks long rebuilds and uniqueness violations mid-migration. + if ($err instanceof DuplicateException && $this->onDuplicate !== OnDuplicate::Fail) { + return true; + } + throw $err; } } diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index 9a36e3695..2b16f401e 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -431,10 +431,14 @@ public function createCollection(string $name, array $attributes = [], array $in { $id = $this->getNamespace() . '_' . $this->filter($name); - // In shared-tables mode or for metadata, the physical collection may - // already exist for another tenant. Return early to avoid a - // "Collection Exists" exception from the client. - if (!$this->inTransaction && ($this->getSharedTables() || $name === Database::METADATA) && $this->exists($this->getNamespace(), $name)) { + // In shared-tables mode, for metadata, or when the caller opted into + // Skip/Upsert, the physical collection may already exist. Return early + // to avoid a "Collection Exists" exception from the client. + $tolerateExisting = $this->getSharedTables() + || $name === Database::METADATA + || $this->onDuplicate !== OnDuplicate::Fail; + + if (!$this->inTransaction && $tolerateExisting && $this->exists($this->getNamespace(), $name)) { return true; } @@ -445,14 +449,19 @@ public function createCollection(string $name, array $attributes = [], array $in } catch (MongoException $e) { $e = $this->processException($e); if ($e instanceof DuplicateException) { + // Also tolerate client-reported duplicates in Skip/Upsert mode. + if ($tolerateExisting) { + return true; + } + // Keep existing shared-tables/metadata behavior — no-op there. return true; } // Client throws code-0 "Collection Exists" when its pre-check - // finds the collection. In shared-tables/metadata context this - // is a no-op; otherwise re-throw as DuplicateException so - // Database::createCollection() can run orphan reconciliation. + // finds the collection. Tolerated contexts no-op; otherwise re-throw + // as DuplicateException so Database::createCollection() can run + // orphan reconciliation. if ($e->getCode() === 0 && stripos($e->getMessage(), 'Collection Exists') !== false) { - if ($this->getSharedTables() || $name === Database::METADATA) { + if ($tolerateExisting) { return true; } throw new DuplicateException('Collection already exists', $e->getCode(), $e); diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 65b1d2866..13a346aad 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -242,8 +242,19 @@ public function createCollection(string $name, array $attributes = [], array $in } $sqlTenant = $this->sharedTables ? '_tenant INTEGER DEFAULT NULL,' : ''; + + $createTable = $this->onDuplicate !== OnDuplicate::Fail + ? 'CREATE TABLE IF NOT EXISTS' + : 'CREATE TABLE'; + $createIndex = $this->onDuplicate !== OnDuplicate::Fail + ? 'CREATE INDEX IF NOT EXISTS' + : 'CREATE INDEX'; + $createUniqueIndex = $this->onDuplicate !== OnDuplicate::Fail + ? 'CREATE UNIQUE INDEX IF NOT EXISTS' + : 'CREATE UNIQUE INDEX'; + $collection = " - CREATE TABLE {$this->getSQLTable($id)} ( + {$createTable} {$this->getSQLTable($id)} ( _id BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, _uid VARCHAR(255) NOT NULL, " . $sqlTenant . " @@ -260,26 +271,26 @@ public function createCollection(string $name, array $attributes = [], array $in $updatedIndex = $this->getShortKey("{$namespace}_{$this->tenant}_{$id}_updated"); $tenantIdIndex = $this->getShortKey("{$namespace}_{$this->tenant}_{$id}_tenant_id"); $collection .= " - CREATE UNIQUE INDEX \"{$uidIndex}\" ON {$this->getSQLTable($id)} (\"_uid\" COLLATE utf8_ci_ai, \"_tenant\"); - CREATE INDEX \"{$createdIndex}\" ON {$this->getSQLTable($id)} (_tenant, \"_createdAt\"); - CREATE INDEX \"{$updatedIndex}\" ON {$this->getSQLTable($id)} (_tenant, \"_updatedAt\"); - CREATE INDEX \"{$tenantIdIndex}\" ON {$this->getSQLTable($id)} (_tenant, _id); + {$createUniqueIndex} \"{$uidIndex}\" ON {$this->getSQLTable($id)} (\"_uid\" COLLATE utf8_ci_ai, \"_tenant\"); + {$createIndex} \"{$createdIndex}\" ON {$this->getSQLTable($id)} (_tenant, \"_createdAt\"); + {$createIndex} \"{$updatedIndex}\" ON {$this->getSQLTable($id)} (_tenant, \"_updatedAt\"); + {$createIndex} \"{$tenantIdIndex}\" ON {$this->getSQLTable($id)} (_tenant, _id); "; } else { $uidIndex = $this->getShortKey("{$namespace}_{$id}_uid"); $createdIndex = $this->getShortKey("{$namespace}_{$id}_created"); $updatedIndex = $this->getShortKey("{$namespace}_{$id}_updated"); $collection .= " - CREATE UNIQUE INDEX \"{$uidIndex}\" ON {$this->getSQLTable($id)} (\"_uid\" COLLATE utf8_ci_ai); - CREATE INDEX \"{$createdIndex}\" ON {$this->getSQLTable($id)} (\"_createdAt\"); - CREATE INDEX \"{$updatedIndex}\" ON {$this->getSQLTable($id)} (\"_updatedAt\"); + {$createUniqueIndex} \"{$uidIndex}\" ON {$this->getSQLTable($id)} (\"_uid\" COLLATE utf8_ci_ai); + {$createIndex} \"{$createdIndex}\" ON {$this->getSQLTable($id)} (\"_createdAt\"); + {$createIndex} \"{$updatedIndex}\" ON {$this->getSQLTable($id)} (\"_updatedAt\"); "; } $collection = $this->trigger(Database::EVENT_COLLECTION_CREATE, $collection); $permissions = " - CREATE TABLE {$this->getSQLTable($id . '_perms')} ( + {$createTable} {$this->getSQLTable($id . '_perms')} ( _id BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, _tenant INTEGER DEFAULT NULL, _type VARCHAR(12) NOT NULL, @@ -292,18 +303,18 @@ public function createCollection(string $name, array $attributes = [], array $in $uniquePermissionIndex = $this->getShortKey("{$namespace}_{$this->tenant}_{$id}_ukey"); $permissionIndex = $this->getShortKey("{$namespace}_{$this->tenant}_{$id}_permission"); $permissions .= " - CREATE UNIQUE INDEX \"{$uniquePermissionIndex}\" + {$createUniqueIndex} \"{$uniquePermissionIndex}\" ON {$this->getSQLTable($id . '_perms')} USING btree (_tenant,_document,_type,_permission); - CREATE INDEX \"{$permissionIndex}\" + {$createIndex} \"{$permissionIndex}\" ON {$this->getSQLTable($id . '_perms')} USING btree (_tenant,_permission,_type); "; } else { $uniquePermissionIndex = $this->getShortKey("{$namespace}_{$id}_ukey"); $permissionIndex = $this->getShortKey("{$namespace}_{$id}_permission"); $permissions .= " - CREATE UNIQUE INDEX \"{$uniquePermissionIndex}\" + {$createUniqueIndex} \"{$uniquePermissionIndex}\" ON {$this->getSQLTable($id . '_perms')} USING btree (_document COLLATE utf8_ci_ai,_type,_permission); - CREATE INDEX \"{$permissionIndex}\" + {$createIndex} \"{$permissionIndex}\" ON {$this->getSQLTable($id . '_perms')} USING btree (_permission,_type); "; } @@ -935,7 +946,11 @@ public function createIndex(string $collection, string $id, string $type, array $attributes = "_tenant, {$attributes}"; } - $sql = "CREATE {$sqlType} \"{$keyName}\" ON {$this->getSQLTable($collection)}"; + $createClause = $this->onDuplicate !== OnDuplicate::Fail + ? "CREATE {$sqlType} IF NOT EXISTS" + : "CREATE {$sqlType}"; + + $sql = "{$createClause} \"{$keyName}\" ON {$this->getSQLTable($collection)}"; // Add USING clause for special index types $sql .= match ($type) { diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index ae50c6a60..a092196d2 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -256,7 +256,13 @@ public function createAttribute(string $collection, string $id, string $type, in ->prepare($sql) ->execute(); } catch (PDOException $e) { - throw $this->processException($e); + $err = $this->processException($e); + // Skip/Upsert: tolerate an existing column. Schema evolution (type/size + // changes) must go through updateAttribute — too risky to auto-apply. + if ($err instanceof DuplicateException && $this->onDuplicate !== OnDuplicate::Fail) { + return true; + } + throw $err; } } diff --git a/src/Database/Adapter/SQLite.php b/src/Database/Adapter/SQLite.php index fdf5a4d02..c1b919680 100644 --- a/src/Database/Adapter/SQLite.php +++ b/src/Database/Adapter/SQLite.php @@ -165,8 +165,12 @@ public function createCollection(string $name, array $attributes = [], array $in $tenantQuery = $this->sharedTables ? '`_tenant` INTEGER DEFAULT NULL,' : ''; + $createTable = $this->onDuplicate !== OnDuplicate::Fail + ? 'CREATE TABLE IF NOT EXISTS' + : 'CREATE TABLE'; + $collection = " - CREATE TABLE {$this->getSQLTable($id)} ( + {$createTable} {$this->getSQLTable($id)} ( `_id` INTEGER PRIMARY KEY AUTOINCREMENT, `_uid` VARCHAR(36) NOT NULL, {$tenantQuery} @@ -180,7 +184,7 @@ public function createCollection(string $name, array $attributes = [], array $in $collection = $this->trigger(Database::EVENT_COLLECTION_CREATE, $collection); $permissions = " - CREATE TABLE {$this->getSQLTable($id . '_perms')} ( + {$createTable} {$this->getSQLTable($id . '_perms')} ( `_id` INTEGER PRIMARY KEY AUTOINCREMENT, {$tenantQuery} `_type` VARCHAR(12) NOT NULL, From 399e83c4066f35a291d83ceab4e49a989d743f78 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Tue, 21 Apr 2026 12:45:15 +0100 Subject: [PATCH 04/10] Adapter permissions-keyword split + SQLite suffix neutralizer + Upsert tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - SQL base: add getInsertPermissionsKeyword() — keeps INSERT IGNORE on Skip/Upsert for the `_perms` side-table so row-Upsert doesn't try to DO-UPDATE-SET permissions (they already match on the composite unique key) - SQLite override: return INSERT OR IGNORE INTO for Skip/Upsert, and override getInsertSuffix to return empty — SQLite realizes Upsert via the `INSERT OR REPLACE` keyword and must not inherit MariaDB's `ON DUPLICATE KEY UPDATE` suffix (syntax error on SQLite) - Postgres override: return plain INSERT INTO for permissions; conflict handling goes through the ON CONFLICT suffix, not INSERT IGNORE (syntax error on Postgres) - tests/DocumentTests: add testCreateDocsUpsertOverwrites and testCreateDocsUpsertAll covering the happy paths; refresh three stale `skipDuplicates` comments to reference OnDuplicate::Skip - Mirror / MirrorTest: refresh stale `skipDuplicates` comment wording --- src/Database/Adapter/Postgres.php | 12 +++ src/Database/Adapter/SQL.php | 16 +++- src/Database/Adapter/SQLite.php | 18 +++++ src/Database/Mirror.php | 5 +- tests/e2e/Adapter/MirrorTest.php | 2 +- tests/e2e/Adapter/Scopes/DocumentTests.php | 88 +++++++++++++++++++++- 6 files changed, 134 insertions(+), 7 deletions(-) diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 13a346aad..2467c5214 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -2367,6 +2367,18 @@ public function getSupportForOptionalSpatialAttributeWithExistingRows(): bool } protected function getInsertKeyword(): string + { + // Postgres doesn't have INSERT IGNORE — Skip/Upsert are realized via + // the ON CONFLICT suffix (see getInsertSuffix). + return 'INSERT INTO'; + } + + /** + * Postgres permissions insert uses ON CONFLICT DO NOTHING via suffix; the + * keyword stays plain INSERT INTO. Override MariaDB's INSERT IGNORE (no-op + * in MySQL dialect, a syntax error in Postgres). + */ + protected function getInsertPermissionsKeyword(): string { return 'INSERT INTO'; } diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index a092196d2..0c9bf671a 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -1050,6 +1050,20 @@ protected function getInsertKeyword(): string }; } + /** + * Returns the INSERT keyword for the `_perms` side-table. Permissions have + * their own composite unique constraint (_document, _type, _permission), + * so on row Upsert we don't want to ON-DUPLICATE-KEY-UPDATE them — they're + * already there. Both Skip and Upsert modes should just silently ignore + * pre-existing permission rows. + */ + protected function getInsertPermissionsKeyword(): string + { + return $this->onDuplicate === OnDuplicate::Fail + ? 'INSERT INTO' + : 'INSERT IGNORE INTO'; + } + /** * Returns a suffix appended after VALUES clause for duplicate handling. * Override in adapter subclasses (e.g., Postgres uses ON CONFLICT DO NOTHING). @@ -2633,7 +2647,7 @@ public function createDocuments(Document $collection, array $documents): array $permissions = \implode(', ', $permissions); $sqlPermissions = " - {$this->getInsertKeyword()} {$this->getSQLTable($name . '_perms')} (_type, _permission, _document {$tenantColumn}) + {$this->getInsertPermissionsKeyword()} {$this->getSQLTable($name . '_perms')} (_type, _permission, _document {$tenantColumn}) VALUES {$permissions} {$this->getInsertPermissionsSuffix()} "; diff --git a/src/Database/Adapter/SQLite.php b/src/Database/Adapter/SQLite.php index c1b919680..d609fe6b4 100644 --- a/src/Database/Adapter/SQLite.php +++ b/src/Database/Adapter/SQLite.php @@ -1950,4 +1950,22 @@ protected function getInsertKeyword(): string OnDuplicate::Fail => 'INSERT INTO', }; } + + protected function getInsertPermissionsKeyword(): string + { + return $this->onDuplicate === OnDuplicate::Fail + ? 'INSERT INTO' + : 'INSERT OR IGNORE INTO'; + } + + /** + * SQLite realizes Upsert via the `INSERT OR REPLACE` keyword, so no SUFFIX + * clause is needed. Override MariaDB's `ON DUPLICATE KEY UPDATE` suffix. + * + * @param array $columns + */ + protected function getInsertSuffix(string $table, array $columns = []): string + { + return ''; + } } diff --git a/src/Database/Mirror.php b/src/Database/Mirror.php index cbc099a9c..3c704170e 100644 --- a/src/Database/Mirror.php +++ b/src/Database/Mirror.php @@ -622,8 +622,9 @@ public function createDocuments( // Forward every input to destination. "upgraded" status means the schema // is mirrored, not that every row is backfilled, so a row that is a - // duplicate on source may not yet exist on destination. In skipDuplicates - // mode the destination runs its own INSERT IGNORE and decides per-row. + // duplicate on source may not yet exist on destination. Under + // OnDuplicate::Skip/Upsert the destination runs its own dialect-specific + // conflict handling and decides per-row. try { $clones = []; foreach ($documents as $document) { diff --git a/tests/e2e/Adapter/MirrorTest.php b/tests/e2e/Adapter/MirrorTest.php index 4e0a1a3f5..15e823c88 100644 --- a/tests/e2e/Adapter/MirrorTest.php +++ b/tests/e2e/Adapter/MirrorTest.php @@ -332,7 +332,7 @@ public function testCreateDocumentsSkipDuplicatesBackfillsDestination(): void ], documentSecurity: false); // Seed the SOURCE only (bypass the mirror) with the row we want to - // skipDuplicates over later. Destination intentionally does NOT have it — + // apply OnDuplicate::Skip over later. Destination intentionally does NOT have it — // this simulates an in-flight backfill where the collection is marked // 'upgraded' (schema mirrored) but not every row has reached destination. $database->getSource()->createDocument($collection, new Document([ diff --git a/tests/e2e/Adapter/Scopes/DocumentTests.php b/tests/e2e/Adapter/Scopes/DocumentTests.php index 8f870827f..c3e30d95b 100644 --- a/tests/e2e/Adapter/Scopes/DocumentTests.php +++ b/tests/e2e/Adapter/Scopes/DocumentTests.php @@ -7902,7 +7902,7 @@ public function testCreateDocumentsIgnoreDuplicates(): void $this->assertNotEmpty($e->getMessage()); } - // With skipDuplicates, duplicates should be silently skipped + // With OnDuplicate::Skip, duplicates should be silently skipped $emittedIds = []; $collection = __FUNCTION__; $count = $database->withOnDuplicate(OnDuplicate::Skip,function () use ($database, $collection, &$emittedIds) { @@ -7964,7 +7964,7 @@ public function testCreateDocumentsIgnoreAllDuplicates(): void ]), ]); - // With skipDuplicates, inserting only duplicates should succeed with no new rows + // With OnDuplicate::Skip, inserting only duplicates should succeed with no new rows $emittedIds = []; $collection = __FUNCTION__; $count = $database->withOnDuplicate(OnDuplicate::Skip,function () use ($database, $collection, &$emittedIds) { @@ -8087,7 +8087,7 @@ public function testCreateDocumentsSkipDuplicatesLargeBatch(): void } $database->createDocuments($collection, $seed); - // Now call skipDuplicates with 300 docs: 50 existing (0-49) + 250 new (50-299). + // Now call with OnDuplicate::Skip and 300 docs: 50 existing (0-49) + 250 new (50-299). // 300 > default INSERT_BATCH_SIZE, so this exercises the chunk loop. $batch = []; for ($i = 0; $i < 300; $i++) { @@ -8261,4 +8261,86 @@ public function testCreateDocumentsSkipDuplicatesRelationships(): void \sort($allChildIds); $this->assertSame(['existingChild', 'newChild', 'retryChild'], $allChildIds); } + + /** + * OnDuplicate::Upsert — existing rows are overwritten with the incoming + * values; new rows are inserted. The returned count reflects every input. + */ + public function testCreateDocsUpsertOverwrites(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + $database->createCollection(__FUNCTION__); + $database->createAttribute(__FUNCTION__, 'name', Database::VAR_STRING, 128, true); + $database->createAttribute(__FUNCTION__, 'tag', Database::VAR_STRING, 128, false); + + $permissions = [ + Permission::read(Role::any()), + Permission::create(Role::any()), + Permission::update(Role::any()), + ]; + + // Seed two docs. + $database->createDocuments(__FUNCTION__, [ + new Document(['$id' => 'a', 'name' => 'original-A', 'tag' => 'keep', '$permissions' => $permissions]), + new Document(['$id' => 'b', 'name' => 'original-B', 'tag' => 'keep', '$permissions' => $permissions]), + ]); + + // Upsert: overwrite 'a', leave 'b' untouched (not in batch), insert 'c'. + $collection = __FUNCTION__; + $count = $database->withOnDuplicate(OnDuplicate::Upsert, function () use ($database, $collection, $permissions) { + return $database->createDocuments($collection, [ + new Document(['$id' => 'a', 'name' => 'replaced-A', 'tag' => 'new', '$permissions' => $permissions]), + new Document(['$id' => 'c', 'name' => 'inserted-C', 'tag' => 'new', '$permissions' => $permissions]), + ]); + }); + $this->assertSame(2, $count); + + $docs = $database->find(__FUNCTION__, [Query::orderAsc('$id')]); + $this->assertCount(3, $docs); + $this->assertSame('replaced-A', $docs[0]->getAttribute('name')); + $this->assertSame('new', $docs[0]->getAttribute('tag')); + $this->assertSame('original-B', $docs[1]->getAttribute('name')); + $this->assertSame('keep', $docs[1]->getAttribute('tag')); + $this->assertSame('inserted-C', $docs[2]->getAttribute('name')); + } + + /** + * OnDuplicate::Upsert — a batch composed entirely of duplicates overwrites + * every existing row; zero rows are skipped. + */ + public function testCreateDocsUpsertAll(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + $database->createCollection(__FUNCTION__); + $database->createAttribute(__FUNCTION__, 'name', Database::VAR_STRING, 128, true); + + $permissions = [ + Permission::read(Role::any()), + Permission::create(Role::any()), + Permission::update(Role::any()), + ]; + + $database->createDocuments(__FUNCTION__, [ + new Document(['$id' => 'x', 'name' => 'v1', '$permissions' => $permissions]), + new Document(['$id' => 'y', 'name' => 'v1', '$permissions' => $permissions]), + ]); + + $collection = __FUNCTION__; + $count = $database->withOnDuplicate(OnDuplicate::Upsert, function () use ($database, $collection, $permissions) { + return $database->createDocuments($collection, [ + new Document(['$id' => 'x', 'name' => 'v2', '$permissions' => $permissions]), + new Document(['$id' => 'y', 'name' => 'v2', '$permissions' => $permissions]), + ]); + }); + $this->assertSame(2, $count); + + $docs = $database->find(__FUNCTION__, [Query::orderAsc('$id')]); + $this->assertCount(2, $docs); + $this->assertSame('v2', $docs[0]->getAttribute('name')); + $this->assertSame('v2', $docs[1]->getAttribute('name')); + } } From 9fcb94e809a151dca272fe08c11eaae4ca50468f Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Tue, 21 Apr 2026 13:13:14 +0100 Subject: [PATCH 05/10] Lint: fix ordered_imports (MirrorTest) + method_argument_space (DocumentTests) Pint violations flagged by CI: - MirrorTest: OnDuplicate import alphabetized after Mirror - DocumentTests: add missing space after ',' in withOnDuplicate(...) calls --- tests/e2e/Adapter/MirrorTest.php | 2 +- tests/e2e/Adapter/Scopes/DocumentTests.php | 19 ++++++++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/tests/e2e/Adapter/MirrorTest.php b/tests/e2e/Adapter/MirrorTest.php index 15e823c88..706fa4ee3 100644 --- a/tests/e2e/Adapter/MirrorTest.php +++ b/tests/e2e/Adapter/MirrorTest.php @@ -13,11 +13,11 @@ use Utopia\Database\Exception\Conflict; use Utopia\Database\Exception\Duplicate; use Utopia\Database\Exception\Limit; -use Utopia\Database\OnDuplicate; use Utopia\Database\Exception\Structure; use Utopia\Database\Helpers\Permission; use Utopia\Database\Helpers\Role; use Utopia\Database\Mirror; +use Utopia\Database\OnDuplicate; use Utopia\Database\PDO; class MirrorTest extends Base diff --git a/tests/e2e/Adapter/Scopes/DocumentTests.php b/tests/e2e/Adapter/Scopes/DocumentTests.php index c3e30d95b..f4940b1ba 100644 --- a/tests/e2e/Adapter/Scopes/DocumentTests.php +++ b/tests/e2e/Adapter/Scopes/DocumentTests.php @@ -7905,7 +7905,7 @@ public function testCreateDocumentsIgnoreDuplicates(): void // With OnDuplicate::Skip, duplicates should be silently skipped $emittedIds = []; $collection = __FUNCTION__; - $count = $database->withOnDuplicate(OnDuplicate::Skip,function () use ($database, $collection, &$emittedIds) { + $count = $database->withOnDuplicate(OnDuplicate::Skip, function () use ($database, $collection, &$emittedIds) { return $database->createDocuments($collection, [ new Document([ '$id' => 'doc1', @@ -7967,7 +7967,7 @@ public function testCreateDocumentsIgnoreAllDuplicates(): void // With OnDuplicate::Skip, inserting only duplicates should succeed with no new rows $emittedIds = []; $collection = __FUNCTION__; - $count = $database->withOnDuplicate(OnDuplicate::Skip,function () use ($database, $collection, &$emittedIds) { + $count = $database->withOnDuplicate(OnDuplicate::Skip, function () use ($database, $collection, &$emittedIds) { return $database->createDocuments($collection, [ new Document([ '$id' => 'existing', @@ -8001,7 +8001,7 @@ public function testCreateDocumentsSkipDuplicatesEmptyBatch(): void $database->createCollection($collection); $database->createAttribute($collection, 'name', Database::VAR_STRING, 128, true); - $count = $database->withOnDuplicate(OnDuplicate::Skip,fn () => $database->createDocuments($collection, [])); + $count = $database->withOnDuplicate(OnDuplicate::Skip, fn () => $database->createDocuments($collection, [])); $this->assertSame(0, $count); $this->assertCount(0, $database->find($collection)); @@ -8030,9 +8030,9 @@ public function testCreateDocumentsSkipDuplicatesNestedScope(): void // Nested scope — inner scope runs inside outer scope. // After inner exits, outer state should still be "skip enabled". // After outer exits, state should restore to "skip disabled". - $countOuter = $database->withOnDuplicate(OnDuplicate::Skip,function () use ($database, $collection, $makeDoc) { + $countOuter = $database->withOnDuplicate(OnDuplicate::Skip, function () use ($database, $collection, $makeDoc) { // Inner scope: add dup + new - $countInner = $database->withOnDuplicate(OnDuplicate::Skip,function () use ($database, $collection, $makeDoc) { + $countInner = $database->withOnDuplicate(OnDuplicate::Skip, function () use ($database, $collection, $makeDoc) { return $database->createDocuments($collection, [ $makeDoc('seed', 'Dup'), $makeDoc('innerNew', 'InnerNew'), @@ -8102,7 +8102,7 @@ public function testCreateDocumentsSkipDuplicatesLargeBatch(): void } $emittedIds = []; - $count = $database->withOnDuplicate(OnDuplicate::Skip,function () use ($database, $collection, $batch, &$emittedIds) { + $count = $database->withOnDuplicate(OnDuplicate::Skip, function () use ($database, $collection, $batch, &$emittedIds) { return $database->createDocuments($collection, $batch, onNext: function (Document $doc) use (&$emittedIds) { $emittedIds[] = $doc->getId(); }); @@ -8142,13 +8142,14 @@ public function testCreateDocumentsSkipDuplicatesSecondCallSkipsAll(): void ); // First call — all new - $firstCount = $database->withOnDuplicate(OnDuplicate::Skip, + $firstCount = $database->withOnDuplicate( + OnDuplicate::Skip, fn () => $database->createDocuments($collection, $makeBatch('First')) ); $this->assertSame(3, $firstCount); $emittedIds = []; - $secondCount = $database->withOnDuplicate(OnDuplicate::Skip,function () use ($database, $collection, $makeBatch, &$emittedIds) { + $secondCount = $database->withOnDuplicate(OnDuplicate::Skip, function () use ($database, $collection, $makeBatch, &$emittedIds) { return $database->createDocuments($collection, $makeBatch('Second'), onNext: function (Document $doc) use (&$emittedIds) { $emittedIds[] = $doc->getId(); }); @@ -8238,7 +8239,7 @@ public function testCreateDocumentsSkipDuplicatesRelationships(): void ]), ]; - $database->withOnDuplicate(OnDuplicate::Skip,fn () => $database->createDocuments($parent, $batch)); + $database->withOnDuplicate(OnDuplicate::Skip, fn () => $database->createDocuments($parent, $batch)); $existing = $database->getDocument($parent, 'existingParent'); $this->assertFalse($existing->isEmpty()); From 4dd3ef681f7e09e0226c45aaafe0fc719cd1c0ea Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Tue, 21 Apr 2026 14:59:01 +0100 Subject: [PATCH 06/10] Schema Upsert: type-aware drop+recreate for attributes, drop+recreate for indexes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Upsert on createAttribute now introspects the existing column and only drops+recreates when the declared type/size differs. Matching columns are a cheap no-op — migration data stays intact. Skip keeps pure tolerate. - SQL base: new attributeMatches() helper (INFORMATION_SCHEMA.COLUMN_TYPE + normalizeColumnType strips MySQL legacy display widths) wired into createAttribute's Upsert branch - Postgres: override attributeMatches() to use pg_catalog.format_type() + Postgres-specific normalization (`character varying` → VARCHAR, strip `without time zone`). Fix createAttribute to tolerate/drop+recreate on duplicate per mode (was previously rethrowing) - SQLite: override attributeMatches() to query PRAGMA table_info - Mongo: createAttribute is already a no-op (schemaless), no change createIndex now drops+recreates on Upsert (no cheap spec comparison; a rebuild is the cost of getting the new definition in place): - MariaDB/Postgres: catch DuplicateException in Upsert → deleteIndex + retry - SQLite: pre-check sqlite_master → if hit + Upsert, deleteIndex + continue; closeCursor before DDL to avoid "database table is locked" - Mongo: catch IndexKeySpecsConflict (code 86) as DuplicateException so the drop+recreate branch fires (85 was already mapped) - Postgres createIndex: Skip uses IF NOT EXISTS, Upsert uses plain CREATE so the duplicate error bubbles up and triggers the drop+recreate branch Database::withOnDuplicate() now delegates to adapter->withOnDuplicate() so schema-level create* operations running directly against the adapter observe the mode. Previously only createDocuments propagated it. Tests (Scopes/DocumentTests): - testUpsertAttributeSameTypeNoop: matching type → data preserved - testUpsertAttrTypeChangedRecreates: widened VARCHAR → drop+recreate (asserted via subsequent same-type Upsert being a no-op + deleteAttribute) - testUpsertIndexRebuilds: index pointing at different columns is rebuilt - testSkipSchemaTolerates: Skip leaves existing column/index untouched even when the declared spec is wider --- src/Database/Adapter/MariaDB.php | 13 +- src/Database/Adapter/Mongo.php | 14 ++- src/Database/Adapter/Postgres.php | 80 ++++++++++++- src/Database/Adapter/SQL.php | 78 +++++++++++- src/Database/Adapter/SQLite.php | 52 +++++++- src/Database/Database.php | 7 +- tests/e2e/Adapter/Scopes/DocumentTests.php | 132 +++++++++++++++++++++ 7 files changed, 350 insertions(+), 26 deletions(-) diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index dbb0ffda6..adec5939a 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -783,11 +783,16 @@ public function createIndex(string $collection, string $id, string $type, array ->execute(); } catch (PDOException $e) { $err = $this->processException($e); - // Skip/Upsert: tolerate an existing index. Spec changes (attributes, type, - // uniqueness) must go through deleteIndex+createIndex — auto-modifying - // indexes risks long rebuilds and uniqueness violations mid-migration. if ($err instanceof DuplicateException && $this->onDuplicate !== OnDuplicate::Fail) { - return true; + if ($this->onDuplicate === OnDuplicate::Skip) { + return true; + } + // Upsert: bring the index in line with the requested spec. + // Index spec comparison is adapter-specific and brittle, so we + // always drop and recreate here; the cost is one extra index + // rebuild, which is acceptable during a re-migration. + $this->deleteIndex($collection->getId(), $id); + return $this->getPDO()->prepare($sql)->execute(); } throw $err; } diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index 2b16f401e..8dd7d2cd6 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -1118,7 +1118,15 @@ public function createIndex(string $collection, string $id, string $type, array return $result; } catch (\Exception $e) { - throw $this->processException($e); + $err = $this->processException($e); + if ($err instanceof DuplicateException && $this->onDuplicate === OnDuplicate::Upsert) { + // Mongo raises IndexKeySpecsConflict when re-creating an index + // with a name that points to different keys/options. Drop the + // existing one and recreate with the new spec. + $this->deleteIndex($collection, $id); + return $this->client->createIndexes($name, [$indexes], $options); + } + throw $err; } } @@ -3604,8 +3612,8 @@ protected function processException(\Throwable $e): \Throwable return new DuplicateException('Collection already exists', $e->getCode(), $e); } - // Index already exists - if ($e->getCode() === 85) { + // Index already exists (85 = IndexOptionsConflict, 86 = IndexKeySpecsConflict) + if ($e->getCode() === 85 || $e->getCode() === 86) { return new DuplicateException('Index already exists', $e->getCode(), $e); } diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 2467c5214..cfef8b8f6 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -495,12 +495,12 @@ public function createAttribute(string $collection, string $id, string $type, in } $name = $this->filter($collection); - $id = $this->filter($id); - $type = $this->getSQLType($type, $size, $signed, $array, $required); + $filteredId = $this->filter($id); + $sqlType = $this->getSQLType($type, $size, $signed, $array, $required); $sql = " ALTER TABLE {$this->getSQLTable($name)} - ADD COLUMN \"{$id}\" {$type} + ADD COLUMN \"{$filteredId}\" {$sqlType} "; $sql = $this->trigger(Database::EVENT_ATTRIBUTE_CREATE, $sql); @@ -509,7 +509,18 @@ public function createAttribute(string $collection, string $id, string $type, in return $this->execute($this->getPDO() ->prepare($sql)); } catch (PDOException $e) { - throw $this->processException($e); + $err = $this->processException($e); + if ($err instanceof DuplicateException && $this->onDuplicate !== OnDuplicate::Fail) { + if ($this->onDuplicate === OnDuplicate::Skip) { + return true; + } + if ($this->attributeMatches($collection, $id, $type, $size, $signed, $array, $required) === true) { + return true; + } + $this->deleteAttribute($collection, $id, $array); + return $this->execute($this->getPDO()->prepare($sql)); + } + throw $err; } } @@ -946,7 +957,10 @@ public function createIndex(string $collection, string $id, string $type, array $attributes = "_tenant, {$attributes}"; } - $createClause = $this->onDuplicate !== OnDuplicate::Fail + // Skip tolerates pre-existing indexes via IF NOT EXISTS; Upsert always + // issues a plain CREATE so the duplicate-key error triggers the + // drop+recreate branch below (we want the new spec, not a silent skip). + $createClause = $this->onDuplicate === OnDuplicate::Skip ? "CREATE {$sqlType} IF NOT EXISTS" : "CREATE {$sqlType}"; @@ -972,7 +986,12 @@ public function createIndex(string $collection, string $id, string $type, array try { return $this->getPDO()->prepare($sql)->execute(); } catch (PDOException $e) { - throw $this->processException($e); + $err = $this->processException($e); + if ($err instanceof DuplicateException && $this->onDuplicate === OnDuplicate::Upsert) { + $this->deleteIndex($collection, $id); + return $this->getPDO()->prepare($sql)->execute(); + } + throw $err; } } /** @@ -1954,6 +1973,55 @@ protected function getFulltextValue(string $value): string * @return string * @throws DatabaseException */ + protected function attributeMatches( + string $collection, + string $id, + string $type, + int $size, + bool $signed = true, + bool $array = false, + bool $required = false + ): ?bool { + // pg_catalog.format_type() emits the canonical textual form (e.g. + // "character varying(128)", "timestamp(3) without time zone") which + // normalizes more predictably than information_schema.data_type. + $stmt = $this->getPDO()->prepare(' + SELECT pg_catalog.format_type(a.atttypid, a.atttypmod) AS type + FROM pg_catalog.pg_attribute a + JOIN pg_catalog.pg_class c ON a.attrelid = c.oid + JOIN pg_catalog.pg_namespace n ON c.relnamespace = n.oid + WHERE n.nspname = :schema + AND c.relname = :table + AND a.attname = :column + AND a.attnum > 0 + AND NOT a.attisdropped + '); + $stmt->bindValue(':schema', $this->getDatabase(), PDO::PARAM_STR); + $stmt->bindValue(':table', "{$this->getNamespace()}_{$this->filter($collection)}", PDO::PARAM_STR); + $stmt->bindValue(':column', $this->filter($id), PDO::PARAM_STR); + $stmt->execute(); + $row = $stmt->fetch(PDO::FETCH_ASSOC); + if (empty($row)) { + return null; + } + + $target = $this->getSQLType($type, $size, $signed, $array, $required); + return $this->normalizeColumnType((string) $row['type']) === $this->normalizeColumnType($target); + } + + protected function normalizeColumnType(string $sql): string + { + $sql = \strtoupper(\trim($sql)); + // `character varying(128)` → `VARCHAR(128)` so it matches getSQLType output. + $sql = \str_replace('CHARACTER VARYING', 'VARCHAR', $sql); + // Postgres's format_type appends `without time zone` to timestamps; strip it. + $sql = \preg_replace('/\s*WITHOUT TIME ZONE\s*/', '', $sql); + $sql = \preg_replace('/\s*,\s*/', ',', $sql); + $sql = \preg_replace('/\s+/', ' ', $sql); + + return \trim($sql); + } + protected function getSQLType(string $type, int $size, bool $signed = true, bool $array = false, bool $required = false): string { if ($array === true) { diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index 0c9bf671a..97e10539d 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -246,9 +246,9 @@ public function list(): array */ public function createAttribute(string $collection, string $id, string $type, int $size, bool $signed = true, bool $array = false, bool $required = false): bool { - $id = $this->quote($this->filter($id)); - $type = $this->getSQLType($type, $size, $signed, $array, $required); - $sql = "ALTER TABLE {$this->getSQLTable($collection)} ADD COLUMN {$id} {$type} {$this->getLockType()};"; + $quoted = $this->quote($this->filter($id)); + $sqlType = $this->getSQLType($type, $size, $signed, $array, $required); + $sql = "ALTER TABLE {$this->getSQLTable($collection)} ADD COLUMN {$quoted} {$sqlType} {$this->getLockType()};"; $sql = $this->trigger(Database::EVENT_ATTRIBUTE_CREATE, $sql); try { @@ -257,10 +257,18 @@ public function createAttribute(string $collection, string $id, string $type, in ->execute(); } catch (PDOException $e) { $err = $this->processException($e); - // Skip/Upsert: tolerate an existing column. Schema evolution (type/size - // changes) must go through updateAttribute — too risky to auto-apply. if ($err instanceof DuplicateException && $this->onDuplicate !== OnDuplicate::Fail) { - return true; + if ($this->onDuplicate === OnDuplicate::Skip) { + return true; + } + // Upsert: drop + recreate only if the declared type differs from + // what's already in the DB — otherwise the existing column is + // already correct and re-creation would pointlessly lose data. + if ($this->attributeMatches($collection, $id, $type, $size, $signed, $array, $required) === true) { + return true; + } + $this->deleteAttribute($collection, $id, $array); + return $this->getPDO()->prepare($sql)->execute(); } throw $err; } @@ -1886,6 +1894,64 @@ abstract protected function getSQLType( bool $required = false ): string; + /** + * Returns whether the existing DB column for $collection.$id matches the + * requested declaration. Used by createAttribute under OnDuplicate::Upsert + * to decide between a no-op and a drop+recreate. + * + * Return values: true = exists and matches, false = exists but differs, + * null = doesn't exist (shouldn't happen from the Upsert path, but + * subclasses should handle it). + * + * Default implementation queries INFORMATION_SCHEMA and compares + * COLUMN_TYPE against getSQLType(). Adapters using non-INFORMATION_SCHEMA + * metadata (SQLite → PRAGMA, Postgres → pg_catalog) must override. + */ + protected function attributeMatches( + string $collection, + string $id, + string $type, + int $size, + bool $signed = true, + bool $array = false, + bool $required = false + ): ?bool { + $stmt = $this->getPDO()->prepare(' + SELECT COLUMN_TYPE + FROM INFORMATION_SCHEMA.COLUMNS + WHERE TABLE_SCHEMA = :schema + AND TABLE_NAME = :table + AND COLUMN_NAME = :column + '); + $stmt->bindValue(':schema', $this->getDatabase(), \PDO::PARAM_STR); + $stmt->bindValue(':table', "{$this->getNamespace()}_{$this->filter($collection)}", \PDO::PARAM_STR); + $stmt->bindValue(':column', $this->filter($id), \PDO::PARAM_STR); + $stmt->execute(); + $row = $stmt->fetch(\PDO::FETCH_ASSOC); + if (empty($row)) { + return null; + } + + $target = $this->getSQLType($type, $size, $signed, $array, $required); + return $this->normalizeColumnType((string) $row['COLUMN_TYPE']) === $this->normalizeColumnType($target); + } + + /** + * Normalize a SQL type declaration for equality comparison: uppercase, + * collapse whitespace, drop MySQL's legacy integer display widths + * (BIGINT(20) → BIGINT) — MySQL 8+ doesn't emit them and comparing them + * against MariaDB's output would otherwise cause false mismatches. + * TINYINT(1) is intentionally preserved because it's the canonical + * BOOLEAN representation. + */ + protected function normalizeColumnType(string $sql): string + { + $sql = \preg_replace('/\b(BIGINT|INT|SMALLINT|MEDIUMINT)\s*\(\s*\d+\s*\)/i', '$1', $sql); + $sql = \preg_replace('/\s+/', ' ', $sql); + + return \strtoupper(\trim($sql)); + } + /** * @throws DatabaseException For unknown type values. */ diff --git a/src/Database/Adapter/SQLite.php b/src/Database/Adapter/SQLite.php index d609fe6b4..46c885073 100644 --- a/src/Database/Adapter/SQLite.php +++ b/src/Database/Adapter/SQLite.php @@ -464,22 +464,33 @@ public function renameIndex(string $collection, string $old, string $new): bool public function createIndex(string $collection, string $id, string $type, array $attributes, array $lengths, array $orders, array $indexAttributeTypes = [], array $collation = [], int $ttl = 1): bool { $name = $this->filter($collection); - $id = $this->filter($id); + $filteredId = $this->filter($id); // Workaround for no support for CREATE INDEX IF NOT EXISTS $stmt = $this->getPDO()->prepare(" - SELECT name - FROM sqlite_master + SELECT name + FROM sqlite_master WHERE type='index' AND name=:_index; "); - $stmt->bindValue(':_index', "{$this->getNamespace()}_{$this->tenant}_{$name}_{$id}"); + $stmt->bindValue(':_index', "{$this->getNamespace()}_{$this->tenant}_{$name}_{$filteredId}"); $stmt->execute(); $index = $stmt->fetch(); + // SQLite holds a table-level lock while the cursor is open; close it + // before we issue DDL in the Upsert branch or the subsequent DROP + // INDEX will error with "database table is locked". + $stmt->closeCursor(); if (!empty($index)) { - return true; + // Upsert: blow away the existing index and recreate it so the + // definition matches the incoming spec. Skip/Fail keep the + // original pre-existing behavior (tolerate). + if ($this->onDuplicate === OnDuplicate::Upsert) { + $this->deleteIndex($collection, $id); + } else { + return true; + } } - $sql = $this->getSQLIndex($name, $id, $type, $attributes); + $sql = $this->getSQLIndex($name, $filteredId, $type, $attributes); $sql = $this->trigger(Database::EVENT_INDEX_CREATE, $sql); @@ -1968,4 +1979,33 @@ protected function getInsertSuffix(string $table, array $columns = []): string { return ''; } + + /** + * SQLite stores declared types verbatim (type affinity only matters at + * value-storage time), so `PRAGMA table_info` returns whatever we emitted + * from getSQLType. Compare declared form against target after the usual + * normalization (strips MariaDB integer display widths, uppercases). + */ + protected function attributeMatches( + string $collection, + string $id, + string $type, + int $size, + bool $signed = true, + bool $array = false, + bool $required = false + ): ?bool { + $table = "{$this->getNamespace()}_{$this->filter($collection)}"; + $stmt = $this->getPDO()->prepare('SELECT type FROM pragma_table_info(:table) WHERE name = :column'); + $stmt->bindValue(':table', $table, PDO::PARAM_STR); + $stmt->bindValue(':column', $this->filter($id), PDO::PARAM_STR); + $stmt->execute(); + $row = $stmt->fetch(PDO::FETCH_ASSOC); + if (empty($row)) { + return null; + } + + $target = $this->getSQLType($type, $size, $signed, $array, $required); + return $this->normalizeColumnType((string) $row['type']) === $this->normalizeColumnType($target); + } } diff --git a/src/Database/Database.php b/src/Database/Database.php index 0ea5e655c..17b4cc73a 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -858,7 +858,12 @@ public function withOnDuplicate(OnDuplicate $mode, callable $callback): mixed $this->onDuplicate = $mode; try { - return $callback(); + // Mirror the mode onto the adapter so schema-level operations + // (createAttribute / createIndex / createCollection) that run + // directly against the adapter can observe it. createDocuments + // still goes through its own adapter->withOnDuplicate dispatch, + // which is nestable and idempotent with this outer scope. + return $this->adapter->withOnDuplicate($mode, $callback); } finally { $this->onDuplicate = $previous; } diff --git a/tests/e2e/Adapter/Scopes/DocumentTests.php b/tests/e2e/Adapter/Scopes/DocumentTests.php index f4940b1ba..2c511513b 100644 --- a/tests/e2e/Adapter/Scopes/DocumentTests.php +++ b/tests/e2e/Adapter/Scopes/DocumentTests.php @@ -8344,4 +8344,136 @@ public function testCreateDocsUpsertAll(): void $this->assertSame('v2', $docs[0]->getAttribute('name')); $this->assertSame('v2', $docs[1]->getAttribute('name')); } + + /** + * OnDuplicate::Upsert on createAttribute with an existing column of the + * SAME type is a no-op — row data must be preserved. + */ + public function testUpsertAttributeSameTypeNoop(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + if (!$database->getAdapter()->getSupportForAttributes()) { + $this->expectNotToPerformAssertions(); + return; + } + + $database->createCollection(__FUNCTION__); + $database->createAttribute(__FUNCTION__, 'name', Database::VAR_STRING, 128, true); + + $database->createDocument(__FUNCTION__, new Document([ + '$id' => 'doc', + 'name' => 'preserve-me', + '$permissions' => [Permission::read(Role::any())], + ])); + + $collection = __FUNCTION__; + $database->withOnDuplicate(OnDuplicate::Upsert, function () use ($database, $collection) { + $database->getAdapter()->createAttribute($collection, 'name', Database::VAR_STRING, 128, true); + }); + + $doc = $database->getDocument(__FUNCTION__, 'doc'); + $this->assertSame('preserve-me', $doc->getAttribute('name')); + } + + /** + * OnDuplicate::Upsert on createAttribute with a DIFFERENT type drops the + * existing column and recreates it with the new spec. Verified at the + * adapter layer via a follow-up createAttribute under Fail — which would + * throw DuplicateException if the column hadn't actually been dropped. + */ + public function testUpsertAttrTypeChangedRecreates(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + if (!$database->getAdapter()->getSupportForAttributes()) { + $this->expectNotToPerformAssertions(); + return; + } + + $database->createCollection(__FUNCTION__); + $database->createAttribute(__FUNCTION__, 'payload', Database::VAR_STRING, 64, true); + + $collection = __FUNCTION__; + // Upsert with a WIDER size: must drop the old VARCHAR(64) and recreate + // VARCHAR(256). + $result = $database->withOnDuplicate(OnDuplicate::Upsert, function () use ($database, $collection) { + return $database->getAdapter()->createAttribute($collection, 'payload', Database::VAR_STRING, 256, true); + }); + $this->assertTrue($result); + + // Second Upsert with the SAME (new) size must be a cheap no-op — the + // matches-check returns true and no DDL runs. + $result = $database->withOnDuplicate(OnDuplicate::Upsert, function () use ($database, $collection) { + return $database->getAdapter()->createAttribute($collection, 'payload', Database::VAR_STRING, 256, true); + }); + $this->assertTrue($result); + + // Sanity: deleteAttribute succeeds, confirming the column is really there. + $this->assertTrue($database->getAdapter()->deleteAttribute(__FUNCTION__, 'payload')); + } + + /** + * OnDuplicate::Upsert on createIndex over an existing index name rebuilds + * the index — the end state matches the requested spec. + */ + public function testUpsertIndexRebuilds(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + if (!$database->getAdapter()->getSupportForAttributes()) { + $this->expectNotToPerformAssertions(); + return; + } + + $database->createCollection(__FUNCTION__); + $database->createAttribute(__FUNCTION__, 'a', Database::VAR_STRING, 64, true); + $database->createAttribute(__FUNCTION__, 'b', Database::VAR_STRING, 64, true); + $database->createIndex(__FUNCTION__, 'idx', Database::INDEX_KEY, ['a']); + + $collection = __FUNCTION__; + $result = $database->withOnDuplicate(OnDuplicate::Upsert, function () use ($database, $collection) { + return $database->getAdapter()->createIndex($collection, 'idx', Database::INDEX_KEY, ['b'], [], []); + }); + + $this->assertTrue($result); + $this->assertTrue($database->getAdapter()->deleteIndex(__FUNCTION__, 'idx')); + } + + /** + * OnDuplicate::Skip on createAttribute / createIndex tolerates pre-existing + * resources without modifying them. + */ + public function testSkipSchemaTolerates(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + if (!$database->getAdapter()->getSupportForAttributes()) { + $this->expectNotToPerformAssertions(); + return; + } + + $database->createCollection(__FUNCTION__); + $database->createAttribute(__FUNCTION__, 'name', Database::VAR_STRING, 128, true); + $database->createIndex(__FUNCTION__, 'idx', Database::INDEX_KEY, ['name']); + + $database->createDocument(__FUNCTION__, new Document([ + '$id' => 'doc', + 'name' => 'keep', + '$permissions' => [Permission::read(Role::any())], + ])); + + $collection = __FUNCTION__; + $database->withOnDuplicate(OnDuplicate::Skip, function () use ($database, $collection) { + $database->getAdapter()->createAttribute($collection, 'name', Database::VAR_STRING, 512, true); + $database->getAdapter()->createIndex($collection, 'idx', Database::INDEX_KEY, ['name'], [], []); + }); + + $doc = $database->getDocument(__FUNCTION__, 'doc'); + $this->assertSame('keep', $doc->getAttribute('name')); + } } From 76bda6ac88d9e010ce6fc819b90c3948cb2e2177 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Tue, 21 Apr 2026 15:09:35 +0100 Subject: [PATCH 07/10] Lint: restore getSQLType phpdoc position in Postgres adapter --- src/Database/Adapter/Postgres.php | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index cfef8b8f6..359c7ce9f 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -1962,17 +1962,6 @@ protected function getFulltextValue(string $value): string return "'" . $value . "'"; } - /** - * Get SQL Type - * - * @param string $type - * @param int $size in chars - * @param bool $signed - * @param bool $array - * @param bool $required - * @return string - * @throws DatabaseException - */ protected function attributeMatches( string $collection, string $id, @@ -2022,6 +2011,17 @@ protected function normalizeColumnType(string $sql): string return \trim($sql); } + /** + * Get SQL Type + * + * @param string $type + * @param int $size in chars + * @param bool $signed + * @param bool $array + * @param bool $required + * @return string + * @throws DatabaseException + */ protected function getSQLType(string $type, int $size, bool $signed = true, bool $array = false, bool $required = false): string { if ($array === true) { From a42f4cb85ff3cace111f9d7a2833094d78a26e00 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Wed, 22 Apr 2026 06:02:45 +0100 Subject: [PATCH 08/10] Refactor DDL duplicate handling from try/catch to pre-check, add indexMatches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces exception-driven control flow in createAttribute and createIndex with pre-check helpers (attributeMatches / indexMatches). The previous pattern had three growth-cost problems: - Error-code classification drift across DB versions (MariaDB vs MySQL vs MySQL compat modes) could silently shift a duplicate to a rethrow path. - On Postgres, a failed CREATE INDEX / ALTER TABLE inside a transaction aborts the outer transaction — catch-and-retry is unsafe. - Layering new concerns (type-check, deadlock retry) stacks branches inside the same catch instead of reading top-to-bottom. Pre-check pattern (mirrors attributeMatches): $match = $this->indexMatches(...); if ($match === true) return true; // spec unchanged, no-op if ($match === false) { if (Skip) return true; // tolerate deleteIndex(...); // Upsert: rebuild } // null: doesn't exist, fall through to CREATE indexMatches implementations: - SQL base (MariaDB/MySQL): INFORMATION_SCHEMA.STATISTICS, compares column list (ordered) + NON_UNIQUE flag + INDEX_TYPE (BTREE/FULLTEXT/SPATIAL). - Postgres override: pg_catalog via unnest(indkey) for column ordering, indisunique for uniqueness, pg_am.amname for index method (btree/gist/ gin/hnsw). - SQLite override: PRAGMA index_list + PRAGMA index_info. Closes cursors before returning — SQLite holds a table-level lock while a read cursor is open which would block subsequent DDL. SQLite.createIndex runs the pre-check in every mode (including Fail): the method has historically been unconditionally idempotent because createCollection intentionally issues the same CREATE INDEX twice for _perms side-tables. Matching the contract keeps that path working. Per-column length and sort order are intentionally ignored in the comparison — a difference there yields a conservative false negative (unnecessary rebuild), never a false positive. New test: testUpsertIndexSameSpecNoop — re-declaring an index with the same spec under OnDuplicate::Upsert is a pure no-op (no DROP / CREATE DDL fires), symmetric with the pre-existing testUpsertAttributeSameTypeNoop. Row Upsert (createDocuments) is intentionally NOT changed: DB-native conflict primitives (INSERT IGNORE / ON DUPLICATE KEY UPDATE / ON CONFLICT DO UPDATE / $set+upsert:true) resolve duplicates atomically per-row in one statement. A find()-then-insert pre-check would be slower, race-prone, and miss conflicts on user-defined unique indexes. --- src/Database/Adapter/MariaDB.php | 27 ++-- src/Database/Adapter/Postgres.php | 141 +++++++++++++++++---- src/Database/Adapter/SQL.php | 106 +++++++++++++--- src/Database/Adapter/SQLite.php | 84 +++++++++--- tests/e2e/Adapter/Scopes/DocumentTests.php | 33 +++++ 5 files changed, 320 insertions(+), 71 deletions(-) diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index adec5939a..70202c3b1 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -736,6 +736,9 @@ public function createIndex(string $collection, string $id, string $type, array $collectionAttributes = \json_decode($collection->getAttribute('attributes', []), true); $id = $this->filter($id); + // Preserve raw attribute list for indexMatches — the loop below + // mutates $attributes into SQL-formatted column fragments. + $attrsRaw = $attributes; foreach ($attributes as $i => $attr) { $attribute = null; @@ -777,25 +780,23 @@ public function createIndex(string $collection, string $id, string $type, array $sql = "CREATE {$sqlType} `{$id}` ON {$this->getSQLTable($collection->getId())} ({$attributes})"; $sql = $this->trigger(Database::EVENT_INDEX_CREATE, $sql); - try { - return $this->getPDO() - ->prepare($sql) - ->execute(); - } catch (PDOException $e) { - $err = $this->processException($e); - if ($err instanceof DuplicateException && $this->onDuplicate !== OnDuplicate::Fail) { + // Skip/Upsert: pre-check via indexMatches() instead of reacting to a + // DDL duplicate. Mirrors the attributeMatches() pattern — if spec + // matches we no-op; if it differs we rebuild only on Upsert. + if ($this->onDuplicate !== OnDuplicate::Fail) { + $match = $this->indexMatches($collection->getId(), $id, $type, $attrsRaw, $lengths, $orders); + if ($match === true) { + return true; + } + if ($match === false) { if ($this->onDuplicate === OnDuplicate::Skip) { return true; } - // Upsert: bring the index in line with the requested spec. - // Index spec comparison is adapter-specific and brittle, so we - // always drop and recreate here; the cost is one extra index - // rebuild, which is acceptable during a re-migration. $this->deleteIndex($collection->getId(), $id); - return $this->getPDO()->prepare($sql)->execute(); } - throw $err; } + + return $this->getPDO()->prepare($sql)->execute(); } /** diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 359c7ce9f..d1dde695c 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -505,23 +505,23 @@ public function createAttribute(string $collection, string $id, string $type, in $sql = $this->trigger(Database::EVENT_ATTRIBUTE_CREATE, $sql); - try { - return $this->execute($this->getPDO() - ->prepare($sql)); - } catch (PDOException $e) { - $err = $this->processException($e); - if ($err instanceof DuplicateException && $this->onDuplicate !== OnDuplicate::Fail) { + // Skip/Upsert: pre-check instead of catching a duplicate from the DDL. + // A failed ALTER TABLE inside a transaction would abort the outer + // transaction on Postgres, making the catch-and-retry pattern unsafe. + if ($this->onDuplicate !== OnDuplicate::Fail) { + $match = $this->attributeMatches($collection, $id, $type, $size, $signed, $array, $required); + if ($match === true) { + return true; + } + if ($match === false) { if ($this->onDuplicate === OnDuplicate::Skip) { return true; } - if ($this->attributeMatches($collection, $id, $type, $size, $signed, $array, $required) === true) { - return true; - } $this->deleteAttribute($collection, $id, $array); - return $this->execute($this->getPDO()->prepare($sql)); } - throw $err; } + + return $this->execute($this->getPDO()->prepare($sql)); } /** @@ -918,6 +918,9 @@ public function createIndex(string $collection, string $id, string $type, array { $collection = $this->filter($collection); $id = $this->filter($id); + // Preserve raw attribute list for indexMatches — the loop below + // mutates $attributes into SQL-formatted column fragments. + $attrsRaw = $attributes; foreach ($attributes as $i => $attr) { $order = empty($orders[$i]) || Database::INDEX_FULLTEXT === $type ? '' : $orders[$i]; @@ -957,14 +960,7 @@ public function createIndex(string $collection, string $id, string $type, array $attributes = "_tenant, {$attributes}"; } - // Skip tolerates pre-existing indexes via IF NOT EXISTS; Upsert always - // issues a plain CREATE so the duplicate-key error triggers the - // drop+recreate branch below (we want the new spec, not a silent skip). - $createClause = $this->onDuplicate === OnDuplicate::Skip - ? "CREATE {$sqlType} IF NOT EXISTS" - : "CREATE {$sqlType}"; - - $sql = "{$createClause} \"{$keyName}\" ON {$this->getSQLTable($collection)}"; + $sql = "CREATE {$sqlType} \"{$keyName}\" ON {$this->getSQLTable($collection)}"; // Add USING clause for special index types $sql .= match ($type) { @@ -983,16 +979,23 @@ public function createIndex(string $collection, string $id, string $type, array $sql = $this->trigger(Database::EVENT_INDEX_CREATE, $sql); - try { - return $this->getPDO()->prepare($sql)->execute(); - } catch (PDOException $e) { - $err = $this->processException($e); - if ($err instanceof DuplicateException && $this->onDuplicate === OnDuplicate::Upsert) { + // Skip/Upsert: pre-check via indexMatches() rather than catching a + // duplicate DDL error. A failed CREATE INDEX on Postgres aborts the + // surrounding transaction, so catch-and-retry is unsafe. + if ($this->onDuplicate !== OnDuplicate::Fail) { + $match = $this->indexMatches($collection, $id, $type, $attrsRaw, $lengths, $orders); + if ($match === true) { + return true; + } + if ($match === false) { + if ($this->onDuplicate === OnDuplicate::Skip) { + return true; + } $this->deleteIndex($collection, $id); - return $this->getPDO()->prepare($sql)->execute(); } - throw $err; } + + return $this->getPDO()->prepare($sql)->execute(); } /** * Delete Index @@ -2011,6 +2014,92 @@ protected function normalizeColumnType(string $sql): string return \trim($sql); } + /** + * Postgres stores indexes in pg_index; the relation name on disk is the + * short-keyed form produced by createIndex (namespace + tenant + + * collection + id, hashed if longer than the identifier limit). + * + * Fetches existing column list via unnest(indkey) joined to pg_attribute, + * and uniqueness via indisunique. Index method is looked up via pg_am + * (btree / gin / gist / hnsw). Lengths and per-column orders are ignored + * — a mismatch there yields a conservative false negative, never a false + * positive. + * + * @param array $attributes + * @param array $lengths + * @param array $orders + */ + protected function indexMatches( + string $collection, + string $id, + string $type, + array $attributes, + array $lengths = [], + array $orders = [], + ): ?bool { + $collectionFiltered = $this->filter($collection); + $idFiltered = $this->filter($id); + $keyName = $this->getShortKey("{$this->getNamespace()}_{$this->tenant}_{$collectionFiltered}_{$idFiltered}"); + + $stmt = $this->getPDO()->prepare(" + SELECT a.attname AS column_name, i.indisunique AS is_unique, am.amname AS method + FROM pg_catalog.pg_index i + JOIN pg_catalog.pg_class c ON c.oid = i.indexrelid + JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace + JOIN pg_catalog.pg_am am ON am.oid = c.relam + CROSS JOIN LATERAL unnest(i.indkey) WITH ORDINALITY AS k(attnum, pos) + JOIN pg_catalog.pg_attribute a ON a.attrelid = i.indrelid AND a.attnum = k.attnum + WHERE n.nspname = :schema + AND c.relname = :index + ORDER BY k.pos + "); + $stmt->bindValue(':schema', $this->getDatabase(), PDO::PARAM_STR); + $stmt->bindValue(':index', $keyName, PDO::PARAM_STR); + $stmt->execute(); + $rows = $stmt->fetchAll(PDO::FETCH_ASSOC); + + if (empty($rows)) { + return null; + } + + if ((bool) $rows[0]['is_unique'] !== ($type === Database::INDEX_UNIQUE)) { + return false; + } + + // Map declared index type to Postgres access method (pg_am.amname) + $targetMethod = match ($type) { + Database::INDEX_SPATIAL => 'gist', + Database::INDEX_HNSW_EUCLIDEAN, + Database::INDEX_HNSW_COSINE, + Database::INDEX_HNSW_DOT => 'hnsw', + Database::INDEX_OBJECT, + Database::INDEX_TRIGRAM => 'gin', + default => 'btree', // INDEX_KEY, INDEX_UNIQUE, INDEX_FULLTEXT + }; + if (\strtolower((string) $rows[0]['method']) !== $targetMethod) { + return false; + } + + // Target column list — mirror createIndex's transformation including + // shared-tables tenant prefix for KEY/UNIQUE indexes. + $targetCols = []; + foreach ($attributes as $attr) { + $targetCols[] = match ($attr) { + '$id' => '_uid', + '$createdAt' => '_createdAt', + '$updatedAt' => '_updatedAt', + default => $this->filter($attr), + }; + } + if ($this->sharedTables && \in_array($type, [Database::INDEX_KEY, Database::INDEX_UNIQUE])) { + \array_unshift($targetCols, '_tenant'); + } + + $existingCols = \array_map(fn ($r) => (string) $r['column_name'], $rows); + + return $targetCols === $existingCols; + } + /** * Get SQL Type * diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index 97e10539d..a09da9f87 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -251,27 +251,25 @@ public function createAttribute(string $collection, string $id, string $type, in $sql = "ALTER TABLE {$this->getSQLTable($collection)} ADD COLUMN {$quoted} {$sqlType} {$this->getLockType()};"; $sql = $this->trigger(Database::EVENT_ATTRIBUTE_CREATE, $sql); - try { - return $this->getPDO() - ->prepare($sql) - ->execute(); - } catch (PDOException $e) { - $err = $this->processException($e); - if ($err instanceof DuplicateException && $this->onDuplicate !== OnDuplicate::Fail) { + // Skip/Upsert: pre-check the existing column so we don't drive control + // flow through PDOException. attributeMatches returns null (doesn't + // exist), true (exists + matches spec), or false (exists + differs). + if ($this->onDuplicate !== OnDuplicate::Fail) { + $match = $this->attributeMatches($collection, $id, $type, $size, $signed, $array, $required); + if ($match === true) { + return true; + } + if ($match === false) { if ($this->onDuplicate === OnDuplicate::Skip) { return true; } - // Upsert: drop + recreate only if the declared type differs from - // what's already in the DB — otherwise the existing column is - // already correct and re-creation would pointlessly lose data. - if ($this->attributeMatches($collection, $id, $type, $size, $signed, $array, $required) === true) { - return true; - } + // Upsert: column exists with a different type — drop and + // recreate so migration can repopulate via row Upsert. $this->deleteAttribute($collection, $id, $array); - return $this->getPDO()->prepare($sql)->execute(); } - throw $err; } + + return $this->getPDO()->prepare($sql)->execute(); } /** @@ -1952,6 +1950,84 @@ protected function normalizeColumnType(string $sql): string return \strtoupper(\trim($sql)); } + /** + * Returns whether the existing DB index for $collection.$id matches the + * requested declaration. Mirrors attributeMatches() semantics: + * - null = doesn't exist + * - true = exists and matches spec + * - false = exists but differs + * + * Comparison covers column list (ordered), uniqueness flag, and index + * kind (BTREE/FULLTEXT/SPATIAL). Per-column lengths and sort orders are + * intentionally ignored — a difference there yields a false negative + * (unnecessary rebuild), never a false positive, so it's conservative. + * + * Default implementation uses INFORMATION_SCHEMA.STATISTICS (one row per + * column in the index); MariaDB/MySQL inherit. Postgres and SQLite + * override with dialect-specific metadata queries. + * + * @param array $attributes + * @param array $lengths + * @param array $orders + */ + protected function indexMatches( + string $collection, + string $id, + string $type, + array $attributes, + array $lengths = [], + array $orders = [], + ): ?bool { + $stmt = $this->getPDO()->prepare(' + SELECT COLUMN_NAME, NON_UNIQUE, INDEX_TYPE + FROM INFORMATION_SCHEMA.STATISTICS + WHERE TABLE_SCHEMA = :schema + AND TABLE_NAME = :table + AND INDEX_NAME = :index + ORDER BY SEQ_IN_INDEX + '); + $stmt->bindValue(':schema', $this->getDatabase(), \PDO::PARAM_STR); + $stmt->bindValue(':table', "{$this->getNamespace()}_{$this->filter($collection)}", \PDO::PARAM_STR); + $stmt->bindValue(':index', $this->filter($id), \PDO::PARAM_STR); + $stmt->execute(); + $rows = $stmt->fetchAll(\PDO::FETCH_ASSOC); + + if (empty($rows)) { + return null; + } + + // Uniqueness + if (((int) $rows[0]['NON_UNIQUE'] === 0) !== ($type === Database::INDEX_UNIQUE)) { + return false; + } + + // Index kind + $existingKind = \strtoupper((string) $rows[0]['INDEX_TYPE']); + $targetKind = match ($type) { + Database::INDEX_FULLTEXT => 'FULLTEXT', + Database::INDEX_SPATIAL => 'SPATIAL', + default => 'BTREE', + }; + if ($existingKind !== $targetKind) { + return false; + } + + // Column list — reconstruct the exact sequence that createIndex would + // have written, including the tenant prefix for shared-tables key / + // unique indexes. Must stay in sync with createIndex. + $targetCols = []; + foreach ($attributes as $attr) { + $targetCols[] = $this->filter($this->getInternalKeyForAttribute($attr)); + } + if ($this->sharedTables && $type !== Database::INDEX_FULLTEXT && $type !== Database::INDEX_SPATIAL) { + \array_unshift($targetCols, '_tenant'); + } + + $existingCols = \array_map(fn ($r) => (string) $r['COLUMN_NAME'], $rows); + + return $targetCols === $existingCols; + } + /** * @throws DatabaseException For unknown type values. */ diff --git a/src/Database/Adapter/SQLite.php b/src/Database/Adapter/SQLite.php index 46c885073..410e0abef 100644 --- a/src/Database/Adapter/SQLite.php +++ b/src/Database/Adapter/SQLite.php @@ -466,23 +466,16 @@ public function createIndex(string $collection, string $id, string $type, array $name = $this->filter($collection); $filteredId = $this->filter($id); - // Workaround for no support for CREATE INDEX IF NOT EXISTS - $stmt = $this->getPDO()->prepare(" - SELECT name - FROM sqlite_master - WHERE type='index' AND name=:_index; - "); - $stmt->bindValue(':_index', "{$this->getNamespace()}_{$this->tenant}_{$name}_{$filteredId}"); - $stmt->execute(); - $index = $stmt->fetch(); - // SQLite holds a table-level lock while the cursor is open; close it - // before we issue DDL in the Upsert branch or the subsequent DROP - // INDEX will error with "database table is locked". - $stmt->closeCursor(); - if (!empty($index)) { - // Upsert: blow away the existing index and recreate it so the - // definition matches the incoming spec. Skip/Fail keep the - // original pre-existing behavior (tolerate). + // Pre-check runs in ALL modes on SQLite: createCollection intentionally + // issues the same CREATE INDEX twice for _perms side-tables, relying on + // the historical "always idempotent" behavior of this method. Matching + // spec → no-op in every mode; differing spec on Upsert rebuilds, on + // Skip/Fail tolerates (historical contract). + $match = $this->indexMatches($collection, $id, $type, $attributes, $lengths, $orders); + if ($match === true) { + return true; + } + if ($match === false) { if ($this->onDuplicate === OnDuplicate::Upsert) { $this->deleteIndex($collection, $id); } else { @@ -2008,4 +2001,61 @@ protected function attributeMatches( $target = $this->getSQLType($type, $size, $signed, $array, $required); return $this->normalizeColumnType((string) $row['type']) === $this->normalizeColumnType($target); } + + /** + * SQLite has no INFORMATION_SCHEMA; indexes live in sqlite_master keyed + * by their fully namespaced name. PRAGMA index_list returns uniqueness, + * PRAGMA index_info returns columns in order. Close cursors before any + * DDL — SQLite holds a table-level lock while a read cursor is open. + * + * @param array $attributes + * @param array $lengths + * @param array $orders + */ + protected function indexMatches( + string $collection, + string $id, + string $type, + array $attributes, + array $lengths = [], + array $orders = [], + ): ?bool { + $collectionFiltered = $this->filter($collection); + $idFiltered = $this->filter($id); + $table = "{$this->getNamespace()}_{$collectionFiltered}"; + $indexName = "{$this->getNamespace()}_{$this->tenant}_{$collectionFiltered}_{$idFiltered}"; + + $stmt = $this->getPDO()->prepare('SELECT "unique" FROM pragma_index_list(:table) WHERE name = :index'); + $stmt->bindValue(':table', $table, PDO::PARAM_STR); + $stmt->bindValue(':index', $indexName, PDO::PARAM_STR); + $stmt->execute(); + $list = $stmt->fetch(PDO::FETCH_ASSOC); + $stmt->closeCursor(); + if (empty($list)) { + return null; + } + + if ((bool) $list['unique'] !== ($type === Database::INDEX_UNIQUE)) { + return false; + } + + $stmt = $this->getPDO()->prepare('SELECT name FROM pragma_index_info(:index) ORDER BY seqno'); + $stmt->bindValue(':index', $indexName, PDO::PARAM_STR); + $stmt->execute(); + $existingCols = \array_map(fn ($r) => (string) $r['name'], $stmt->fetchAll(PDO::FETCH_ASSOC)); + $stmt->closeCursor(); + + // Mirror SQLite's getSQLIndex transformation for target column list. + // Shared-tables tenant prefix: SQLite follows MariaDB's behavior since + // it extends MariaDB — _tenant prepended for KEY/UNIQUE. + $targetCols = []; + foreach ($attributes as $attr) { + $targetCols[] = $this->filter($this->getInternalKeyForAttribute($attr)); + } + if ($this->sharedTables && $type !== Database::INDEX_FULLTEXT && $type !== Database::INDEX_SPATIAL) { + \array_unshift($targetCols, '_tenant'); + } + + return $targetCols === $existingCols; + } } diff --git a/tests/e2e/Adapter/Scopes/DocumentTests.php b/tests/e2e/Adapter/Scopes/DocumentTests.php index 2c511513b..ecdd084e0 100644 --- a/tests/e2e/Adapter/Scopes/DocumentTests.php +++ b/tests/e2e/Adapter/Scopes/DocumentTests.php @@ -8443,6 +8443,39 @@ public function testUpsertIndexRebuilds(): void $this->assertTrue($database->getAdapter()->deleteIndex(__FUNCTION__, 'idx')); } + /** + * OnDuplicate::Upsert on createIndex with a matching spec is a cheap + * no-op — indexMatches() returns true, so no DROP / CREATE fires. Proves + * the symmetry with testUpsertAttributeSameTypeNoop. + */ + public function testUpsertIndexSameSpecNoop(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + if (!$database->getAdapter()->getSupportForAttributes()) { + $this->expectNotToPerformAssertions(); + return; + } + + $database->createCollection(__FUNCTION__); + $database->createAttribute(__FUNCTION__, 'name', Database::VAR_STRING, 128, true); + $database->createIndex(__FUNCTION__, 'idx', Database::INDEX_KEY, ['name']); + + // Same-spec re-declare under Upsert → indexMatches returns true, no DDL. + // We can't directly observe "no rebuild happened" without a spy, but + // we can assert the operation completed and the index is still there. + $collection = __FUNCTION__; + $result = $database->withOnDuplicate(OnDuplicate::Upsert, function () use ($database, $collection) { + return $database->getAdapter()->createIndex($collection, 'idx', Database::INDEX_KEY, ['name'], [], []); + }); + $this->assertTrue($result); + + // Deleting it must succeed — confirming the index is exactly where + // it was before the Upsert no-op. + $this->assertTrue($database->getAdapter()->deleteIndex(__FUNCTION__, 'idx')); + } + /** * OnDuplicate::Skip on createAttribute / createIndex tolerates pre-existing * resources without modifying them. From b24ece89e525db16dd44aba6bee5f0467e3603ec Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Wed, 22 Apr 2026 06:35:59 +0100 Subject: [PATCH 09/10] Restore processException() wrap on DDL execute paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pre-check refactor (a42f4cb) accidentally dropped the try/catch { throw processException($e) } wrap around the final execute() in createAttribute / createIndex. Duplicate handling moved to the pre-check, but other DDL errors (row-size limit, timeout, lock wait, connection drop, NOT NULL violation on table with existing rows, etc.) were leaking as raw PDOException instead of being mapped to the adapter domain exceptions (LimitException, TimeoutException, StructureException, ...). CI caught this via testCreateSpatialColumnWithExistingData on MySQL / SharedTables-MySQL / Pool — the test expects a Structure exception when adding a NOT NULL spatial column to a table that already has rows, and the raw PDOException failed the instanceof assertion. Flagged in review as greptile P1 + coderabbit Major. Sibling DDL methods (createAttributes, renameAttribute, deleteAttribute, deleteIndex) wrap the same way; we were the outliers. Affects: - SQL.createAttribute - Postgres.createAttribute - MariaDB.createIndex - Postgres.createIndex - SQLite.createIndex --- src/Database/Adapter/MariaDB.php | 6 +++++- src/Database/Adapter/Postgres.php | 12 ++++++++++-- src/Database/Adapter/SQL.php | 6 +++++- src/Database/Adapter/SQLite.php | 10 +++++++--- 4 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index 70202c3b1..b33ad6f46 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -796,7 +796,11 @@ public function createIndex(string $collection, string $id, string $type, array } } - return $this->getPDO()->prepare($sql)->execute(); + try { + return $this->getPDO()->prepare($sql)->execute(); + } catch (PDOException $e) { + throw $this->processException($e); + } } /** diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index d1dde695c..ade6a997e 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -521,7 +521,11 @@ public function createAttribute(string $collection, string $id, string $type, in } } - return $this->execute($this->getPDO()->prepare($sql)); + try { + return $this->execute($this->getPDO()->prepare($sql)); + } catch (PDOException $e) { + throw $this->processException($e); + } } /** @@ -995,7 +999,11 @@ public function createIndex(string $collection, string $id, string $type, array } } - return $this->getPDO()->prepare($sql)->execute(); + try { + return $this->getPDO()->prepare($sql)->execute(); + } catch (PDOException $e) { + throw $this->processException($e); + } } /** * Delete Index diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index a09da9f87..2011b1fba 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -269,7 +269,11 @@ public function createAttribute(string $collection, string $id, string $type, in } } - return $this->getPDO()->prepare($sql)->execute(); + try { + return $this->getPDO()->prepare($sql)->execute(); + } catch (PDOException $e) { + throw $this->processException($e); + } } /** diff --git a/src/Database/Adapter/SQLite.php b/src/Database/Adapter/SQLite.php index 410e0abef..c8924fae7 100644 --- a/src/Database/Adapter/SQLite.php +++ b/src/Database/Adapter/SQLite.php @@ -487,9 +487,13 @@ public function createIndex(string $collection, string $id, string $type, array $sql = $this->trigger(Database::EVENT_INDEX_CREATE, $sql); - return $this->getPDO() - ->prepare($sql) - ->execute(); + try { + return $this->getPDO() + ->prepare($sql) + ->execute(); + } catch (PDOException $e) { + throw $this->processException($e); + } } /** From 14a8a3a23efaa8802d243caec67e57ffcfd090fc Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Wed, 22 Apr 2026 08:47:52 +0100 Subject: [PATCH 10/10] Move schema Skip/Upsert to Database layer, tolerate-only (no drop+recreate) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Design change after review: drop+recreate for schema reconciliation belongs in the migration package, not in the database library. Migration compares source vs destination _metadata._updatedAt and issues deleteAttribute / deleteIndex itself before re-creating — the library just needs to not throw when the resource already exists. Consequences: 1. Adapter-level pre-check pattern and introspection helpers are all gone — ~400 lines of dialect-specific INFORMATION_SCHEMA / pg_catalog / PRAGMA code reverted. Schema introspection was the fragile part; removing it shrinks the PR's surface area significantly. - SQL.php: createAttribute back to plain try/catch → processException; attributeMatches, normalizeColumnType, indexMatches all deleted. - Postgres.php: createAttribute + createIndex reverted; IF NOT EXISTS variants in createCollection removed; overrides for attributeMatches / normalizeColumnType / indexMatches deleted. - MariaDB.php: createIndex back to plain try/catch; CREATE TABLE IF NOT EXISTS removed (always plain CREATE). - SQLite.php: createCollection back to plain CREATE TABLE; createIndex back to historical "always idempotent" pre-check via sqlite_master (no mode-aware branches); attributeMatches / indexMatches deleted. - Mongo.php: createIndex Upsert catch-retry removed; createCollection's tolerateExisting no longer includes onDuplicate != Fail. 2. Database.php now owns schema Skip/Upsert dispatch. All three create methods check _metadata and return early on existing resources: - createCollection: returns the existing metadata document. - createAttribute: scans _metadata.attributes JSON for the $id. - createIndex: scans _metadata.indexes JSON for the $id. Upsert == Skip at schema level — both tolerate. Callers that need to change a spec drop+recreate themselves. 3. Mirror.php forwards the OnDuplicate scope on every schema op (createCollection / createAttribute / createIndex) to both source and destination so their Database-layer dispatch observes it. Previously the mode only propagated on createDocuments. 4. Tests rewritten to route through $database-> (not the adapter directly), exercising the full Database-layer flow: - testCreateCollSkipUpsertTolerates - testCreateAttrSkipUpsertTolerates (asserts: wider-spec re-declare is ignored, metadata unchanged) - testCreateIdxSkipUpsertTolerates (same — different-column re-declare is tolerated, metadata unchanged) Row-level Skip/Upsert is unchanged — DB-native conflict primitives (INSERT IGNORE / ON CONFLICT / bulkWrite+$setOnInsert|$set) stay in the adapters. The OnDuplicate enum + withOnDuplicate scope on Database + Adapter + Mirror are also unchanged. --- src/Database/Adapter/MariaDB.php | 31 +-- src/Database/Adapter/Mongo.php | 24 +-- src/Database/Adapter/Postgres.php | 219 ++------------------- src/Database/Adapter/SQL.php | 164 +-------------- src/Database/Adapter/SQLite.php | 134 ++----------- src/Database/Database.php | 31 ++- src/Database/Mirror.php | 126 +++++++----- tests/e2e/Adapter/Scopes/DocumentTests.php | 188 +++++++----------- 8 files changed, 233 insertions(+), 684 deletions(-) diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index b33ad6f46..e881dcb4e 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -158,12 +158,8 @@ public function createCollection(string $name, array $attributes = [], array $in $indexStrings[$key] = "{$indexType} `{$indexId}` ({$indexAttributes}),"; } - $createKeyword = $this->onDuplicate !== OnDuplicate::Fail - ? 'CREATE TABLE IF NOT EXISTS' - : 'CREATE TABLE'; - $collection = " - {$createKeyword} {$this->getSQLTable($id)} ( + CREATE TABLE {$this->getSQLTable($id)} ( _id BIGINT UNSIGNED NOT NULL AUTO_INCREMENT, _uid VARCHAR(255) NOT NULL, _createdAt DATETIME(3) DEFAULT NULL, @@ -194,7 +190,7 @@ public function createCollection(string $name, array $attributes = [], array $in $collection = $this->trigger(Database::EVENT_COLLECTION_CREATE, $collection); $permissions = " - {$createKeyword} {$this->getSQLTable($id . '_perms')} ( + CREATE TABLE {$this->getSQLTable($id . '_perms')} ( _id BIGINT UNSIGNED NOT NULL AUTO_INCREMENT, _type VARCHAR(12) NOT NULL, _permission VARCHAR(255) NOT NULL, @@ -736,9 +732,6 @@ public function createIndex(string $collection, string $id, string $type, array $collectionAttributes = \json_decode($collection->getAttribute('attributes', []), true); $id = $this->filter($id); - // Preserve raw attribute list for indexMatches — the loop below - // mutates $attributes into SQL-formatted column fragments. - $attrsRaw = $attributes; foreach ($attributes as $i => $attr) { $attribute = null; @@ -780,24 +773,10 @@ public function createIndex(string $collection, string $id, string $type, array $sql = "CREATE {$sqlType} `{$id}` ON {$this->getSQLTable($collection->getId())} ({$attributes})"; $sql = $this->trigger(Database::EVENT_INDEX_CREATE, $sql); - // Skip/Upsert: pre-check via indexMatches() instead of reacting to a - // DDL duplicate. Mirrors the attributeMatches() pattern — if spec - // matches we no-op; if it differs we rebuild only on Upsert. - if ($this->onDuplicate !== OnDuplicate::Fail) { - $match = $this->indexMatches($collection->getId(), $id, $type, $attrsRaw, $lengths, $orders); - if ($match === true) { - return true; - } - if ($match === false) { - if ($this->onDuplicate === OnDuplicate::Skip) { - return true; - } - $this->deleteIndex($collection->getId(), $id); - } - } - try { - return $this->getPDO()->prepare($sql)->execute(); + return $this->getPDO() + ->prepare($sql) + ->execute(); } catch (PDOException $e) { throw $this->processException($e); } diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index 8dd7d2cd6..89137b5c1 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -431,12 +431,10 @@ public function createCollection(string $name, array $attributes = [], array $in { $id = $this->getNamespace() . '_' . $this->filter($name); - // In shared-tables mode, for metadata, or when the caller opted into - // Skip/Upsert, the physical collection may already exist. Return early - // to avoid a "Collection Exists" exception from the client. - $tolerateExisting = $this->getSharedTables() - || $name === Database::METADATA - || $this->onDuplicate !== OnDuplicate::Fail; + // In shared-tables mode or for metadata the physical collection may + // already exist. Return early to avoid "Collection Exists" from the + // client. + $tolerateExisting = $this->getSharedTables() || $name === Database::METADATA; if (!$this->inTransaction && $tolerateExisting && $this->exists($this->getNamespace(), $name)) { return true; @@ -449,10 +447,6 @@ public function createCollection(string $name, array $attributes = [], array $in } catch (MongoException $e) { $e = $this->processException($e); if ($e instanceof DuplicateException) { - // Also tolerate client-reported duplicates in Skip/Upsert mode. - if ($tolerateExisting) { - return true; - } // Keep existing shared-tables/metadata behavior — no-op there. return true; } @@ -1118,15 +1112,7 @@ public function createIndex(string $collection, string $id, string $type, array return $result; } catch (\Exception $e) { - $err = $this->processException($e); - if ($err instanceof DuplicateException && $this->onDuplicate === OnDuplicate::Upsert) { - // Mongo raises IndexKeySpecsConflict when re-creating an index - // with a name that points to different keys/options. Drop the - // existing one and recreate with the new spec. - $this->deleteIndex($collection, $id); - return $this->client->createIndexes($name, [$indexes], $options); - } - throw $err; + throw $this->processException($e); } } diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index ade6a997e..a8e4f9830 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -243,18 +243,8 @@ public function createCollection(string $name, array $attributes = [], array $in $sqlTenant = $this->sharedTables ? '_tenant INTEGER DEFAULT NULL,' : ''; - $createTable = $this->onDuplicate !== OnDuplicate::Fail - ? 'CREATE TABLE IF NOT EXISTS' - : 'CREATE TABLE'; - $createIndex = $this->onDuplicate !== OnDuplicate::Fail - ? 'CREATE INDEX IF NOT EXISTS' - : 'CREATE INDEX'; - $createUniqueIndex = $this->onDuplicate !== OnDuplicate::Fail - ? 'CREATE UNIQUE INDEX IF NOT EXISTS' - : 'CREATE UNIQUE INDEX'; - $collection = " - {$createTable} {$this->getSQLTable($id)} ( + CREATE TABLE {$this->getSQLTable($id)} ( _id BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, _uid VARCHAR(255) NOT NULL, " . $sqlTenant . " @@ -271,26 +261,26 @@ public function createCollection(string $name, array $attributes = [], array $in $updatedIndex = $this->getShortKey("{$namespace}_{$this->tenant}_{$id}_updated"); $tenantIdIndex = $this->getShortKey("{$namespace}_{$this->tenant}_{$id}_tenant_id"); $collection .= " - {$createUniqueIndex} \"{$uidIndex}\" ON {$this->getSQLTable($id)} (\"_uid\" COLLATE utf8_ci_ai, \"_tenant\"); - {$createIndex} \"{$createdIndex}\" ON {$this->getSQLTable($id)} (_tenant, \"_createdAt\"); - {$createIndex} \"{$updatedIndex}\" ON {$this->getSQLTable($id)} (_tenant, \"_updatedAt\"); - {$createIndex} \"{$tenantIdIndex}\" ON {$this->getSQLTable($id)} (_tenant, _id); + CREATE UNIQUE INDEX \"{$uidIndex}\" ON {$this->getSQLTable($id)} (\"_uid\" COLLATE utf8_ci_ai, \"_tenant\"); + CREATE INDEX \"{$createdIndex}\" ON {$this->getSQLTable($id)} (_tenant, \"_createdAt\"); + CREATE INDEX \"{$updatedIndex}\" ON {$this->getSQLTable($id)} (_tenant, \"_updatedAt\"); + CREATE INDEX \"{$tenantIdIndex}\" ON {$this->getSQLTable($id)} (_tenant, _id); "; } else { $uidIndex = $this->getShortKey("{$namespace}_{$id}_uid"); $createdIndex = $this->getShortKey("{$namespace}_{$id}_created"); $updatedIndex = $this->getShortKey("{$namespace}_{$id}_updated"); $collection .= " - {$createUniqueIndex} \"{$uidIndex}\" ON {$this->getSQLTable($id)} (\"_uid\" COLLATE utf8_ci_ai); - {$createIndex} \"{$createdIndex}\" ON {$this->getSQLTable($id)} (\"_createdAt\"); - {$createIndex} \"{$updatedIndex}\" ON {$this->getSQLTable($id)} (\"_updatedAt\"); + CREATE UNIQUE INDEX \"{$uidIndex}\" ON {$this->getSQLTable($id)} (\"_uid\" COLLATE utf8_ci_ai); + CREATE INDEX \"{$createdIndex}\" ON {$this->getSQLTable($id)} (\"_createdAt\"); + CREATE INDEX \"{$updatedIndex}\" ON {$this->getSQLTable($id)} (\"_updatedAt\"); "; } $collection = $this->trigger(Database::EVENT_COLLECTION_CREATE, $collection); $permissions = " - {$createTable} {$this->getSQLTable($id . '_perms')} ( + CREATE TABLE {$this->getSQLTable($id . '_perms')} ( _id BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, _tenant INTEGER DEFAULT NULL, _type VARCHAR(12) NOT NULL, @@ -303,19 +293,19 @@ public function createCollection(string $name, array $attributes = [], array $in $uniquePermissionIndex = $this->getShortKey("{$namespace}_{$this->tenant}_{$id}_ukey"); $permissionIndex = $this->getShortKey("{$namespace}_{$this->tenant}_{$id}_permission"); $permissions .= " - {$createUniqueIndex} \"{$uniquePermissionIndex}\" + CREATE UNIQUE INDEX \"{$uniquePermissionIndex}\" ON {$this->getSQLTable($id . '_perms')} USING btree (_tenant,_document,_type,_permission); - {$createIndex} \"{$permissionIndex}\" - ON {$this->getSQLTable($id . '_perms')} USING btree (_tenant,_permission,_type); + CREATE INDEX \"{$permissionIndex}\" + ON {$this->getSQLTable($id . '_perms')} USING btree (_tenant,_permission,_type); "; } else { $uniquePermissionIndex = $this->getShortKey("{$namespace}_{$id}_ukey"); $permissionIndex = $this->getShortKey("{$namespace}_{$id}_permission"); $permissions .= " - {$createUniqueIndex} \"{$uniquePermissionIndex}\" + CREATE UNIQUE INDEX \"{$uniquePermissionIndex}\" ON {$this->getSQLTable($id . '_perms')} USING btree (_document COLLATE utf8_ci_ai,_type,_permission); - {$createIndex} \"{$permissionIndex}\" - ON {$this->getSQLTable($id . '_perms')} USING btree (_permission,_type); + CREATE INDEX \"{$permissionIndex}\" + ON {$this->getSQLTable($id . '_perms')} USING btree (_permission,_type); "; } @@ -495,34 +485,19 @@ public function createAttribute(string $collection, string $id, string $type, in } $name = $this->filter($collection); - $filteredId = $this->filter($id); - $sqlType = $this->getSQLType($type, $size, $signed, $array, $required); + $id = $this->filter($id); + $type = $this->getSQLType($type, $size, $signed, $array, $required); $sql = " ALTER TABLE {$this->getSQLTable($name)} - ADD COLUMN \"{$filteredId}\" {$sqlType} + ADD COLUMN \"{$id}\" {$type} "; $sql = $this->trigger(Database::EVENT_ATTRIBUTE_CREATE, $sql); - // Skip/Upsert: pre-check instead of catching a duplicate from the DDL. - // A failed ALTER TABLE inside a transaction would abort the outer - // transaction on Postgres, making the catch-and-retry pattern unsafe. - if ($this->onDuplicate !== OnDuplicate::Fail) { - $match = $this->attributeMatches($collection, $id, $type, $size, $signed, $array, $required); - if ($match === true) { - return true; - } - if ($match === false) { - if ($this->onDuplicate === OnDuplicate::Skip) { - return true; - } - $this->deleteAttribute($collection, $id, $array); - } - } - try { - return $this->execute($this->getPDO()->prepare($sql)); + return $this->execute($this->getPDO() + ->prepare($sql)); } catch (PDOException $e) { throw $this->processException($e); } @@ -922,9 +897,6 @@ public function createIndex(string $collection, string $id, string $type, array { $collection = $this->filter($collection); $id = $this->filter($id); - // Preserve raw attribute list for indexMatches — the loop below - // mutates $attributes into SQL-formatted column fragments. - $attrsRaw = $attributes; foreach ($attributes as $i => $attr) { $order = empty($orders[$i]) || Database::INDEX_FULLTEXT === $type ? '' : $orders[$i]; @@ -983,22 +955,6 @@ public function createIndex(string $collection, string $id, string $type, array $sql = $this->trigger(Database::EVENT_INDEX_CREATE, $sql); - // Skip/Upsert: pre-check via indexMatches() rather than catching a - // duplicate DDL error. A failed CREATE INDEX on Postgres aborts the - // surrounding transaction, so catch-and-retry is unsafe. - if ($this->onDuplicate !== OnDuplicate::Fail) { - $match = $this->indexMatches($collection, $id, $type, $attrsRaw, $lengths, $orders); - if ($match === true) { - return true; - } - if ($match === false) { - if ($this->onDuplicate === OnDuplicate::Skip) { - return true; - } - $this->deleteIndex($collection, $id); - } - } - try { return $this->getPDO()->prepare($sql)->execute(); } catch (PDOException $e) { @@ -1973,141 +1929,6 @@ protected function getFulltextValue(string $value): string return "'" . $value . "'"; } - protected function attributeMatches( - string $collection, - string $id, - string $type, - int $size, - bool $signed = true, - bool $array = false, - bool $required = false - ): ?bool { - // pg_catalog.format_type() emits the canonical textual form (e.g. - // "character varying(128)", "timestamp(3) without time zone") which - // normalizes more predictably than information_schema.data_type. - $stmt = $this->getPDO()->prepare(' - SELECT pg_catalog.format_type(a.atttypid, a.atttypmod) AS type - FROM pg_catalog.pg_attribute a - JOIN pg_catalog.pg_class c ON a.attrelid = c.oid - JOIN pg_catalog.pg_namespace n ON c.relnamespace = n.oid - WHERE n.nspname = :schema - AND c.relname = :table - AND a.attname = :column - AND a.attnum > 0 - AND NOT a.attisdropped - '); - $stmt->bindValue(':schema', $this->getDatabase(), PDO::PARAM_STR); - $stmt->bindValue(':table', "{$this->getNamespace()}_{$this->filter($collection)}", PDO::PARAM_STR); - $stmt->bindValue(':column', $this->filter($id), PDO::PARAM_STR); - $stmt->execute(); - $row = $stmt->fetch(PDO::FETCH_ASSOC); - if (empty($row)) { - return null; - } - - $target = $this->getSQLType($type, $size, $signed, $array, $required); - return $this->normalizeColumnType((string) $row['type']) === $this->normalizeColumnType($target); - } - - protected function normalizeColumnType(string $sql): string - { - $sql = \strtoupper(\trim($sql)); - // `character varying(128)` → `VARCHAR(128)` so it matches getSQLType output. - $sql = \str_replace('CHARACTER VARYING', 'VARCHAR', $sql); - // Postgres's format_type appends `without time zone` to timestamps; strip it. - $sql = \preg_replace('/\s*WITHOUT TIME ZONE\s*/', '', $sql); - $sql = \preg_replace('/\s*,\s*/', ',', $sql); - $sql = \preg_replace('/\s+/', ' ', $sql); - - return \trim($sql); - } - - /** - * Postgres stores indexes in pg_index; the relation name on disk is the - * short-keyed form produced by createIndex (namespace + tenant + - * collection + id, hashed if longer than the identifier limit). - * - * Fetches existing column list via unnest(indkey) joined to pg_attribute, - * and uniqueness via indisunique. Index method is looked up via pg_am - * (btree / gin / gist / hnsw). Lengths and per-column orders are ignored - * — a mismatch there yields a conservative false negative, never a false - * positive. - * - * @param array $attributes - * @param array $lengths - * @param array $orders - */ - protected function indexMatches( - string $collection, - string $id, - string $type, - array $attributes, - array $lengths = [], - array $orders = [], - ): ?bool { - $collectionFiltered = $this->filter($collection); - $idFiltered = $this->filter($id); - $keyName = $this->getShortKey("{$this->getNamespace()}_{$this->tenant}_{$collectionFiltered}_{$idFiltered}"); - - $stmt = $this->getPDO()->prepare(" - SELECT a.attname AS column_name, i.indisunique AS is_unique, am.amname AS method - FROM pg_catalog.pg_index i - JOIN pg_catalog.pg_class c ON c.oid = i.indexrelid - JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace - JOIN pg_catalog.pg_am am ON am.oid = c.relam - CROSS JOIN LATERAL unnest(i.indkey) WITH ORDINALITY AS k(attnum, pos) - JOIN pg_catalog.pg_attribute a ON a.attrelid = i.indrelid AND a.attnum = k.attnum - WHERE n.nspname = :schema - AND c.relname = :index - ORDER BY k.pos - "); - $stmt->bindValue(':schema', $this->getDatabase(), PDO::PARAM_STR); - $stmt->bindValue(':index', $keyName, PDO::PARAM_STR); - $stmt->execute(); - $rows = $stmt->fetchAll(PDO::FETCH_ASSOC); - - if (empty($rows)) { - return null; - } - - if ((bool) $rows[0]['is_unique'] !== ($type === Database::INDEX_UNIQUE)) { - return false; - } - - // Map declared index type to Postgres access method (pg_am.amname) - $targetMethod = match ($type) { - Database::INDEX_SPATIAL => 'gist', - Database::INDEX_HNSW_EUCLIDEAN, - Database::INDEX_HNSW_COSINE, - Database::INDEX_HNSW_DOT => 'hnsw', - Database::INDEX_OBJECT, - Database::INDEX_TRIGRAM => 'gin', - default => 'btree', // INDEX_KEY, INDEX_UNIQUE, INDEX_FULLTEXT - }; - if (\strtolower((string) $rows[0]['method']) !== $targetMethod) { - return false; - } - - // Target column list — mirror createIndex's transformation including - // shared-tables tenant prefix for KEY/UNIQUE indexes. - $targetCols = []; - foreach ($attributes as $attr) { - $targetCols[] = match ($attr) { - '$id' => '_uid', - '$createdAt' => '_createdAt', - '$updatedAt' => '_updatedAt', - default => $this->filter($attr), - }; - } - if ($this->sharedTables && \in_array($type, [Database::INDEX_KEY, Database::INDEX_UNIQUE])) { - \array_unshift($targetCols, '_tenant'); - } - - $existingCols = \array_map(fn ($r) => (string) $r['column_name'], $rows); - - return $targetCols === $existingCols; - } - /** * Get SQL Type * diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index 2011b1fba..9fd461563 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -246,31 +246,15 @@ public function list(): array */ public function createAttribute(string $collection, string $id, string $type, int $size, bool $signed = true, bool $array = false, bool $required = false): bool { - $quoted = $this->quote($this->filter($id)); - $sqlType = $this->getSQLType($type, $size, $signed, $array, $required); - $sql = "ALTER TABLE {$this->getSQLTable($collection)} ADD COLUMN {$quoted} {$sqlType} {$this->getLockType()};"; + $id = $this->quote($this->filter($id)); + $type = $this->getSQLType($type, $size, $signed, $array, $required); + $sql = "ALTER TABLE {$this->getSQLTable($collection)} ADD COLUMN {$id} {$type} {$this->getLockType()};"; $sql = $this->trigger(Database::EVENT_ATTRIBUTE_CREATE, $sql); - // Skip/Upsert: pre-check the existing column so we don't drive control - // flow through PDOException. attributeMatches returns null (doesn't - // exist), true (exists + matches spec), or false (exists + differs). - if ($this->onDuplicate !== OnDuplicate::Fail) { - $match = $this->attributeMatches($collection, $id, $type, $size, $signed, $array, $required); - if ($match === true) { - return true; - } - if ($match === false) { - if ($this->onDuplicate === OnDuplicate::Skip) { - return true; - } - // Upsert: column exists with a different type — drop and - // recreate so migration can repopulate via row Upsert. - $this->deleteAttribute($collection, $id, $array); - } - } - try { - return $this->getPDO()->prepare($sql)->execute(); + return $this->getPDO() + ->prepare($sql) + ->execute(); } catch (PDOException $e) { throw $this->processException($e); } @@ -1896,142 +1880,6 @@ abstract protected function getSQLType( bool $required = false ): string; - /** - * Returns whether the existing DB column for $collection.$id matches the - * requested declaration. Used by createAttribute under OnDuplicate::Upsert - * to decide between a no-op and a drop+recreate. - * - * Return values: true = exists and matches, false = exists but differs, - * null = doesn't exist (shouldn't happen from the Upsert path, but - * subclasses should handle it). - * - * Default implementation queries INFORMATION_SCHEMA and compares - * COLUMN_TYPE against getSQLType(). Adapters using non-INFORMATION_SCHEMA - * metadata (SQLite → PRAGMA, Postgres → pg_catalog) must override. - */ - protected function attributeMatches( - string $collection, - string $id, - string $type, - int $size, - bool $signed = true, - bool $array = false, - bool $required = false - ): ?bool { - $stmt = $this->getPDO()->prepare(' - SELECT COLUMN_TYPE - FROM INFORMATION_SCHEMA.COLUMNS - WHERE TABLE_SCHEMA = :schema - AND TABLE_NAME = :table - AND COLUMN_NAME = :column - '); - $stmt->bindValue(':schema', $this->getDatabase(), \PDO::PARAM_STR); - $stmt->bindValue(':table', "{$this->getNamespace()}_{$this->filter($collection)}", \PDO::PARAM_STR); - $stmt->bindValue(':column', $this->filter($id), \PDO::PARAM_STR); - $stmt->execute(); - $row = $stmt->fetch(\PDO::FETCH_ASSOC); - if (empty($row)) { - return null; - } - - $target = $this->getSQLType($type, $size, $signed, $array, $required); - return $this->normalizeColumnType((string) $row['COLUMN_TYPE']) === $this->normalizeColumnType($target); - } - - /** - * Normalize a SQL type declaration for equality comparison: uppercase, - * collapse whitespace, drop MySQL's legacy integer display widths - * (BIGINT(20) → BIGINT) — MySQL 8+ doesn't emit them and comparing them - * against MariaDB's output would otherwise cause false mismatches. - * TINYINT(1) is intentionally preserved because it's the canonical - * BOOLEAN representation. - */ - protected function normalizeColumnType(string $sql): string - { - $sql = \preg_replace('/\b(BIGINT|INT|SMALLINT|MEDIUMINT)\s*\(\s*\d+\s*\)/i', '$1', $sql); - $sql = \preg_replace('/\s+/', ' ', $sql); - - return \strtoupper(\trim($sql)); - } - - /** - * Returns whether the existing DB index for $collection.$id matches the - * requested declaration. Mirrors attributeMatches() semantics: - * - null = doesn't exist - * - true = exists and matches spec - * - false = exists but differs - * - * Comparison covers column list (ordered), uniqueness flag, and index - * kind (BTREE/FULLTEXT/SPATIAL). Per-column lengths and sort orders are - * intentionally ignored — a difference there yields a false negative - * (unnecessary rebuild), never a false positive, so it's conservative. - * - * Default implementation uses INFORMATION_SCHEMA.STATISTICS (one row per - * column in the index); MariaDB/MySQL inherit. Postgres and SQLite - * override with dialect-specific metadata queries. - * - * @param array $attributes - * @param array $lengths - * @param array $orders - */ - protected function indexMatches( - string $collection, - string $id, - string $type, - array $attributes, - array $lengths = [], - array $orders = [], - ): ?bool { - $stmt = $this->getPDO()->prepare(' - SELECT COLUMN_NAME, NON_UNIQUE, INDEX_TYPE - FROM INFORMATION_SCHEMA.STATISTICS - WHERE TABLE_SCHEMA = :schema - AND TABLE_NAME = :table - AND INDEX_NAME = :index - ORDER BY SEQ_IN_INDEX - '); - $stmt->bindValue(':schema', $this->getDatabase(), \PDO::PARAM_STR); - $stmt->bindValue(':table', "{$this->getNamespace()}_{$this->filter($collection)}", \PDO::PARAM_STR); - $stmt->bindValue(':index', $this->filter($id), \PDO::PARAM_STR); - $stmt->execute(); - $rows = $stmt->fetchAll(\PDO::FETCH_ASSOC); - - if (empty($rows)) { - return null; - } - - // Uniqueness - if (((int) $rows[0]['NON_UNIQUE'] === 0) !== ($type === Database::INDEX_UNIQUE)) { - return false; - } - - // Index kind - $existingKind = \strtoupper((string) $rows[0]['INDEX_TYPE']); - $targetKind = match ($type) { - Database::INDEX_FULLTEXT => 'FULLTEXT', - Database::INDEX_SPATIAL => 'SPATIAL', - default => 'BTREE', - }; - if ($existingKind !== $targetKind) { - return false; - } - - // Column list — reconstruct the exact sequence that createIndex would - // have written, including the tenant prefix for shared-tables key / - // unique indexes. Must stay in sync with createIndex. - $targetCols = []; - foreach ($attributes as $attr) { - $targetCols[] = $this->filter($this->getInternalKeyForAttribute($attr)); - } - if ($this->sharedTables && $type !== Database::INDEX_FULLTEXT && $type !== Database::INDEX_SPATIAL) { - \array_unshift($targetCols, '_tenant'); - } - - $existingCols = \array_map(fn ($r) => (string) $r['COLUMN_NAME'], $rows); - - return $targetCols === $existingCols; - } - /** * @throws DatabaseException For unknown type values. */ diff --git a/src/Database/Adapter/SQLite.php b/src/Database/Adapter/SQLite.php index c8924fae7..411457116 100644 --- a/src/Database/Adapter/SQLite.php +++ b/src/Database/Adapter/SQLite.php @@ -165,12 +165,8 @@ public function createCollection(string $name, array $attributes = [], array $in $tenantQuery = $this->sharedTables ? '`_tenant` INTEGER DEFAULT NULL,' : ''; - $createTable = $this->onDuplicate !== OnDuplicate::Fail - ? 'CREATE TABLE IF NOT EXISTS' - : 'CREATE TABLE'; - $collection = " - {$createTable} {$this->getSQLTable($id)} ( + CREATE TABLE {$this->getSQLTable($id)} ( `_id` INTEGER PRIMARY KEY AUTOINCREMENT, `_uid` VARCHAR(36) NOT NULL, {$tenantQuery} @@ -184,7 +180,7 @@ public function createCollection(string $name, array $attributes = [], array $in $collection = $this->trigger(Database::EVENT_COLLECTION_CREATE, $collection); $permissions = " - {$createTable} {$this->getSQLTable($id . '_perms')} ( + CREATE TABLE {$this->getSQLTable($id . '_perms')} ( `_id` INTEGER PRIMARY KEY AUTOINCREMENT, {$tenantQuery} `_type` VARCHAR(12) NOT NULL, @@ -464,36 +460,29 @@ public function renameIndex(string $collection, string $old, string $new): bool public function createIndex(string $collection, string $id, string $type, array $attributes, array $lengths, array $orders, array $indexAttributeTypes = [], array $collation = [], int $ttl = 1): bool { $name = $this->filter($collection); - $filteredId = $this->filter($id); - - // Pre-check runs in ALL modes on SQLite: createCollection intentionally - // issues the same CREATE INDEX twice for _perms side-tables, relying on - // the historical "always idempotent" behavior of this method. Matching - // spec → no-op in every mode; differing spec on Upsert rebuilds, on - // Skip/Fail tolerates (historical contract). - $match = $this->indexMatches($collection, $id, $type, $attributes, $lengths, $orders); - if ($match === true) { + $id = $this->filter($id); + + // Workaround for no support for CREATE INDEX IF NOT EXISTS + $stmt = $this->getPDO()->prepare(" + SELECT name + FROM sqlite_master + WHERE type='index' AND name=:_index; + "); + $stmt->bindValue(':_index', "{$this->getNamespace()}_{$this->tenant}_{$name}_{$id}"); + $stmt->execute(); + $index = $stmt->fetch(); + $stmt->closeCursor(); + if (!empty($index)) { return true; } - if ($match === false) { - if ($this->onDuplicate === OnDuplicate::Upsert) { - $this->deleteIndex($collection, $id); - } else { - return true; - } - } - $sql = $this->getSQLIndex($name, $filteredId, $type, $attributes); + $sql = $this->getSQLIndex($name, $id, $type, $attributes); $sql = $this->trigger(Database::EVENT_INDEX_CREATE, $sql); - try { - return $this->getPDO() - ->prepare($sql) - ->execute(); - } catch (PDOException $e) { - throw $this->processException($e); - } + return $this->getPDO() + ->prepare($sql) + ->execute(); } /** @@ -1977,89 +1966,4 @@ protected function getInsertSuffix(string $table, array $columns = []): string return ''; } - /** - * SQLite stores declared types verbatim (type affinity only matters at - * value-storage time), so `PRAGMA table_info` returns whatever we emitted - * from getSQLType. Compare declared form against target after the usual - * normalization (strips MariaDB integer display widths, uppercases). - */ - protected function attributeMatches( - string $collection, - string $id, - string $type, - int $size, - bool $signed = true, - bool $array = false, - bool $required = false - ): ?bool { - $table = "{$this->getNamespace()}_{$this->filter($collection)}"; - $stmt = $this->getPDO()->prepare('SELECT type FROM pragma_table_info(:table) WHERE name = :column'); - $stmt->bindValue(':table', $table, PDO::PARAM_STR); - $stmt->bindValue(':column', $this->filter($id), PDO::PARAM_STR); - $stmt->execute(); - $row = $stmt->fetch(PDO::FETCH_ASSOC); - if (empty($row)) { - return null; - } - - $target = $this->getSQLType($type, $size, $signed, $array, $required); - return $this->normalizeColumnType((string) $row['type']) === $this->normalizeColumnType($target); - } - - /** - * SQLite has no INFORMATION_SCHEMA; indexes live in sqlite_master keyed - * by their fully namespaced name. PRAGMA index_list returns uniqueness, - * PRAGMA index_info returns columns in order. Close cursors before any - * DDL — SQLite holds a table-level lock while a read cursor is open. - * - * @param array $attributes - * @param array $lengths - * @param array $orders - */ - protected function indexMatches( - string $collection, - string $id, - string $type, - array $attributes, - array $lengths = [], - array $orders = [], - ): ?bool { - $collectionFiltered = $this->filter($collection); - $idFiltered = $this->filter($id); - $table = "{$this->getNamespace()}_{$collectionFiltered}"; - $indexName = "{$this->getNamespace()}_{$this->tenant}_{$collectionFiltered}_{$idFiltered}"; - - $stmt = $this->getPDO()->prepare('SELECT "unique" FROM pragma_index_list(:table) WHERE name = :index'); - $stmt->bindValue(':table', $table, PDO::PARAM_STR); - $stmt->bindValue(':index', $indexName, PDO::PARAM_STR); - $stmt->execute(); - $list = $stmt->fetch(PDO::FETCH_ASSOC); - $stmt->closeCursor(); - if (empty($list)) { - return null; - } - - if ((bool) $list['unique'] !== ($type === Database::INDEX_UNIQUE)) { - return false; - } - - $stmt = $this->getPDO()->prepare('SELECT name FROM pragma_index_info(:index) ORDER BY seqno'); - $stmt->bindValue(':index', $indexName, PDO::PARAM_STR); - $stmt->execute(); - $existingCols = \array_map(fn ($r) => (string) $r['name'], $stmt->fetchAll(PDO::FETCH_ASSOC)); - $stmt->closeCursor(); - - // Mirror SQLite's getSQLIndex transformation for target column list. - // Shared-tables tenant prefix: SQLite follows MariaDB's behavior since - // it extends MariaDB — _tenant prepended for KEY/UNIQUE. - $targetCols = []; - foreach ($attributes as $attr) { - $targetCols[] = $this->filter($this->getInternalKeyForAttribute($attr)); - } - if ($this->sharedTables && $type !== Database::INDEX_FULLTEXT && $type !== Database::INDEX_SPATIAL) { - \array_unshift($targetCols, '_tenant'); - } - - return $targetCols === $existingCols; - } } diff --git a/src/Database/Database.php b/src/Database/Database.php index 17b4cc73a..7bca15026 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -1721,6 +1721,13 @@ public function createCollection(string $id, array $attributes = [], array $inde $collection = $this->silent(fn () => $this->getCollection($id)); if (!$collection->isEmpty() && $id !== self::METADATA) { + // Skip/Upsert: collection data is never destroyed — both modes + // tolerate the existing collection and return its current metadata + // document. Per-attribute / per-index reconciliation happens via + // the dedicated createAttribute / createIndex paths. + if ($this->onDuplicate !== OnDuplicate::Fail) { + return $collection; + } throw new DuplicateException('Collection ' . $id . ' already exists'); } @@ -2158,6 +2165,20 @@ public function createAttribute(string $collection, string $id, string $type, in $filters = array_unique($filters); } + // Skip/Upsert: if the attribute already exists in metadata, tolerate + // and return. Spec reconciliation (drop + recreate on type change) is + // a caller concern — migration consults source vs destination metadata + // _updatedAt and issues deleteAttribute before a re-creation itself, + // so by the time this is called the attribute is either truly new or + // intentionally unchanged. + if ($this->onDuplicate !== OnDuplicate::Fail) { + foreach ($collection->getAttribute('attributes', []) as $existing) { + if (\strtolower($existing->getAttribute('key', $existing->getId())) === \strtolower($id)) { + return true; + } + } + } + $existsInSchema = false; $schemaAttributes = $this->adapter->getSupportForSchemaAttributes() @@ -4528,9 +4549,15 @@ public function createIndex(string $collection, string $id, string $type, array /** @var array $indexes */ foreach ($indexes as $index) { - if (\strtolower($index->getId()) === \strtolower($id)) { - throw new DuplicateException('Index already exists'); + if (\strtolower($index->getId()) !== \strtolower($id)) { + continue; + } + // Skip/Upsert: tolerate the existing index. Caller (e.g. migration) + // is responsible for dropping it first if the spec needs to change. + if ($this->onDuplicate !== OnDuplicate::Fail) { + return true; } + throw new DuplicateException('Index already exists'); } if ($this->adapter->getCountOfIndexes($collection) >= $this->adapter->getLimitForIndexes()) { diff --git a/src/Database/Mirror.php b/src/Database/Mirror.php index 3c704170e..7a76af2e2 100644 --- a/src/Database/Mirror.php +++ b/src/Database/Mirror.php @@ -205,12 +205,23 @@ public function delete(?string $database = null): bool public function createCollection(string $id, array $attributes = [], array $indexes = [], ?array $permissions = null, bool $documentSecurity = true): Document { - $result = $this->source->createCollection( - $id, - $attributes, - $indexes, - $permissions, - $documentSecurity + // Forward the OnDuplicate scope to source/destination so their + // Database-layer dispatch observes it. Skip/Upsert tolerate an + // existing collection; Fail rethrows as before. + $forward = fn (Database $target, callable $call) => + $this->onDuplicate !== OnDuplicate::Fail + ? $target->withOnDuplicate($this->onDuplicate, $call) + : $call(); + + $result = $forward( + $this->source, + fn () => $this->source->createCollection( + $id, + $attributes, + $indexes, + $permissions, + $documentSecurity + ) ); if ($this->destination === null) { @@ -227,12 +238,15 @@ public function createCollection(string $id, array $attributes = [], array $inde ); } - $this->destination->createCollection( - $id, - $attributes, - $indexes, - $permissions, - $documentSecurity + $forward( + $this->destination, + fn () => $this->destination->createCollection( + $id, + $attributes, + $indexes, + $permissions, + $documentSecurity + ) ); $this->silent(function () use ($id) { @@ -303,18 +317,26 @@ public function deleteCollection(string $id): bool public function createAttribute(string $collection, string $id, string $type, int $size, bool $required, $default = null, bool $signed = true, bool $array = false, ?string $format = null, array $formatOptions = [], array $filters = []): bool { - $result = $this->source->createAttribute( - $collection, - $id, - $type, - $size, - $required, - $default, - $signed, - $array, - $format, - $formatOptions, - $filters + $forward = fn (Database $target, callable $call) => + $this->onDuplicate !== OnDuplicate::Fail + ? $target->withOnDuplicate($this->onDuplicate, $call) + : $call(); + + $result = $forward( + $this->source, + fn () => $this->source->createAttribute( + $collection, + $id, + $type, + $size, + $required, + $default, + $signed, + $array, + $format, + $formatOptions, + $filters + ) ); if ($this->destination === null) { @@ -345,18 +367,21 @@ public function createAttribute(string $collection, string $id, string $type, in ); } - $result = $this->destination->createAttribute( - $collection, - $document->getId(), - $document->getAttribute('type'), - $document->getAttribute('size'), - $document->getAttribute('required'), - $document->getAttribute('default'), - $document->getAttribute('signed'), - $document->getAttribute('array'), - $document->getAttribute('format'), - $document->getAttribute('formatOptions'), - $document->getAttribute('filters'), + $result = $forward( + $this->destination, + fn () => $this->destination->createAttribute( + $collection, + $document->getId(), + $document->getAttribute('type'), + $document->getAttribute('size'), + $document->getAttribute('required'), + $document->getAttribute('default'), + $document->getAttribute('signed'), + $document->getAttribute('array'), + $document->getAttribute('format'), + $document->getAttribute('formatOptions'), + $document->getAttribute('filters'), + ) ); } catch (\Throwable $err) { $this->logError('createAttribute', $err); @@ -480,7 +505,15 @@ public function deleteAttribute(string $collection, string $id): bool public function createIndex(string $collection, string $id, string $type, array $attributes, array $lengths = [], array $orders = [], int $ttl = 1): bool { - $result = $this->source->createIndex($collection, $id, $type, $attributes, $lengths, $orders, $ttl); + $forward = fn (Database $target, callable $call) => + $this->onDuplicate !== OnDuplicate::Fail + ? $target->withOnDuplicate($this->onDuplicate, $call) + : $call(); + + $result = $forward( + $this->source, + fn () => $this->source->createIndex($collection, $id, $type, $attributes, $lengths, $orders, $ttl) + ); if ($this->destination === null) { return $result; @@ -505,14 +538,17 @@ public function createIndex(string $collection, string $id, string $type, array ); } - $result = $this->destination->createIndex( - $collection, - $document->getId(), - $document->getAttribute('type'), - $document->getAttribute('attributes'), - $document->getAttribute('lengths'), - $document->getAttribute('orders'), - $document->getAttribute('ttl', 0) + $result = $forward( + $this->destination, + fn () => $this->destination->createIndex( + $collection, + $document->getId(), + $document->getAttribute('type'), + $document->getAttribute('attributes'), + $document->getAttribute('lengths'), + $document->getAttribute('orders'), + $document->getAttribute('ttl', 0) + ) ); } catch (\Throwable $err) { $this->logError('createIndex', $err); diff --git a/tests/e2e/Adapter/Scopes/DocumentTests.php b/tests/e2e/Adapter/Scopes/DocumentTests.php index ecdd084e0..24ca56e70 100644 --- a/tests/e2e/Adapter/Scopes/DocumentTests.php +++ b/tests/e2e/Adapter/Scopes/DocumentTests.php @@ -8346,167 +8346,115 @@ public function testCreateDocsUpsertAll(): void } /** - * OnDuplicate::Upsert on createAttribute with an existing column of the - * SAME type is a no-op — row data must be preserved. + * OnDuplicate::Skip and Upsert tolerate an existing collection — they + * return the current metadata document instead of throwing. Collections + * are never destructive at the library layer; callers that need to + * reconcile schema drop/recreate the individual attributes / indexes. */ - public function testUpsertAttributeSameTypeNoop(): void + public function testCreateCollSkipUpsertTolerates(): void { /** @var Database $database */ $database = $this->getDatabase(); - if (!$database->getAdapter()->getSupportForAttributes()) { - $this->expectNotToPerformAssertions(); - return; - } - $database->createCollection(__FUNCTION__); $database->createAttribute(__FUNCTION__, 'name', Database::VAR_STRING, 128, true); - $database->createDocument(__FUNCTION__, new Document([ '$id' => 'doc', - 'name' => 'preserve-me', + 'name' => 'keep', '$permissions' => [Permission::read(Role::any())], ])); $collection = __FUNCTION__; - $database->withOnDuplicate(OnDuplicate::Upsert, function () use ($database, $collection) { - $database->getAdapter()->createAttribute($collection, 'name', Database::VAR_STRING, 128, true); - }); + $database->withOnDuplicate(OnDuplicate::Skip, fn () => $database->createCollection($collection)); + $this->assertSame('keep', $database->getDocument(__FUNCTION__, 'doc')->getAttribute('name')); - $doc = $database->getDocument(__FUNCTION__, 'doc'); - $this->assertSame('preserve-me', $doc->getAttribute('name')); + $database->withOnDuplicate(OnDuplicate::Upsert, fn () => $database->createCollection($collection)); + $this->assertSame('keep', $database->getDocument(__FUNCTION__, 'doc')->getAttribute('name')); } /** - * OnDuplicate::Upsert on createAttribute with a DIFFERENT type drops the - * existing column and recreates it with the new spec. Verified at the - * adapter layer via a follow-up createAttribute under Fail — which would - * throw DuplicateException if the column hadn't actually been dropped. + * OnDuplicate::Skip and Upsert tolerate an existing attribute. Caller is + * responsible for dropping first if the spec needs to change (migration + * consults source vs destination _metadata._updatedAt to decide). */ - public function testUpsertAttrTypeChangedRecreates(): void + public function testCreateAttrSkipUpsertTolerates(): void { /** @var Database $database */ $database = $this->getDatabase(); - if (!$database->getAdapter()->getSupportForAttributes()) { - $this->expectNotToPerformAssertions(); - return; - } - $database->createCollection(__FUNCTION__); - $database->createAttribute(__FUNCTION__, 'payload', Database::VAR_STRING, 64, true); + $database->createAttribute(__FUNCTION__, 'name', Database::VAR_STRING, 128, true); + + $database->createDocument(__FUNCTION__, new Document([ + '$id' => 'doc', + 'name' => 'keep', + '$permissions' => [Permission::read(Role::any())], + ])); $collection = __FUNCTION__; - // Upsert with a WIDER size: must drop the old VARCHAR(64) and recreate - // VARCHAR(256). - $result = $database->withOnDuplicate(OnDuplicate::Upsert, function () use ($database, $collection) { - return $database->getAdapter()->createAttribute($collection, 'payload', Database::VAR_STRING, 256, true); - }); - $this->assertTrue($result); - // Second Upsert with the SAME (new) size must be a cheap no-op — the - // matches-check returns true and no DDL runs. - $result = $database->withOnDuplicate(OnDuplicate::Upsert, function () use ($database, $collection) { - return $database->getAdapter()->createAttribute($collection, 'payload', Database::VAR_STRING, 256, true); - }); - $this->assertTrue($result); + // Skip: same-spec re-declare is a no-op. + $this->assertTrue($database->withOnDuplicate( + OnDuplicate::Skip, + fn () => $database->createAttribute($collection, 'name', Database::VAR_STRING, 128, true) + )); - // Sanity: deleteAttribute succeeds, confirming the column is really there. - $this->assertTrue($database->getAdapter()->deleteAttribute(__FUNCTION__, 'payload')); + // Skip: even a wider-spec re-declare is tolerated (not applied). + $this->assertTrue($database->withOnDuplicate( + OnDuplicate::Skip, + fn () => $database->createAttribute($collection, 'name', Database::VAR_STRING, 512, true) + )); + + // Upsert: same — tolerate existing. Migration handles drop+recreate itself. + $this->assertTrue($database->withOnDuplicate( + OnDuplicate::Upsert, + fn () => $database->createAttribute($collection, 'name', Database::VAR_STRING, 512, true) + )); + + // Metadata still reflects the ORIGINAL spec — library didn't touch it. + $nameAttr = null; + foreach ($database->getCollection(__FUNCTION__)->getAttribute('attributes', []) as $attr) { + if ($attr->getAttribute('key') === 'name') { + $nameAttr = $attr; + break; + } + } + $this->assertNotNull($nameAttr); + $this->assertSame(128, (int) $nameAttr->getAttribute('size')); + $this->assertSame('keep', $database->getDocument(__FUNCTION__, 'doc')->getAttribute('name')); } /** - * OnDuplicate::Upsert on createIndex over an existing index name rebuilds - * the index — the end state matches the requested spec. + * OnDuplicate::Skip and Upsert tolerate an existing index. End state is + * always the first-declared spec; callers that need a different spec + * deleteIndex() first. */ - public function testUpsertIndexRebuilds(): void + public function testCreateIdxSkipUpsertTolerates(): void { /** @var Database $database */ $database = $this->getDatabase(); - if (!$database->getAdapter()->getSupportForAttributes()) { - $this->expectNotToPerformAssertions(); - return; - } - $database->createCollection(__FUNCTION__); $database->createAttribute(__FUNCTION__, 'a', Database::VAR_STRING, 64, true); $database->createAttribute(__FUNCTION__, 'b', Database::VAR_STRING, 64, true); $database->createIndex(__FUNCTION__, 'idx', Database::INDEX_KEY, ['a']); $collection = __FUNCTION__; - $result = $database->withOnDuplicate(OnDuplicate::Upsert, function () use ($database, $collection) { - return $database->getAdapter()->createIndex($collection, 'idx', Database::INDEX_KEY, ['b'], [], []); - }); - $this->assertTrue($result); - $this->assertTrue($database->getAdapter()->deleteIndex(__FUNCTION__, 'idx')); - } - - /** - * OnDuplicate::Upsert on createIndex with a matching spec is a cheap - * no-op — indexMatches() returns true, so no DROP / CREATE fires. Proves - * the symmetry with testUpsertAttributeSameTypeNoop. - */ - public function testUpsertIndexSameSpecNoop(): void - { - /** @var Database $database */ - $database = $this->getDatabase(); - - if (!$database->getAdapter()->getSupportForAttributes()) { - $this->expectNotToPerformAssertions(); - return; - } - - $database->createCollection(__FUNCTION__); - $database->createAttribute(__FUNCTION__, 'name', Database::VAR_STRING, 128, true); - $database->createIndex(__FUNCTION__, 'idx', Database::INDEX_KEY, ['name']); - - // Same-spec re-declare under Upsert → indexMatches returns true, no DDL. - // We can't directly observe "no rebuild happened" without a spy, but - // we can assert the operation completed and the index is still there. - $collection = __FUNCTION__; - $result = $database->withOnDuplicate(OnDuplicate::Upsert, function () use ($database, $collection) { - return $database->getAdapter()->createIndex($collection, 'idx', Database::INDEX_KEY, ['name'], [], []); - }); - $this->assertTrue($result); - - // Deleting it must succeed — confirming the index is exactly where - // it was before the Upsert no-op. - $this->assertTrue($database->getAdapter()->deleteIndex(__FUNCTION__, 'idx')); - } - - /** - * OnDuplicate::Skip on createAttribute / createIndex tolerates pre-existing - * resources without modifying them. - */ - public function testSkipSchemaTolerates(): void - { - /** @var Database $database */ - $database = $this->getDatabase(); - - if (!$database->getAdapter()->getSupportForAttributes()) { - $this->expectNotToPerformAssertions(); - return; - } - - $database->createCollection(__FUNCTION__); - $database->createAttribute(__FUNCTION__, 'name', Database::VAR_STRING, 128, true); - $database->createIndex(__FUNCTION__, 'idx', Database::INDEX_KEY, ['name']); - - $database->createDocument(__FUNCTION__, new Document([ - '$id' => 'doc', - 'name' => 'keep', - '$permissions' => [Permission::read(Role::any())], - ])); - - $collection = __FUNCTION__; - $database->withOnDuplicate(OnDuplicate::Skip, function () use ($database, $collection) { - $database->getAdapter()->createAttribute($collection, 'name', Database::VAR_STRING, 512, true); - $database->getAdapter()->createIndex($collection, 'idx', Database::INDEX_KEY, ['name'], [], []); - }); - - $doc = $database->getDocument(__FUNCTION__, 'doc'); - $this->assertSame('keep', $doc->getAttribute('name')); + $this->assertTrue($database->withOnDuplicate( + OnDuplicate::Skip, + fn () => $database->createIndex($collection, 'idx', Database::INDEX_KEY, ['b']) + )); + + $this->assertTrue($database->withOnDuplicate( + OnDuplicate::Upsert, + fn () => $database->createIndex($collection, 'idx', Database::INDEX_KEY, ['b']) + )); + + // Metadata still reflects the original column — library tolerates, + // caller must drop first to change spec. + $indexes = $database->getCollection(__FUNCTION__)->getAttribute('indexes', []); + $this->assertCount(1, $indexes); + $this->assertSame(['a'], $indexes[0]->getAttribute('attributes')); } }