skipDuplicates: drop deferred-relationships, inline pre-filter, tighten comments

premtsd-code · premtsd-code · commit 89e4cf8b0c8e · 2026-04-13T07:35:14.000+01:00
The deferred-relationships mechanism was useful when the adapter could
report which docs were actually inserted (via reconciliation/
upsertWithCounts). With those gone, $batch always equals $chunk, so the
defer-then-match dance is dead weight. The orchestrator already pre-
filters known duplicates upfront, so any survivor is expected to insert
and relationships can be created inline like the non-skipDuplicates path.

- Database.php: pre-filter $documents once, before encode/validate,
  instead of filtering each chunk inline. Drop $deferredRelationships
  scaffolding and the per-chunk match loop. Collapse if/elseif into
  one inline createDocumentRelationships call.
- Mongo.php: tighten withTransaction skipDuplicates comment (the
  "relationship writes are deferred" rationale is no longer accurate).
- Mirror.php: tighten the captureOnNext comment.
- composer.lock: revert unrelated symfony-polyfill-php85 / mongo 1.0.0→
  1.0.2 bumps that crept in from an earlier composer update.

Race window unchanged: a doc that passes pre-filter but loses to a
concurrent insert still produces an orphan relationship and an
over-counted $modified — same as before this commit.
diff --git a/composer.lock b/composer.lock
diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php
@@ -122,10 +122,7 @@ public function withTransaction(callable $callback): mixed
             return $callback();
         }
 
-        // skipDuplicates uses upsert + $setOnInsert, whose filter query would hit
-        // WriteConflict (E112) under transaction snapshot isolation. The upsert is
-        // atomic on its own and relationship writes are deferred by the Database
-        // layer, so running outside a transaction is both safe and necessary.
+        // upsert + $setOnInsert hits WriteConflict (E112) under txn snapshot isolation.
         if ($this->skipDuplicates) {
             return $callback();
         }
@@ -1500,11 +1497,7 @@ public function createDocuments(Document $collection, array $documents): array
             $records[] = $record;
         }
 
-        // skipDuplicates: use upsert + $setOnInsert so an already-present row (whether
-        // from a prior batch or a concurrent writer) becomes a silent no-op. MongoDB has
-        // no INSERT IGNORE, and plain insertMany aborts the transaction on any duplicate.
-        // Adapter::withTransaction opts out of a real transaction in this mode to avoid
-        // snapshot-isolation-induced WriteConflict errors from the upsert's filter query.
+        // insertMany aborts the txn on any duplicate; upsert + $setOnInsert no-ops instead.
         if ($this->skipDuplicates) {
             if (empty($records)) {
                 return [];
@@ -1517,8 +1510,7 @@ public function createDocuments(Document $collection, array $documents): array
                     $filter['_tenant'] = $record['_tenant'] ?? $this->getTenant();
                 }
 
-                // Filter fields cannot reappear in $setOnInsert — MongoDB rejects
-                // the update with a path-conflict error otherwise.
+                // Filter fields can't reappear in $setOnInsert (mongo path-conflict error).
                 $setOnInsert = $record;
                 unset($setOnInsert['_uid'], $setOnInsert['_tenant']);
 
diff --git a/src/Database/Database.php b/src/Database/Database.php
@@ -5748,7 +5748,8 @@ public function createDocuments(
         $time = DateTime::now();
         $modified = 0;
 
-        // Deduplicate intra-batch documents by ID (tenant-aware). First occurrence wins.
+        // skipDuplicates: dedupe intra-batch (first wins) then drop already-existing rows.
+        // Adapter INSERT IGNORE / ON CONFLICT / upsert is the race-safety net.
         if ($this->skipDuplicates) {
             $seenIds = [];
             $deduplicated = [];
@@ -5763,21 +5764,14 @@ public function createDocuments(
                 $deduplicated[] = $document;
             }
             $documents = $deduplicated;
-        }
-
-        // Pre-fetch existing IDs so chunks only carry docs that don't already exist.
-        // The adapter still applies INSERT IGNORE / ON CONFLICT DO NOTHING / upsert as a
-        // race-safety net, but the pre-filter handles the common case so the adapter can
-        // simply return its input as the inserted set without per-row reconciliation.
-        $preExistingIds = $this->skipDuplicates
-            ? $this->fetchExistingByIds($collection, $documents, idsOnly: true)
-            : [];
 
-        /** @var array<string, array<string, mixed>> $deferredRelationships */
-        $deferredRelationships = [];
-        $relationships = [];
-        if ($this->skipDuplicates && $this->resolveRelationships) {
-            $relationships = \array_filter($collection->getAttribute('attributes', []), fn ($attr) => $attr['type'] === self::VAR_RELATIONSHIP);
+            $preExistingIds = $this->fetchExistingByIds($collection, $documents, idsOnly: true);
+            if (!empty($preExistingIds)) {
+                $documents = \array_values(\array_filter(
+                    $documents,
+                    fn (Document $doc) => !isset($preExistingIds[$this->tenantKey($doc)])
+                ));
+            }
         }
 
         foreach ($documents as $document) {
@@ -5819,60 +5813,20 @@ public function createDocuments(
                 }
             }
 
-            if ($this->resolveRelationships && !empty($relationships)) {
-                // Defer relationship writes until after INSERT so we don't orphan records for duplicates.
-                $relationshipData = [];
-                foreach ($relationships as $rel) {
-                    $key = $rel['key'];
-                    $value = $document->getAttribute($key);
-                    if ($value !== null) {
-                        $relationshipData[$key] = $value;
-                    }
-                    $document->removeAttribute($key);
-                }
-                if (!empty($relationshipData)) {
-                    $deferredRelationships[$this->tenantKey($document)] = $relationshipData;
-                }
-            } elseif ($this->resolveRelationships) {
+            if ($this->resolveRelationships) {
                 $document = $this->silent(fn () => $this->createDocumentRelationships($collection, $document));
             }
 
             $document = $this->adapter->castingBefore($collection, $document);
         }
 
         foreach (\array_chunk($documents, $batchSize) as $chunk) {
-            if ($this->skipDuplicates && !empty($preExistingIds)) {
-                $chunk = \array_values(\array_filter(
-                    $chunk,
-                    fn (Document $doc) => !isset($preExistingIds[$this->tenantKey($doc)])
-                ));
-                if (empty($chunk)) {
-                    continue;
-                }
-            }
-
-            // skipDuplicates wraps withTransaction so the adapter's flag is set before
-            // Mongo::withTransaction decides whether to start a real transaction.
+            // Set adapter flag before withTransaction so Mongo can opt out of a real txn.
             $batch = $this->adapter->skipDuplicates(
                 fn () => $this->withTransaction(fn () => $this->adapter->createDocuments($collection, $chunk)),
                 $this->skipDuplicates
             );
 
-            // Create deferred relationships only for docs that were actually inserted
-            if ($this->skipDuplicates && $this->resolveRelationships && \count($deferredRelationships) > 0) {
-                foreach ($batch as $insertedDoc) {
-                    $deferKey = $this->tenantKey($insertedDoc);
-                    if (\array_key_exists($deferKey, $deferredRelationships)) {
-                        $relDoc = clone $insertedDoc;
-                        foreach ($deferredRelationships[$deferKey] as $key => $value) {
-                            $relDoc->setAttribute($key, $value);
-                        }
-                        $this->silent(fn () => $this->createDocumentRelationships($collection, $relDoc));
-                        unset($deferredRelationships[$deferKey]);
-                    }
-                }
-            }
-
             $batch = $this->adapter->getSequences($collection->getId(), $batch);
 
             if (!$this->inBatchRelationshipPopulation && $this->resolveRelationships) {
diff --git a/src/Database/Mirror.php b/src/Database/Mirror.php
@@ -601,10 +601,8 @@ public function createDocuments(
         ?callable $onNext = null,
         ?callable $onError = null,
     ): int {
-        // Capture the docs the source actually persisted (rather than the input)
-        // so that, in skipDuplicates mode, we don't end up forwarding skipped
-        // duplicates to the destination — that would let the destination diverge
-        // from the source whenever the source no-ops a write.
+        // Forward only docs the source actually persisted, so skipDuplicates no-ops
+        // on the source don't inject would-be values into the destination.
         /** @var array<Document> $insertedFromSource */
         $insertedFromSource = [];
         $captureOnNext = function (Document $doc) use (&$insertedFromSource, $onNext): void {
