Add ignore param to createDocuments for silent duplicate handling#852
Add ignore param to createDocuments for silent duplicate handling#852premtsd-code wants to merge 4 commits intomainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an optional Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Database
participant Adapter
participant Storage as DB
rect rgba(200,100,150,0.5)
Note over Client,Storage: Existing flow (ignore: false)
Client->>Database: createDocuments(docs, ignore: false)
Database->>Database: prepare documents, no pre-filter
Database->>Adapter: createDocuments(docs, ignore: false)
Adapter->>DB: INSERT INTO ...
DB-->>Adapter: ✗ duplicate error (may throw)
Adapter-->>Database: throws DuplicateException
Database-->>Client: exception propagated
end
sequenceDiagram
participant Client
participant Database
participant Adapter
participant Storage as DB
rect rgba(100,180,120,0.5)
Note over Client,Storage: New flow (ignore: true)
Client->>Database: createDocuments(docs, ignore: true)
Database->>Database: intra-batch dedup by id / tenant:id
Database->>Adapter: find(existing ids) [chunked]
Adapter->>DB: SELECT _uid WHERE _uid IN (...)
DB-->>Adapter: existing ids
Adapter-->>Database: $preExistingIds
Database->>Database: filter out pre-existing docs, strip relationships
Database->>Adapter: createDocuments(filtered_docs, ignore: true)
Adapter->>DB: INSERT IGNORE / ON CONFLICT DO NOTHING
DB-->>Adapter: ✓ success (duplicates skipped)
Adapter-->>Database: inserted rows info
Database->>Database: create relationships for actually inserted docs
Database-->>Client: count (new inserts only)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR adds a Confidence Score: 4/5Safe to merge for most workloads; prior-thread P1 concerns around MongoDB race conditions and SQL timestamp collision reconciliation should be addressed first New findings are P2 only, but prior threads raised unresolved P1 concerns: (1) MongoDB src/Database/Adapter/Mongo.php (missing Important Files Changed
Reviews (12): Last reviewed commit: "Revert "Fix Mongo adapter ignore mode: p..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
tests/e2e/Adapter/Scopes/DocumentTests.php (1)
7815-7818: Prefer__FUNCTION__for collection naming consistency.Using
__FUNCTION__here (as in adjacent tests) improves isolation and avoids accidental name reuse across reruns.♻️ Suggested tweak
- $col = 'createDocsIgnoreIntraBatch'; + $col = __FUNCTION__;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/Adapter/Scopes/DocumentTests.php` around lines 7815 - 7818, The collection name is hard-coded as 'createDocsIgnoreIntraBatch' which can collide across test runs; change the $col assignment to use the current function name (replace the literal with __FUNCTION__) so the calls to $database->createCollection($col) and $database->createAttribute($col, 'name', Database::VAR_STRING, 128, true) use a unique, test-local collection name consistent with adjacent tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Database/Adapter/Mongo.php`:
- Around line 1501-1524: The current dedupe prefetch uses only _uid and a single
tenant filter, causing cross-tenant collisions for shared tables; update the
logic that builds existingUids (and the prefetch loop around
$this->client->find/$this->client->getMore) to be tenant-aware by using a
composite key (e.g., "$tenant:$uid") when populating $existingUids, include the
tenant in the $filters returned by $this->getTenantFilters($collection->getId())
per chunk, and prefetch in per-tenant chunks using array_chunk($uidValues,
max(1, $this->maxQueryValues)) (use the chunk size when calling getMore instead
of count($uidValues)); then when filtering $records check the composite key
($record['_tenant'] . ':' . ($record['_uid'] ?? '')) against $existingUids so
duplicate detection respects tenant boundaries.
- Around line 1504-1516: The prefetch block calls $this->client->find() and
$this->client->getMore() without exception mapping; wrap each driver call in a
try/catch that catches \Throwable and rethrows the normalized exception via
processException($e) (same pattern used elsewhere in this class), i.e. surround
the $this->client->find(...) call and the $this->client->getMore(...) call with
try { ... } catch (\Throwable $e) { throw $this->processException($e); } so any
raw driver errors from the $response/$more handling are mapped consistently.
In `@src/Database/Database.php`:
- Around line 7210-7255: The upsertDocumentsWithIncrease() flow builds
$existingDocs keyed as tenant:id when $upsertTenantPerDocument is true, but
later duplicate-input detection still uses only $document->getId(), causing
false DuplicateException across tenants; update the duplicate-input check and
any intermediate maps (e.g., $seenIds/$inputKeys) to use the same composite
lookup key (use the existing $lookupKey logic: $tenant . ':' .
$document->getId() when $upsertTenantPerDocument is true) so the tenant-aware
key is reused consistently across the whole upsert path.
---
Nitpick comments:
In `@tests/e2e/Adapter/Scopes/DocumentTests.php`:
- Around line 7815-7818: The collection name is hard-coded as
'createDocsIgnoreIntraBatch' which can collide across test runs; change the $col
assignment to use the current function name (replace the literal with
__FUNCTION__) so the calls to $database->createCollection($col) and
$database->createAttribute($col, 'name', Database::VAR_STRING, 128, true) use a
unique, test-local collection name consistent with adjacent tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c9f1924e-3ed0-43a8-8859-6e6c299aadb5
📒 Files selected for processing (9)
src/Database/Adapter.phpsrc/Database/Adapter/Mongo.phpsrc/Database/Adapter/Pool.phpsrc/Database/Adapter/Postgres.phpsrc/Database/Adapter/SQL.phpsrc/Database/Adapter/SQLite.phpsrc/Database/Database.phpsrc/Database/Mirror.phptests/e2e/Adapter/Scopes/DocumentTests.php
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/Database/Database.php (2)
5682-5726:⚠️ Potential issue | 🟠 MajorDon’t let the outer prefetch snapshot suppress writes.
The adapters already do the authoritative duplicate filtering inside the transaction/session, but this code still drops
$chunkentries from a pre-transactionfind(). If a matching row is deleted after that snapshot but before the insert transaction starts,ignore: truewill incorrectly skip a document that should now insert, and$modified/$onNextwill be low.Also applies to: 5805-5815
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Database/Database.php` around lines 5682 - 5726, The current prefetch logic builds $preExistingIds (when $ignore is true, in the tenantPerDocument and non-tenant branches using find()/authorization->skip/etc.) and later uses that snapshot to skip writes, which can wrongly suppress inserts if a matching row was deleted after the prefetch; remove or stop using this prefetch-based suppression. Instead, eliminate reliance on $preExistingIds to skip document inserts/relationship writes (and the related checks that reference it around the relationship-resolution code and at the later block noted ~5805-5815), and either rely on the adapters’ authoritative duplicate filtering inside the transaction/session or re-check existence inside the transaction. Keep the prefetch only if it’s used for optimizations that do not affect whether an insert occurs, or remove the prefetch entirely if it’s unused for non-authoritative optimizations.
5788-5790:⚠️ Potential issue | 🟠 MajorUse the same tenant-aware key for deferred relationship writes.
In tenant-per-document mode, two inserted documents can legitimately share the same
$id. This map is keyed only by$id, so the later payload overwrites the earlier one, andunset()after the first insert prevents the second tenant’s relationship write entirely.♻️ Proposed fix
if (!empty($relationshipData)) { - $deferredRelationships[$document->getId()] = $relationshipData; + $relationshipKey = $tenantPerDocument + ? $document->getTenant() . ':' . $document->getId() + : $document->getId(); + $deferredRelationships[$relationshipKey] = $relationshipData; } @@ foreach ($batch as $insertedDoc) { - $docId = $insertedDoc->getId(); - if (\array_key_exists($docId, $deferredRelationships)) { + $relationshipKey = $tenantPerDocument + ? $insertedDoc->getTenant() . ':' . $insertedDoc->getId() + : $insertedDoc->getId(); + if (\array_key_exists($relationshipKey, $deferredRelationships)) { $relDoc = clone $insertedDoc; - foreach ($deferredRelationships[$docId] as $key => $value) { + foreach ($deferredRelationships[$relationshipKey] as $key => $value) { $relDoc->setAttribute($key, $value); } $this->silent(fn () => $this->createDocumentRelationships($collection, $relDoc)); - unset($deferredRelationships[$docId]); + unset($deferredRelationships[$relationshipKey]); } }Also applies to: 5823-5832
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Database/Database.php` around lines 5788 - 5790, The deferred relationship map ($deferredRelationships) is keyed only by $document->getId(), which causes collisions in tenant-per-document mode; change the key to be tenant-aware (e.g., combine tenant id and document id or use the document's tenant identifier method) wherever entries are set and removed so each tenant/document pair is unique—update the code that assigns $deferredRelationships[$document->getId()] = $relationshipData; and the matching unset() logic (and the same pattern around the related block at lines referenced 5823-5832) to use the composite tenant-aware key instead of just getId().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Database/Adapter/SQL.php`:
- Around line 2482-2527: The pre-filter SELECT that checks for existing _uid
rows (the block building $sql, preparing via $this->getPDO()->prepare, binding
values and calling $stmt->execute()/fetchAll()) can throw a raw PDOException
because it runs outside the method's main try/catch; wrap the entire
prepare/execute/fetch block in the adapter's processException(...) wrapper so
exceptions are mapped consistently (i.e., call $this->processException(...)
around the closure that prepares, binds, executes and fetches, returning $rows),
keeping the logic inside createDocuments() and referencing the existing $sql,
$binds, $stmt and $rows variables.
- Around line 2665-2713: The reconciliation currently keys timestamps only by
_uid and uses a single :_vfy_tenant bind when $this->sharedTables is true, which
corrupts mixed-tenant batches; update the verification to be tenant-aware by
binding the tenant per document and keying by a composite tenant+uid.
Concretely, in the block that builds $verifyPlaceholders/$verifyBinds and
$verifySql (the code using $verifyPlaceholders, $verifyBinds, $verifySql and
$verifyStmt), include the tenant value for each document (use the document's
tenant when tenant-per-document applies), change the WHERE to match pairs (e.g.
use row-value tuples or explicit (_uid = :uX AND _tenant = :tX) clauses), and
then build $actualTimestamps keyed by "$tenant|$uid" (and compare against
$expectedTimestamps keyed the same way) so the subsequent loop that populates
$insertedDocs/$insertedUids only matches rows with the correct tenant+uid.
- Around line 2650-2652: Replace the use of "INSERT IGNORE" (returned by
getInsertKeyword) with a plain "INSERT" and remove any IGNORE-specific behavior
in getInsertSuffix so that constraint violations are not globally suppressed;
instead let the DB raise errors and update the insert/execute path that catches
SQL exceptions to inspect the database error/constraint name (e.g., index or key
name such as _uid) and map errors to DuplicateException only when the violated
constraint corresponds to the _uid duplicate, otherwise map to UniqueException
(or other specific exceptions) for PRIMARY/UNIQUE/NOT NULL/truncation
violations—use the existing functions getInsertKeyword and getInsertSuffix to
locate changes and adjust the exception-mapping in the code that performs the
insert and error handling.
---
Duplicate comments:
In `@src/Database/Database.php`:
- Around line 5682-5726: The current prefetch logic builds $preExistingIds (when
$ignore is true, in the tenantPerDocument and non-tenant branches using
find()/authorization->skip/etc.) and later uses that snapshot to skip writes,
which can wrongly suppress inserts if a matching row was deleted after the
prefetch; remove or stop using this prefetch-based suppression. Instead,
eliminate reliance on $preExistingIds to skip document inserts/relationship
writes (and the related checks that reference it around the
relationship-resolution code and at the later block noted ~5805-5815), and
either rely on the adapters’ authoritative duplicate filtering inside the
transaction/session or re-check existence inside the transaction. Keep the
prefetch only if it’s used for optimizations that do not affect whether an
insert occurs, or remove the prefetch entirely if it’s unused for
non-authoritative optimizations.
- Around line 5788-5790: The deferred relationship map ($deferredRelationships)
is keyed only by $document->getId(), which causes collisions in
tenant-per-document mode; change the key to be tenant-aware (e.g., combine
tenant id and document id or use the document's tenant identifier method)
wherever entries are set and removed so each tenant/document pair is
unique—update the code that assigns $deferredRelationships[$document->getId()] =
$relationshipData; and the matching unset() logic (and the same pattern around
the related block at lines referenced 5823-5832) to use the composite
tenant-aware key instead of just getId().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d2ab610b-bb64-4dcb-979c-f3880d9e99df
📒 Files selected for processing (3)
src/Database/Adapter/Mongo.phpsrc/Database/Adapter/SQL.phpsrc/Database/Database.php
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Database/Adapter/Mongo.php
There was a problem hiding this comment.
♻️ Duplicate comments (3)
src/Database/Adapter/SQL.php (3)
2482-2527:⚠️ Potential issue | 🟠 MajorWrap the ignore-mode pre-filter in
processException()handling.This query runs before the method’s main
try/catch, so SQL or connection failures here still escape as rawPDOExceptionwhile the rest ofcreateDocuments()is adapter-mapped.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Database/Adapter/SQL.php` around lines 2482 - 2527, The pre-filter block in createDocuments() that prepares and executes the SELECT (using getSQLTable(), getPDO(), getPDOType(), preparing $sql and executing $stmt) should be wrapped inside a call to processException() so any PDOException is adapter-mapped instead of escaping; move the existing logic that builds $placeholders/$binds, $tenantFilter, $sql, $stmt->bindValue(), $stmt->execute(), $rows = $stmt->fetchAll() and $stmt->closeCursor() into a closure passed to processException(), and return/assign the result from that closure (or rethrow via processException) so behavior matches the rest of createDocuments().
2782-2785:⚠️ Potential issue | 🟠 MajorDon’t use
INSERT IGNOREas the base implementation forignore=true.For adapters that inherit this default,
ignore=truestops meaning “skip duplicate document IDs” and starts suppressing a broader class of insert failures on both the document and permission insert paths. The base hook should stay plainINSERT INTO, and adapter-specific duplicate handling should be expressed with targeted conflict clauses or constraint-aware exception mapping.Does MariaDB/MySQL `INSERT IGNORE` suppress constraint violations beyond duplicate-key conflicts, such as PRIMARY KEY, other UNIQUE, NOT NULL, truncation, or conversion errors?Based on learnings, in
src/Database/Adapter/MariaDB.phponly duplicate_uidviolations should throwDuplicateException; other unique orPRIMARYcollisions should still throwUniqueException.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Database/Adapter/SQL.php` around lines 2782 - 2785, The base method getInsertKeyword in class SQL should not return 'INSERT IGNORE INTO' for ignore=true; change it to always return the plain 'INSERT INTO' and move any ignore semantics into adapter-specific code (e.g., Adapter/MariaDB.php) by using targeted conflict clauses or by mapping constraint/SQLSTATE errors to the correct exceptions; specifically, in MariaDB adapter ensure only duplicate violations on the document _uid map to DuplicateException while other UNIQUE/PRIMARY/constraint failures map to UniqueException (and do not rely on INSERT IGNORE in SQL::getInsertKeyword).
2664-2728:⚠️ Potential issue | 🔴 CriticalKeep
expectedTimestampstenant-aware too.
$actualTimestampsis keyed bytenant:uidnow, but$expectedTimestampsis still keyed only by_uid. IntenantPerDocumentmode, two documents with the same_uidin different tenants can overwrite each other here, causing an inserted document to be dropped from$documentsand miss its permission insert.💡 Proposed fix
- $expectedTimestamps = []; - foreach ($documents as $doc) { - $expectedTimestamps[$doc->getId()] = $doc->getCreatedAt(); - } + $expectedTimestamps = []; + foreach ($documents as $doc) { + $key = ($this->sharedTables && $this->tenantPerDocument) + ? $doc->getTenant() . ':' . $doc->getId() + : $doc->getId(); + $expectedTimestamps[$key] = $doc->getCreatedAt(); + } @@ - if (isset($actualTimestamps[$key]) && $actualTimestamps[$key] === $expectedTimestamps[$doc->getId()]) { + if (isset($actualTimestamps[$key]) && $actualTimestamps[$key] === $expectedTimestamps[$key]) { $insertedDocs[] = $doc; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Database/Adapter/SQL.php` around lines 2664 - 2728, The expectedTimestamps map is not tenant-aware while actualTimestamps is, so when sharedTables && tenantPerDocument you must key expectedTimestamps the same way as actualTimestamps (tenant + ':' + uid) during the initial foreach over $documents and also use that key when comparing later (instead of $expectedTimestamps[$doc->getId()]); update the loop that builds $expectedTimestamps and the comparison that checks equality to construct/look up keys as ($doc->getTenant() . ':' . $doc->getId()) when $this->sharedTables && $this->tenantPerDocument so entries don't collide across tenants.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/Database/Adapter/SQL.php`:
- Around line 2482-2527: The pre-filter block in createDocuments() that prepares
and executes the SELECT (using getSQLTable(), getPDO(), getPDOType(), preparing
$sql and executing $stmt) should be wrapped inside a call to processException()
so any PDOException is adapter-mapped instead of escaping; move the existing
logic that builds $placeholders/$binds, $tenantFilter, $sql, $stmt->bindValue(),
$stmt->execute(), $rows = $stmt->fetchAll() and $stmt->closeCursor() into a
closure passed to processException(), and return/assign the result from that
closure (or rethrow via processException) so behavior matches the rest of
createDocuments().
- Around line 2782-2785: The base method getInsertKeyword in class SQL should
not return 'INSERT IGNORE INTO' for ignore=true; change it to always return the
plain 'INSERT INTO' and move any ignore semantics into adapter-specific code
(e.g., Adapter/MariaDB.php) by using targeted conflict clauses or by mapping
constraint/SQLSTATE errors to the correct exceptions; specifically, in MariaDB
adapter ensure only duplicate violations on the document _uid map to
DuplicateException while other UNIQUE/PRIMARY/constraint failures map to
UniqueException (and do not rely on INSERT IGNORE in SQL::getInsertKeyword).
- Around line 2664-2728: The expectedTimestamps map is not tenant-aware while
actualTimestamps is, so when sharedTables && tenantPerDocument you must key
expectedTimestamps the same way as actualTimestamps (tenant + ':' + uid) during
the initial foreach over $documents and also use that key when comparing later
(instead of $expectedTimestamps[$doc->getId()]); update the loop that builds
$expectedTimestamps and the comparison that checks equality to construct/look up
keys as ($doc->getTenant() . ':' . $doc->getId()) when $this->sharedTables &&
$this->tenantPerDocument so entries don't collide across tenants.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 892638bd-ac20-414b-9059-7f7e20295028
📒 Files selected for processing (2)
src/Database/Adapter/SQL.phpsrc/Database/Database.php
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Database/Database.php
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Database/Adapter/SQL.php`:
- Around line 2676-2682: The verification query is binding composite keys from
$expectedTimestamps into $verifyBinds/placeholders and then comparing them
against the _uid column, but when tenantPerDocument is enabled _uid was set to
the raw document ID ($attributes['_uid'] = $document->getId()), so the WHERE
_uid IN (...) must use the document IDs not the composite keys. Fix by
extracting the document ID portion from each composite key before creating $ph
and adding to $verifyBinds (or if keys are already plain IDs, leave unchanged);
update the loop that builds $verifyPlaceholders/$verifyBinds (the foreach over
array_values(array_unique(array_keys($expectedTimestamps)))) to derive $uid =
<documentIdFromComposite($uid)> and bind that instead so the verification query
matches rows by actual _uid values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 43af96a2-d479-4354-86c9-7d68abad7b56
📒 Files selected for processing (1)
src/Database/Adapter/SQL.php
Adapters: - MariaDB/MySQL: INSERT IGNORE INTO - PostgreSQL: ON CONFLICT DO NOTHING - SQLite: INSERT OR IGNORE INTO - MongoDB: session-scoped pre-filter before insertMany Database.php: - Intra-batch dedup by ID (tenant-aware, first occurrence wins) - Pre-fetch existing IDs to skip known duplicates - Deferred relationship creation for ignore mode (no orphans) - Race-condition reconciliation via _createdAt timestamp verification upsertDocuments: - Batch-fetch existing docs with find() instead of per-row getDocument() - Tenant-aware composite keys for seenIds duplicate check All paths are tenant-per-document aware with null-safe array_filter.
- testCreateDocumentsIgnoreDuplicates: mixed batch with onNext assertions - testCreateDocumentsIgnoreIntraBatchDuplicates: first-wins, no ACL drift - testCreateDocumentsIgnoreAllDuplicates: zero inserts, empty onNext
premtsd-code
force-pushed
the
csv-import-upsert-v2
branch
from
f2b08dc to
bee42d4
Compare
| @@ -2583,13 +2660,111 @@ public function createDocuments(Document $collection, array $documents): array | |||
|
|
|||
| $this->execute($stmt); | |||
There was a problem hiding this comment.
INSERT IGNORE INTO suppresses all errors, not only duplicates
MySQL/MariaDB's INSERT IGNORE INTO converts every constraint violation — including NOT NULL violations, FK violations, and value truncation — into a silent warning. If a document is dropped for a reason other than a duplicate-key conflict, the adapter returns it in $documents as if it were inserted (no rowCount discrepancy), onNext fires, and $modified is incremented for a row that was never written.
ON CONFLICT DO NOTHING (Postgres) and INSERT OR IGNORE INTO (SQLite) share this characteristic for constraint violations. Consider logging PDOStatement::errorInfo() after the INSERT when $ignore is true so non-duplicate failures are at least observable.
There was a problem hiding this comment.
StructureValidator will handle the required NOT NULL columns
…x _uid mapping - Pass ignoreDuplicates option to insertMany for race-condition safety - Extract _uid from raw array before replaceChars (excluded from transformation) - Map inserted records back to original Document objects by _uid
…t and fix _uid mapping" This reverts commit 93a9136.
1 participant
Summary
bool $ignore = falseparameter tocreateDocumentsacross all adapters for silent duplicate handlingupsertDocuments: batch-fetch existing docs withfindinstead of per-rowgetDocumentcalls (~2x speedup)Adapter Implementation
INSERT IGNORE INTO+ pre-filter existing UIDs in transactionON CONFLICT DO NOTHING+ pre-filter existing UIDs in transactionINSERT OR IGNORE INTO+ pre-filter existing UIDs in transactioninsertManywith sessionTest plan
ignore: true— only new docs insertedignore: true— no rows inserted, count = 0onNextcallback assertions — fires only for actually inserted docsupsertDocuments~2x faster with batch-fetch optimizationSummary by CodeRabbit
New Features
Tests