-
Notifications
You must be signed in to change notification settings - Fork 55
Add ignore param to createDocuments for silent duplicate handling #851
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
287420b
2b4fd1e
37aace4
9653052
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1365,6 +1365,35 @@ public function updateDocument(Document $collection, string $id, Document $docum | |||||||||||||||||||||
| return $document; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| protected function getInsertKeyword(bool $ignore): string | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| return 'INSERT INTO'; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| protected function getInsertSuffix(bool $ignore, string $table): string | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| if (!$ignore) { | ||||||||||||||||||||||
| return ''; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| $conflictTarget = $this->sharedTables ? '("_uid", "_tenant")' : '("_uid")'; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return "ON CONFLICT {$conflictTarget} DO NOTHING"; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| protected function getInsertPermissionsSuffix(bool $ignore): string | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| if (!$ignore) { | ||||||||||||||||||||||
| return ''; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| $conflictTarget = $this->sharedTables | ||||||||||||||||||||||
| ? '("_type", "_permission", "_document", "_tenant")' | ||||||||||||||||||||||
| : '("_type", "_permission", "_document")'; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return "ON CONFLICT {$conflictTarget} DO NOTHING"; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
+1373
to
+1395
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Both
Actual unique indexes vs. conflict targets:
The fixes: // getInsertSuffix – main document table
protected function getInsertSuffix(bool $ignore, string $table): string
{
if (!$ignore) {
return '';
}
$conflictTarget = $this->sharedTables
? '("_uid" COLLATE utf8_ci_ai, "_tenant")'
: '("_uid" COLLATE utf8_ci_ai)';
return "ON CONFLICT {$conflictTarget} DO NOTHING";
}
// getInsertPermissionsSuffix – permissions table
protected function getInsertPermissionsSuffix(bool $ignore): string
{
if (!$ignore) {
return '';
}
$conflictTarget = $this->sharedTables
? '("_type", "_permission", "_document", "_tenant")'
: '("_document" COLLATE utf8_ci_ai, "_type", "_permission")';
return "ON CONFLICT {$conflictTarget} DO NOTHING";
}Without this fix,
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The existing upsertDocuments at [Postgres.php:1462] uses ("_uid") without COLLATE and works in production |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| /** | ||||||||||||||||||||||
| * @param string $tableName | ||||||||||||||||||||||
| * @param string $columns | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2471,11 +2471,59 @@ protected function execute(mixed $stmt): bool | |
| * @throws DuplicateException | ||
| * @throws \Throwable | ||
| */ | ||
| public function createDocuments(Document $collection, array $documents): array | ||
| public function createDocuments(Document $collection, array $documents, bool $ignore = false): array | ||
| { | ||
| if (empty($documents)) { | ||
| return $documents; | ||
| } | ||
|
|
||
| // Pre-filter duplicates inside the transaction to prevent race conditions. | ||
| // Query which UIDs already exist and remove them from the batch. | ||
| if ($ignore) { | ||
| $collectionId = $collection->getId(); | ||
| $name = $this->filter($collectionId); | ||
| $uids = \array_filter(\array_map(fn (Document $doc) => $doc->getId(), $documents)); | ||
|
|
||
| if (!empty($uids)) { | ||
| $placeholders = []; | ||
| $binds = []; | ||
| foreach (\array_values(\array_unique($uids)) as $i => $uid) { | ||
| $key = ':_dup_uid_' . $i; | ||
| $placeholders[] = $key; | ||
| $binds[$key] = $uid; | ||
| } | ||
|
|
||
| $tenantFilter = ''; | ||
| if ($this->sharedTables) { | ||
| $tenantFilter = ' AND _tenant = :_dup_tenant'; | ||
| $binds[':_dup_tenant'] = $this->getTenant(); | ||
| } | ||
|
Comment on lines
+2496
to
+2500
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Those undetected duplicates are passed to The minimal fix is to filter by |
||
|
|
||
| $sql = 'SELECT _uid FROM ' . $this->getSQLTable($name) | ||
| . ' WHERE _uid IN (' . \implode(', ', $placeholders) . ')' | ||
| . $tenantFilter; | ||
|
|
||
| $stmt = $this->getPDO()->prepare($sql); | ||
| foreach ($binds as $k => $v) { | ||
| $stmt->bindValue($k, $v, $this->getPDOType($v)); | ||
| } | ||
| $stmt->execute(); | ||
| $existingUids = \array_flip(\array_column($stmt->fetchAll(), '_uid')); | ||
| $stmt->closeCursor(); | ||
|
|
||
| if (!empty($existingUids)) { | ||
| $documents = \array_values(\array_filter( | ||
| $documents, | ||
| fn (Document $doc) => !isset($existingUids[$doc->getId()]) | ||
| )); | ||
| } | ||
| } | ||
|
|
||
| if (empty($documents)) { | ||
| return []; | ||
| } | ||
| } | ||
|
|
||
| $spatialAttributes = $this->getSpatialAttributes($collection); | ||
| $collection = $collection->getId(); | ||
| try { | ||
|
|
@@ -2573,8 +2621,9 @@ public function createDocuments(Document $collection, array $documents): array | |
| $batchKeys = \implode(', ', $batchKeys); | ||
|
|
||
| $stmt = $this->getPDO()->prepare(" | ||
| INSERT INTO {$this->getSQLTable($name)} {$columns} | ||
| {$this->getInsertKeyword($ignore)} {$this->getSQLTable($name)} {$columns} | ||
| VALUES {$batchKeys} | ||
| {$this->getInsertSuffix($ignore, $name)} | ||
| "); | ||
|
|
||
| foreach ($bindValues as $key => $value) { | ||
|
|
@@ -2588,8 +2637,9 @@ public function createDocuments(Document $collection, array $documents): array | |
| $permissions = \implode(', ', $permissions); | ||
|
|
||
| $sqlPermissions = " | ||
| INSERT INTO {$this->getSQLTable($name . '_perms')} (_type, _permission, _document {$tenantColumn}) | ||
| VALUES {$permissions}; | ||
| {$this->getInsertKeyword($ignore)} {$this->getSQLTable($name . '_perms')} (_type, _permission, _document {$tenantColumn}) | ||
| VALUES {$permissions} | ||
| {$this->getInsertPermissionsSuffix($ignore)} | ||
| "; | ||
|
|
||
| $stmtPermissions = $this->getPDO()->prepare($sqlPermissions); | ||
|
|
@@ -2608,6 +2658,33 @@ public function createDocuments(Document $collection, array $documents): array | |
| return $documents; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the INSERT keyword, optionally with IGNORE for duplicate handling. | ||
| * Override in adapter subclasses for DB-specific syntax. | ||
| */ | ||
| protected function getInsertKeyword(bool $ignore): string | ||
| { | ||
| return $ignore ? 'INSERT IGNORE INTO' : 'INSERT INTO'; | ||
| } | ||
|
|
||
| /** | ||
| * Returns a suffix appended after VALUES clause for duplicate handling. | ||
| * Override in adapter subclasses (e.g., Postgres uses ON CONFLICT DO NOTHING). | ||
| */ | ||
| protected function getInsertSuffix(bool $ignore, string $table): string | ||
| { | ||
| return ''; | ||
| } | ||
|
|
||
| /** | ||
| * Returns a suffix for the permissions INSERT statement when ignoring duplicates. | ||
| * Override in adapter subclasses for DB-specific syntax. | ||
| */ | ||
| protected function getInsertPermissionsSuffix(bool $ignore): string | ||
| { | ||
| return ''; | ||
| } | ||
|
|
||
| /** | ||
| * @param Document $collection | ||
| * @param string $attribute | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.