From 5803460b6bf5d13394a0b694cf3a4799dd674cb7 Mon Sep 17 00:00:00 2001 From: fogelito Date: Tue, 7 Jul 2026 10:07:20 +0300 Subject: [PATCH 1/4] fetch by $sequence --- src/Database/Database.php | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 74e75d883d..9bfca3ef3c 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -807,6 +807,7 @@ public function skipRelationships(callable $callback): mixed * @param Document $collection * @param array $documents * @return array + * @throws DatabaseException */ protected function refetchDocuments(Document $collection, array $documents): array { @@ -814,21 +815,27 @@ protected function refetchDocuments(Document $collection, array $documents): arr return $documents; } - $docIds = array_map(fn ($doc) => $doc->getId(), $documents); + $sequences = array_map(function ($doc) { + $sequence = $doc->getSequence(); + if ($sequence === null) { + throw new DatabaseException('Cannot refetch document without a $sequence: ' . $doc->getId()); + } + return $sequence; + }, $documents); // Fetch fresh copies with computed operator values $refetched = $this->getAuthorization()->skip(fn () => $this->silent( - fn () => $this->find($collection->getId(), [Query::equal('$id', $docIds)]) + fn () => $this->find($collection->getId(), [Query::equal('$sequence', $sequences)]) )); $refetchedMap = []; foreach ($refetched as $doc) { - $refetchedMap[$doc->getId()] = $doc; + $refetchedMap[$doc->getSequence()] = $doc; } $result = []; foreach ($documents as $doc) { - $result[] = $refetchedMap[$doc->getId()] ?? $doc; + $result[] = $refetchedMap[$doc->getSequence()] ?? $doc; } return $result; From c67b7572e22757f2bb78965d6e251d8a54016f74 Mon Sep 17 00:00:00 2001 From: fogelito Date: Tue, 7 Jul 2026 14:51:17 +0300 Subject: [PATCH 2/4] Refetch --- src/Database/Database.php | 44 +++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 9bfca3ef3c..1919d955ea 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -806,10 +806,11 @@ public function skipRelationships(callable $callback): mixed * * @param Document $collection * @param array $documents + * @param array $selections Select queries from the caller, preserved so the refetch honors the original projection * @return array * @throws DatabaseException */ - protected function refetchDocuments(Document $collection, array $documents): array + protected function refetchDocuments(Document $collection, array $documents, array $selections = []): array { if (empty($documents)) { return $documents; @@ -823,9 +824,12 @@ protected function refetchDocuments(Document $collection, array $documents): arr return $sequence; }, $documents); - // Fetch fresh copies with computed operator values + // Fetch fresh copies with computed operator values, preserving the caller's projection $refetched = $this->getAuthorization()->skip(fn () => $this->silent( - fn () => $this->find($collection->getId(), [Query::equal('$sequence', $sequences)]) + fn () => $this->find( + $collection->getId(), + array_merge([Query::equal('$sequence', $sequences)], $selections) + ) )); $refetchedMap = []; @@ -834,8 +838,8 @@ protected function refetchDocuments(Document $collection, array $documents): arr } $result = []; - foreach ($documents as $doc) { - $result[] = $refetchedMap[$doc->getSequence()] ?? $doc; + foreach ($documents as $index => $doc) { + $result[$index] = $refetchedMap[$sequences[$index]] ?? $doc; } return $result; @@ -6383,20 +6387,6 @@ public function updateDocument(string $collection, string $id, Document $documen $this->purgeCachedDocument($collection->getId(), $document->getId()); } - // If operators were used, refetch document to get computed values - $hasOperators = false; - foreach ($document->getArrayCopy() as $value) { - if (Operator::isOperator($value)) { - $hasOperators = true; - break; - } - } - - if ($hasOperators) { - $refetched = $this->refetchDocuments($collection, [$document]); - $document = $refetched[0]; - } - return $document; }); @@ -6407,6 +6397,20 @@ public function updateDocument(string $collection, string $id, Document $documen // Purge again after commit so readers cannot re-cache the pre-commit version $this->purgeCachedDocumentInternal($collection->getId(), $id); + // If operators were used, refetch (outside the transaction) to get committed computed values + $hasOperators = false; + foreach ($document->getArrayCopy() as $value) { + if (Operator::isOperator($value)) { + $hasOperators = true; + break; + } + } + + if ($hasOperators) { + $refetched = $this->refetchDocuments($collection, [$document]); + $document = $refetched[0]; + } + if (!$this->inBatchRelationshipPopulation && $this->resolveRelationships) { $documents = $this->silent(fn () => $this->populateDocumentsRelationships([$document], $collection, $this->relationshipFetchDepth)); $document = $documents[0]; @@ -6630,7 +6634,7 @@ public function updateDocuments( } if ($hasOperators) { - $batch = $this->refetchDocuments($collection, $batch); + $batch = $this->refetchDocuments($collection, $batch, $grouped['selections']); } foreach ($batch as $index => $doc) { From 89a5621235eaf34729105182d49ef7d05a6ff77e Mon Sep 17 00:00:00 2001 From: fogelito Date: Tue, 7 Jul 2026 15:17:20 +0300 Subject: [PATCH 3/4] Run --- src/Database/Database.php | 6 ++- tests/e2e/Adapter/Scopes/OperatorTests.php | 62 ++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 1919d955ea..e3b456a682 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -6641,7 +6641,11 @@ public function updateDocuments( $doc = $this->adapter->castingAfter($collection, $doc); $doc->removeAttribute('$skipPermissionsUpdate'); $this->purgeCachedDocument($collection->getId(), $doc->getId()); - $doc = $this->decode($collection, $doc); + // Operator refetch already returns decoded documents honoring the caller's select; + // decoding again (without selections) would re-materialize non-selected attributes. + if (!$hasOperators) { + $doc = $this->decode($collection, $doc); + } try { $onNext && $onNext($doc, $old[$index]); } catch (Throwable $th) { diff --git a/tests/e2e/Adapter/Scopes/OperatorTests.php b/tests/e2e/Adapter/Scopes/OperatorTests.php index 89c64bee5b..36abf9ff6e 100644 --- a/tests/e2e/Adapter/Scopes/OperatorTests.php +++ b/tests/e2e/Adapter/Scopes/OperatorTests.php @@ -431,6 +431,68 @@ public function testUpdateDocumentsOperatorsWithQueries(): void $database->deleteCollection($collectionId); } + public function testUpdateDocumentsOperatorsWithSelect(): void + { + /** @var Database $database */ + $database = static::getDatabase(); + + if (!$database->getAdapter()->getSupportForOperators()) { + $this->expectNotToPerformAssertions(); + return; + } + + $collectionId = 'test_operators_with_select'; + $database->createCollection($collectionId); + + $database->createAttribute($collectionId, 'category', Database::VAR_STRING, 50, true); + $database->createAttribute($collectionId, 'count', Database::VAR_INTEGER, 0, false, 0); + $database->createAttribute($collectionId, 'score', Database::VAR_FLOAT, 0, false, 0.0); + + for ($i = 1; $i <= 3; $i++) { + $database->createDocument($collectionId, new Document([ + '$id' => "select_doc_{$i}", + '$permissions' => [Permission::read(Role::any()), Permission::update(Role::any())], + 'category' => 'A', + 'count' => $i * 10, + 'score' => $i * 1.5, + ])); + } + + // Update with an operator while selecting only a subset of attributes. + // The operator path refetches to compute values; it must still honor the caller's select. + $updated = []; + $count = $database->updateDocuments( + $collectionId, + new Document([ + 'count' => Operator::increment(100), + ]), + [Query::select(['count']), Query::orderAsc('$id')], + onNext: function (Document $doc) use (&$updated) { + $updated[] = $doc; + } + ); + + $this->assertEquals(3, $count); + $this->assertCount(3, $updated); + + // A plain find with the same select defines the expected projection. + $found = $database->find($collectionId, [Query::select(['count']), Query::orderAsc('$id')]); + + foreach ($updated as $index => $doc) { + // Computed operator value is present and correct. + $this->assertEquals(($index + 1) * 10 + 100, $doc->getAttribute('count')); + + // The operator path must return the same projection as a normal select find, + // i.e. it must not leak the non-selected attributes (regression check). + $this->assertEquals( + \array_keys($found[$index]->getArrayCopy()), + \array_keys($doc->getArrayCopy()) + ); + } + + $database->deleteCollection($collectionId); + } + public function testOperatorErrorHandling(): void { /** @var Database $database */ From a09b39bc3e452cfb872d9217933e101cd9e547f9 Mon Sep 17 00:00:00 2001 From: fogelito Date: Tue, 7 Jul 2026 15:59:12 +0300 Subject: [PATCH 4/4] Add tests and chunk logic --- src/Database/Database.php | 32 +++++++----- tests/e2e/Adapter/Scopes/OperatorTests.php | 57 ++++++++++++++++++++++ 2 files changed, 77 insertions(+), 12 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index e3b456a682..6d4b3c86de 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -824,17 +824,24 @@ protected function refetchDocuments(Document $collection, array $documents, arra return $sequence; }, $documents); - // Fetch fresh copies with computed operator values, preserving the caller's projection - $refetched = $this->getAuthorization()->skip(fn () => $this->silent( - fn () => $this->find( - $collection->getId(), - array_merge([Query::equal('$sequence', $sequences)], $selections) - ) - )); - + // Fetch fresh copies with computed operator values, preserving the caller's projection. + // Chunk by maxQueryValues (the batch can be up to INSERT_BATCH_SIZE) and bound each find() + // to the chunk size, otherwise find()'s default limit would silently drop rows past it. $refetchedMap = []; - foreach ($refetched as $doc) { - $refetchedMap[$doc->getSequence()] = $doc; + foreach (\array_chunk($sequences, \max(1, $this->maxQueryValues)) as $chunk) { + $refetched = $this->getAuthorization()->skip(fn () => $this->silent( + fn () => $this->find( + $collection->getId(), + array_merge([ + Query::equal('$sequence', $chunk), + Query::limit(\count($chunk)), + ], $selections) + ) + )); + + foreach ($refetched as $doc) { + $refetchedMap[$doc->getSequence()] = $doc; + } } $result = []; @@ -6641,8 +6648,9 @@ public function updateDocuments( $doc = $this->adapter->castingAfter($collection, $doc); $doc->removeAttribute('$skipPermissionsUpdate'); $this->purgeCachedDocument($collection->getId(), $doc->getId()); - // Operator refetch already returns decoded documents honoring the caller's select; - // decoding again (without selections) would re-materialize non-selected attributes. + // The operator refetch goes through find(), which already returns fully decoded + // documents. Decoding again would double-apply the decode filters (and, because + // this call passes no selections, re-materialize non-selected attributes). if (!$hasOperators) { $doc = $this->decode($collection, $doc); } diff --git a/tests/e2e/Adapter/Scopes/OperatorTests.php b/tests/e2e/Adapter/Scopes/OperatorTests.php index 36abf9ff6e..77fb5b1622 100644 --- a/tests/e2e/Adapter/Scopes/OperatorTests.php +++ b/tests/e2e/Adapter/Scopes/OperatorTests.php @@ -493,6 +493,63 @@ public function testUpdateDocumentsOperatorsWithSelect(): void $database->deleteCollection($collectionId); } + public function testUpdateDocumentsOperatorsBatchLargerThanDefaultLimit(): void + { + /** @var Database $database */ + $database = static::getDatabase(); + + if (!$database->getAdapter()->getSupportForOperators()) { + $this->expectNotToPerformAssertions(); + return; + } + + $collectionId = 'test_operators_large_batch'; + $database->createCollection($collectionId); + + $database->createAttribute($collectionId, 'count', Database::VAR_INTEGER, 0, false, 0); + + // More documents than find()'s default limit (25) so the refetch must page/limit correctly. + $total = 60; + for ($i = 1; $i <= $total; $i++) { + $database->createDocument($collectionId, new Document([ + '$id' => "batch_doc_{$i}", + '$permissions' => [Permission::read(Role::any()), Permission::update(Role::any())], + 'count' => $i, + ])); + } + + // Update every document with an operator. The refetch must return computed values for the + // whole batch, not just the first 25 rows (which would otherwise fall back to stale values). + $updated = []; + $count = $database->updateDocuments( + $collectionId, + new Document([ + 'count' => Operator::increment(1000), + ]), + [], + batchSize: $total, + onNext: function (Document $doc) use (&$updated) { + $updated[$doc->getId()] = $doc; + } + ); + + $this->assertEquals($total, $count); + $this->assertCount($total, $updated); + + // Every document must reflect the computed operator value. + for ($i = 1; $i <= $total; $i++) { + $doc = $updated["batch_doc_{$i}"] ?? null; + $this->assertNotNull($doc, "Missing callback document batch_doc_{$i}"); + $this->assertEquals($i + 1000, $doc->getAttribute('count'), "Stale value for batch_doc_{$i}"); + + // And it must be persisted, not just returned. + $fresh = $database->getDocument($collectionId, "batch_doc_{$i}"); + $this->assertEquals($i + 1000, $fresh->getAttribute('count')); + } + + $database->deleteCollection($collectionId); + } + public function testOperatorErrorHandling(): void { /** @var Database $database */