Address Jake's review on PR #852

#1 Drop $enable flag on skipDuplicates() scope guard
The $enable param made every non-skipDuplicates createDocuments call pay
for a closure allocation + extra function call. Branch at the call site
instead so the cost only applies when the flag is actually set.

  - Adapter::skipDuplicates(callable, bool) → skipDuplicates(callable)
  - Database::skipDuplicates(callable, bool) → skipDuplicates(callable)
  - Database::createDocuments, Mirror::createDocuments, Pool::delegate,
    Pool::withTransaction now branch inline.

#2 Drop fetchExistingByIds helper, inline find()
The helper's per-tenant grouping defended a hypothetical multi-tenant
batching scenario that no caller exercises (relationships are intra-
tenant, callers always batch per tenant). Existing patterns in the same
file (refetchDocuments, relationship loading) just call find() directly.
Match that idiom and drop ~70 lines.

#4 Mirror: only capture inserted docs in skipDuplicates mode
The captureOnNext accumulator paid the cost (closure + per-doc array
push) on every createDocuments call, including the common non-skip path.
Branch at the entry of Mirror::createDocuments so the capture only
happens when skipDuplicates is set; the non-skip path passes through
to source/destination unchanged.

#5 Move getInsertKeyword/Suffix/PermissionsSuffix to getters cluster
Were sitting next to createDocuments(); moved to the getSupport*
cluster around line 1030 where other adapter-capability shims live.

Not addressed:
- #2 partial: the existing patterns (refetchDocuments etc.) don't handle
  tenant-per-document multi-tenant batches either, so this is consistent.
- #3 (drop the pre-filter): rejected. createDocumentRelationships runs
  in the encoding loop BEFORE the adapter's INSERT IGNORE no-ops the
  parent, so dropping the pre-filter would deterministically duplicate
  child rows on every CSV re-import of a collection with relationships
  (not a race window — every call). The relationships test verifies
  this. Reverting would require reintroducing the deferred-relationships
  scaffolding we just removed, and the adapter still couldn't tell us
  which parents were actually inserted (SQL INSERT IGNORE has no per-row
  reporting). Pre-filter stays.

Author: premtsd-code

src/Database/Adapter.php

@@ -39,20 +39,13 @@ abstract class Adapter
      * Run a callback with skipDuplicates enabled.
      * Duplicate key errors during createDocuments() will be silently skipped
      * instead of thrown. Nestable — saves and restores previous state.
-     * Pass $enable = false to run the callback without toggling the flag
-     * (useful for conditional forwarding from wrappers like Pool/Mirror).
      *
      * @template T
      * @param callable(): T $callback
-     * @param bool $enable
      * @return T
      */
-    public function skipDuplicates(callable $callback, bool $enable = true): mixed
+    public function skipDuplicates(callable $callback): mixed
     {
-        if (!$enable) {
-            return $callback();
-        }
-
         $previous = $this->skipDuplicates;
         $this->skipDuplicates = true;
 

src/Database/Adapter/Pool.php

@@ -43,10 +43,12 @@ public function __construct(UtopiaPool $pool)
     public function delegate(string $method, array $args): mixed
     {
         if ($this->pinnedAdapter !== null) {
-            return $this->pinnedAdapter->skipDuplicates(
-                fn () => $this->pinnedAdapter->{$method}(...$args),
-                $this->skipDuplicates
-            );
+            if ($this->skipDuplicates) {
+                return $this->pinnedAdapter->skipDuplicates(
+                    fn () => $this->pinnedAdapter->{$method}(...$args)
+                );
+            }
+            return $this->pinnedAdapter->{$method}(...$args);
         }
 
         return $this->pool->use(function (Adapter $adapter) use ($method, $args) {
@@ -69,10 +71,12 @@ public function delegate(string $method, array $args): mixed
                 $adapter->setMetadata($key, $value);
             }
 
-            return $adapter->skipDuplicates(
-                fn () => $adapter->{$method}(...$args),
-                $this->skipDuplicates
-            );
+            if ($this->skipDuplicates) {
+                return $adapter->skipDuplicates(
+                    fn () => $adapter->{$method}(...$args)
+                );
+            }
+            return $adapter->{$method}(...$args);
         });
     }
 
@@ -152,10 +156,12 @@ public function withTransaction(callable $callback): mixed
 
             $this->pinnedAdapter = $adapter;
             try {
-                return $adapter->skipDuplicates(
-                    fn () => $adapter->withTransaction($callback),
-                    $this->skipDuplicates
-                );
+                if ($this->skipDuplicates) {
+                    return $adapter->skipDuplicates(
+                        fn () => $adapter->withTransaction($callback)
+                    );
+                }
+                return $adapter->withTransaction($callback);
             } finally {
                 $this->pinnedAdapter = null;
             }

src/Database/Adapter/SQL.php

@@ -1029,6 +1029,33 @@ public function getSupportForHostname(): bool
         return true;
     }
 
+    /**
+     * Returns the INSERT keyword, optionally with IGNORE for duplicate handling.
+     * Override in adapter subclasses for DB-specific syntax.
+     */
+    protected function getInsertKeyword(): string
+    {
+        return $this->skipDuplicates ? '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(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(): string
+    {
+        return '';
+    }
+
     /**
      * Get current attribute count from collection document
      *
@@ -2611,33 +2638,6 @@ 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(): string
-    {
-        return $this->skipDuplicates ? '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(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(): string
-    {
-        return '';
-    }
-
     /**
      * @param Document $collection
      * @param string $attribute

src/Database/Database.php

@@ -844,12 +844,8 @@ public function skipRelationshipsExistCheck(callable $callback): mixed
         }
     }
 
-    public function skipDuplicates(callable $callback, bool $enable = true): mixed
+    public function skipDuplicates(callable $callback): mixed
     {
-        if (!$enable) {
-            return $callback();
-        }
-
         $previous = $this->skipDuplicates;
         $this->skipDuplicates = true;
 
@@ -871,72 +867,6 @@ private function tenantKey(Document $document): string
             : $document->getId();
     }
 
-    /**
-     * Batch-fetch documents by ID from a collection, keyed by tenantKey().
-     * Chunks by maxQueryValues and groups queries by tenant in tenant-per-document mode.
-     *
-     * @param Document $collection
-     * @param array<Document> $documents Source documents (only IDs are read)
-     * @param bool $idsOnly When true, uses Query::select(['$id']) for a lighter fetch
-     * @return array<string, Document>
-     */
-    private function fetchExistingByIds(Document $collection, array $documents, bool $idsOnly = false): array
-    {
-        $collectionId = $collection->getId();
-        $tenantPerDocument = $this->adapter->getSharedTables() && $this->adapter->getTenantPerDocument();
-        $result = [];
-
-        $buildQueries = function (array $chunk) use ($idsOnly) {
-            $queries = [Query::equal('$id', $chunk)];
-            if ($idsOnly) {
-                $queries[] = Query::select(['$id']);
-            }
-            $queries[] = Query::limit(\count($chunk));
-            return $queries;
-        };
-
-        if ($tenantPerDocument) {
-            $idsByTenant = [];
-            foreach ($documents as $doc) {
-                $id = $doc->getId();
-                if ($id !== '') {
-                    $idsByTenant[$doc->getTenant()][] = $id;
-                }
-            }
-            foreach ($idsByTenant as $tenant => $tenantIds) {
-                $tenantIds = \array_values(\array_unique($tenantIds));
-                foreach (\array_chunk($tenantIds, \max(1, $this->maxQueryValues)) as $idChunk) {
-                    $fetched = $this->authorization->skip(
-                        fn () => $this->withTenant(
-                            $tenant,
-                            fn () => $this->silent(fn () => $this->find($collectionId, $buildQueries($idChunk)))
-                        )
-                    );
-                    foreach ($fetched as $doc) {
-                        $result[$tenant . ':' . $doc->getId()] = $doc;
-                    }
-                }
-            }
-
-            return $result;
-        }
-
-        $ids = \array_values(\array_unique(\array_filter(
-            \array_map(fn (Document $doc) => $doc->getId(), $documents),
-            fn ($v) => $v !== null && $v !== ''
-        )));
-        foreach (\array_chunk($ids, \max(1, $this->maxQueryValues)) as $idChunk) {
-            $fetched = $this->authorization->skip(
-                fn () => $this->silent(fn () => $this->find($collectionId, $buildQueries($idChunk)))
-            );
-            foreach ($fetched as $doc) {
-                $result[$doc->getId()] = $doc;
-            }
-        }
-
-        return $result;
-    }
-
     /**
      * Trigger callback for events
      *
@@ -5765,12 +5695,31 @@ public function createDocuments(
             }
             $documents = $deduplicated;
 
-            $preExistingIds = $this->fetchExistingByIds($collection, $documents, idsOnly: true);
-            if (!empty($preExistingIds)) {
-                $documents = \array_values(\array_filter(
-                    $documents,
-                    fn (Document $doc) => !isset($preExistingIds[$this->tenantKey($doc)])
+            $docIds = \array_values(\array_unique(\array_filter(
+                \array_map(fn (Document $doc) => $doc->getId(), $documents),
+                fn ($id) => $id !== ''
+            )));
+
+            if (!empty($docIds)) {
+                $existing = $this->authorization->skip(fn () => $this->silent(
+                    fn () => $this->find($collection->getId(), [
+                        Query::equal('$id', $docIds),
+                        Query::select(['$id']),
+                        Query::limit(\count($docIds)),
+                    ])
                 ));
+
+                $preExistingIds = [];
+                foreach ($existing as $doc) {
+                    $preExistingIds[$this->tenantKey($doc)] = true;
+                }
+
+                if (!empty($preExistingIds)) {
+                    $documents = \array_values(\array_filter(
+                        $documents,
+                        fn (Document $doc) => !isset($preExistingIds[$this->tenantKey($doc)])
+                    ));
+                }
             }
         }
 
@@ -5821,11 +5770,11 @@ 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->adapter->skipDuplicates(
-                fn () => $this->withTransaction(fn () => $this->adapter->createDocuments($collection, $chunk)),
-                $this->skipDuplicates
-            );
+            $batch = $this->skipDuplicates
+                ? $this->adapter->skipDuplicates($insert)
+                : $insert();
 
             $batch = $this->adapter->getSequences($collection->getId(), $batch);
 
@@ -7241,7 +7190,23 @@ public function upsertDocumentsWithIncrease(
         $seenIds = [];
 
         // Batch-fetch existing documents in one query instead of N individual getDocument() calls
-        $existingDocs = $this->fetchExistingByIds($collection, $documents);
+        $docIds = \array_values(\array_unique(\array_filter(
+            \array_map(fn (Document $doc) => $doc->getId(), $documents),
+            fn ($id) => $id !== ''
+        )));
+
+        $existingDocs = [];
+        if (!empty($docIds)) {
+            $existing = $this->authorization->skip(fn () => $this->silent(
+                fn () => $this->find($collection->getId(), [
+                    Query::equal('$id', $docIds),
+                    Query::limit(\count($docIds)),
+                ])
+            ));
+            foreach ($existing as $doc) {
+                $existingDocs[$this->tenantKey($doc)] = $doc;
+            }
+        }
 
         foreach ($documents as $key => $document) {
             $old = $existingDocs[$this->tenantKey($document)] ?? new Document();