From 6920e36c7c17155bb3519f71f8e834824d25a691 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=C5=82as=20Piotr?= Date: Tue, 29 Sep 2026 11:25:31 +0200 Subject: [PATCH 1/2] feat(collection): full-collection scans via iterDocs() / ZVecDocIterator (#226) There was no way to read every document in a collection. fetch() needs primary keys, queryByFilter() needs both a filter and a topk, and neither can walk a whole collection -- so export, backup and migration were not possible without one. foreach ($collection->iterDocs() as $pk => $doc) { ... } Backed by upstream's Collection::create_iterator() (zvec v0.7.0), exposed as Python does with iter_docs(). Constant memory regardless of collection size, over an isolated snapshot taken at call time. Ownership is the subtle part, and it is why this is split the way it is. The native iterator holds a std::shared_ptr to the collection, copied out of the collections registry at creation. Two consequences: - release_slot in upstream captures a raw Collection*, so freeing the handle underneath a live iterator would make it release a slot in freed memory. The shared_ptr makes that impossible: zvec_collection_free() only drops *our* reference, and the C++ collection stays alive until the last iterator goes. - Member order in DocIteratorHolder matters. Members are destroyed in reverse declaration order, so `collection` is declared first and `iterator` last -- the iterator is released first, giving its slot back, and the collection second. The other order would deadlock: upstream's ~CollectionImpl waits on a condition variable for every iterator to close, which in a single-threaded PHP process means waiting for something that can never happen. tests/test_doc_iterator_shutdown.phpt exists for exactly this -- the failure mode is a hang, not a crash, so nothing else would catch it. On the PHP side, ZVecDocIterator holds a reference to its ZVec object, so the collection cannot be collected out from under a live iterator. The upstream guards are surfaced rather than worked around: while an iterator is open, close(), destroy(), DDL and optimize() return FAILED_PRECONDITION and change nothing. That only works because of #222 -- before it, a rejected destroy() freed the native collection and the next call was a use-after-free. Writes, flush and queries are unaffected, and the test asserts that. Implemented as \Iterator rather than IteratorAggregate returning a Generator: a second getIterator() call on the same object would silently yield nothing from the exhausted handle, where rewind() failing loudly is better. Nullable normalization follows fetch() since #192, but only for the fields actually requested -- otherwise hasField('weight') would become true for outputFields: ['id'], which contradicts upstream's own expectations. Tests: 208/208, 0 skipped, 0 failed, 2 expected fail. - test_doc_iterator.phpt: empty collection, 50-doc scan, vectors on/off, outputFields, primary-key-only, nullable normalization, deleted documents skipped, snapshot isolation from later writes, auto-close on exhaustion, early close, forward-only rewind, and the three validation paths. - test_doc_iterator_lifecycle.phpt: the FAILED_PRECONDITION guards, the collection still usable after a rejected destroy(), writes during iteration, dropping the collection variable mid-iteration, destructor with an open iterator, and the lock being released afterwards. - test_doc_iterator_shutdown.phpt: clean process exit with both a collection and an open iterator alive. - test_memory_doc_iterator.phpt: no handle or char*[N] leak across early close, full scan and output-fields scans. - test_ffi_load.phpt gained the three symbols, test_null_handle_collection the three null cases. --- AGENTS.md | 2 + CHANGELOG.md | 11 ++ README.md | 33 +++++ ffi/zvec_ffi.cc | 138 ++++++++++++++++++ ffi/zvec_ffi.h | 27 ++++ ffi/zvec_ffi_php.h | 5 + src/ZVec.php | 62 ++++++++ src/ZVecDocIterator.php | 160 ++++++++++++++++++++ tests/test_doc_iterator.phpt | 193 +++++++++++++++++++++++++ tests/test_doc_iterator_lifecycle.phpt | 138 ++++++++++++++++++ tests/test_doc_iterator_shutdown.phpt | 47 ++++++ tests/test_ffi_load.phpt | 5 +- tests/test_memory_doc_iterator.phpt | 93 ++++++++++++ tests/test_null_handle_collection.phpt | 17 +++ 14 files changed, 930 insertions(+), 1 deletion(-) create mode 100644 src/ZVecDocIterator.php create mode 100644 tests/test_doc_iterator.phpt create mode 100644 tests/test_doc_iterator_lifecycle.phpt create mode 100644 tests/test_doc_iterator_shutdown.phpt create mode 100644 tests/test_memory_doc_iterator.phpt diff --git a/AGENTS.md b/AGENTS.md index b912458..0c9cf97 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,6 +16,7 @@ zvec-php/ ├── src/ZVecGroupByVectorQuery.php # Group-by vector query builder ├── src/ZVecSchema.php # Schema definition (field types, metrics, vectors) ├── src/ZVecDoc.php # Document handle (getters/setters, serialization) +├── src/ZVecDocIterator.php # Full-scan document iterator (iterDocs) ├── src/ZVecReRanker.php # Base re-ranker class ├── src/ZVecRerankedDoc.php # Reranked document class ├── src/ZVecRrfReRanker.php # RRF re-ranker @@ -448,6 +449,7 @@ and runner output. - `__destruct()` calls `close()` automatically, and never throws — on failure it falls back to dropping the handle so the C++ object is not leaked - After `destroy()`, any method call causes **segfault** (handle invalidated) - A **failed** `close()` or `destroy()` must leave the object open and usable. Do not free the handle before the status is known: the C++ object is only safe to drop once upstream says the operation succeeded, or once it says the collection is closed regardless +- While a `ZVecDocIterator` is open, `close()` / `destroy()` / DDL / `optimize()` throw `FAILED_PRECONDITION`. Close iterators first — they auto-close when exhausted ### Memory Leak Regression Tests diff --git a/CHANGELOG.md b/CHANGELOG.md index 451e098..9a4a21e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Full-collection scans: `ZVec::iterDocs()` / `ZVecDocIterator`** (#226) + - `iterDocs(?array $outputFields = null, bool $includeVector = true): ZVecDocIterator` returns a forward-only iterator over every document, backed by an isolated snapshot taken at call time. Memory use is constant regardless of collection size. Mirrors the Python SDK's `Collection.iter_docs()` and upstream's `Collection::create_iterator()` (zvec v0.7.0). + - This closes the gap for export, backup and migration: `fetch()` needs primary keys, `queryByFilter()` needs both a filter and a `topk`, and neither can walk a whole collection. + - The iterator closes itself when exhausted, so a completed `foreach` needs no cleanup. `close()` is idempotent for breaking out early, and `isClosed()` reports the state. `rewind()` after the first advance throws, by design — a second `getIterator()` on the same object would otherwise silently yield nothing. + - **While an iterator is open, `close()`, `destroy()`, schema DDL and `optimize()` throw `ZVecException` with code 5 (`FAILED_PRECONDITION`)**; the collection stays open and usable, and writes, `flush()` and queries are unaffected. This depends on the `close()` / `destroy()` behaviour from #222 — a rejected `destroy()` has to leave the handle valid. + - The adapter gives each iterator its own `shared_ptr` to the collection and releases it *after* the iterator. Upstream requires the collection to outlive its iterators, and `~CollectionImpl` blocks until every iterator is closed, so the wrong release order would deadlock a single-threaded PHP process at shutdown. + - `ZVecDocIterator` holds a PHP reference to its `ZVec` object, so the collection cannot be destroyed out from under a live iterator; `tests/test_doc_iterator_lifecycle.phpt` covers dropping the collection variable mid-iteration and the destructor path. + - `outputFields` accepts scalar fields only (`null` = all, `[]` = primary key only); unknown or vector names are rejected upstream. Unset nullable fields come back as `null`, matching `fetch()` since #192 — but only for the fields actually requested, so `outputFields: ['id']` does not make `hasField('weight')` true. + - FFI: `zvec_doc_iterator_t`, `zvec_collection_create_iterator()`, `zvec_doc_iterator_next()`, `zvec_doc_iterator_free()`. + - Tests: `tests/test_doc_iterator.phpt` (empty collection, full scan, vectors on/off, output fields, primary-key-only, nullable normalization, deleted documents skipped, snapshot isolation, auto-close, early close, forward-only, validation, closed collection), `tests/test_doc_iterator_lifecycle.phpt` (the `FAILED_PRECONDITION` guards, writes during iteration, collection lifetime, destructor safety), `tests/test_doc_iterator_shutdown.phpt` (clean process exit with an open iterator — an infinite hang rather than a crash if the release order is wrong), and `tests/test_memory_doc_iterator.phpt` (no handle or C string array leak). + - **Vamana `two_pass_build` and query prefetch** (#220) - `ZVecIndexParams::forVamana()` takes a trailing `bool $twoPassBuild = false`, which runs a second full-graph Vamana construction pass for better graph quality at the cost of build time. Upstream and Python both default to `false`. Last and optional, so existing positional calls are unaffected. - `ZVecVectorQuery::setVamanaPrefetch(int $prefetchOffset, int $prefetchLines)` tunes software prefetch during the Vamana graph search, the counterpart of the existing `setHnswPrefetch()`. An offset of `0` disables prefetch; `0` lines means "derive from vector size". Both reject negative values. diff --git a/README.md b/README.md index 43c409e..563c3ca 100644 --- a/README.md +++ b/README.md @@ -259,6 +259,7 @@ $collection->updateBatch(ZVecDoc ...$docs): array // Returns per-doc status $collection->delete(string ...$pks): void $collection->deleteByFilter(string $filter): void $collection->fetch(string ...$pks): ZVecDoc[] // also fetch(array $pks, ?array $outputFields = null, includeVector: bool = true) +$collection->iterDocs(?array $outputFields = null, bool $includeVector = true): ZVecDocIterator // full scan over a snapshot; foreach ($it as $pk => $doc) // named `outputFields:` supported; named-arg `pks:` and mixing scalar PKs with an array are rejected with a hint // Search @@ -329,6 +330,36 @@ $schema::METRIC_MIPSL2 = 4 // Modified Inner Product with L2 // that emit E_USER_DEPRECATED warnings. ``` +### ZVecDocIterator + +Returned by `iterDocs()`. Scans every document with constant memory, over a +snapshot taken when the iterator was created — so documents written afterwards +are not visible to it. This is the full-scan path: no primary keys, no filter, +no `topk`. Useful for export, backup and migration, which `fetch()` (needs PKs) +and `queryByFilter()` (needs a filter and a topk) cannot serve. + +```php +foreach ($collection->iterDocs() as $pk => $doc) { + fwrite($fh, json_encode(['pk' => $pk, 'id' => $doc->getInt64('id')]) . "\n"); +} +``` + +- **Forward-only.** `rewind()` after the first advance throws. Call + `iterDocs()` again to scan afresh. +- **Auto-closes on exhaustion.** A completed `foreach` needs no cleanup. Call + `close()` explicitly when breaking out early; it is idempotent, and + `isClosed()` reports the state. +- **Guards the collection.** While one is open, `close()`, `destroy()`, + `addColumn*()` / `alterColumn()` / `dropColumn()` and `optimize()` throw + `ZVecException` with code `5` (`FAILED_PRECONDITION`). Writes, `flush()` and + queries keep working. Close iterators first. +- **Seals a segment.** On a writable collection each call seals the current + writing segment, which may create a small one. A read-only collection is + scanned without writing. +- `outputFields` accepts scalar fields only — `null` means all of them, `[]` + means primary key only. Vector or unknown names are rejected by upstream. + Nullable fields that are unset come back as `null`, matching `fetch()`. + ### ZVecDoc ```php @@ -868,6 +899,7 @@ zvec-php/ │ ├── ZVecGroupByVectorQuery.php # Group-by vector query builder │ ├── ZVecSchema.php # Schema definition │ ├── ZVecDoc.php # Document handle +│ ├── ZVecDocIterator.php # Full-scan document iterator │ ├── ZVecReRanker.php # Base re-ranker interface │ ├── ZVecRerankedDoc.php # Reranked document class │ ├── ZVecRrfReRanker.php # RRF re-ranker @@ -933,6 +965,7 @@ See `tasks/done/` for detailed planning documents. - [x] Group-by vector query builder (`ZVecGroupByVectorQuery`) - [x] Version API (`getVersion()`, `checkVersion()`) - [x] DiskANN I/O backend introspection (`getIoBackendType()`, `getIoBackendDescription()`) +- [x] Full-collection scans (`iterDocs()` / `ZVecDocIterator`) - [x] `allowedBasePath` security restriction in `init()` - [x] Verbose error details with file/line info - [x] Collection lifecycle options via `getOptions()` diff --git a/ffi/zvec_ffi.cc b/ffi/zvec_ffi.cc index 83cf8a1..190a87c 100644 --- a/ffi/zvec_ffi.cc +++ b/ffi/zvec_ffi.cc @@ -1,6 +1,7 @@ #include "zvec_ffi.h" #include +#include #include #include #include @@ -2820,6 +2821,143 @@ zvec_status_t zvec_collection_fetch(zvec_collection_t coll, const char** pks, in return ok_status(); } +// --- Doc iterator --- + +namespace { +struct DocIteratorHolder { + // Declaration order is destruction order reversed, so `iterator` is + // released first -- it hands its slot back to the collection -- and + // `collection` last. Upstream requires the collection to outlive its + // iterators (create_iterator's release_slot captures the raw pointer), and + // ~CollectionImpl blocks until every iterator is closed, which would + // deadlock a single-threaded PHP process. Keeping our own shared_ptr copy + // also means zvec_collection_free() on the PHP side cannot pull the + // collection out from under a live iterator. + std::shared_ptr collection; + std::vector nullable_fields; // requested nullable fields only + DocIterator::Ptr iterator; +}; +} // namespace + +zvec_status_t zvec_collection_create_iterator(zvec_collection_t coll, + int has_output_fields, + const char** output_fields, + int output_field_count, + int include_vector, + zvec_doc_iterator_t* out) { + if (!out) { + zvec_status_t st = {1, "null out pointer"}; + SET_FFI_ERROR(st); + return st; + } + *out = nullptr; + if (!coll) { + zvec_status_t st = {1, "null handle"}; + SET_FFI_ERROR(st); + return st; + } + if (output_field_count < 0 || (output_field_count > 0 && !output_fields)) { + zvec_status_t st = {1, "invalid output fields"}; + SET_FFI_ERROR(st); + return st; + } + + // Copy the collection out of the registry so the iterator owns a reference. + std::shared_ptr owner; + { + std::unique_lock lock(g_collections_mutex); + auto& reg = collections_registry(); + auto it = reg.find(static_cast(coll)); + if (it == reg.end()) { + zvec_status_t st = {1, "collection handle is not open"}; + SET_FFI_ERROR(st); + return st; + } + owner = it->second; + } + + IteratorOptions opts; + opts.include_vector_ = include_vector != 0; + if (has_output_fields) { + std::vector fields; + fields.reserve(output_field_count); + for (int i = 0; i < output_field_count; i++) { + if (!output_fields[i]) { + zvec_status_t st = {1, "null output field"}; + SET_FFI_ERROR(st); + return st; + } + fields.emplace_back(output_fields[i]); + } + // Assigned even when empty: upstream reads an empty vector as "PK only". + opts.output_fields_ = std::move(fields); + } + + // Since #192 fetch() reports an absent nullable field as present-and-null. + // Iterate consistently, but only for the fields actually requested -- with + // outputFields: ['id'], hasField('weight') must stay false. + std::vector nullable_fields; + auto schema_res = owner->schema(); + if (schema_res.has_value()) { + for (const auto& field : schema_res.value().fields()) { + if (field && field->nullable()) { + if (!has_output_fields) { + nullable_fields.push_back(field->name()); + } else if (opts.output_fields_ && + std::find(opts.output_fields_->begin(), opts.output_fields_->end(), field->name()) != + opts.output_fields_->end()) { + nullable_fields.push_back(field->name()); + } + } + } + } + + auto res = owner->create_iterator(opts); + if (!res.has_value()) { + return MAKE_STATUS(res.error()); + } + auto* h = new DocIteratorHolder{std::move(owner), std::move(nullable_fields), std::move(res).value()}; + *out = static_cast(h); + return ok_status(); +} + +zvec_status_t zvec_doc_iterator_next(zvec_doc_iterator_t it, zvec_doc_t* out_doc) { + if (!out_doc) { + zvec_status_t st = {1, "null out pointer"}; + SET_FFI_ERROR(st); + return st; + } + *out_doc = nullptr; + if (!it) { + zvec_status_t st = {1, "null handle"}; + SET_FFI_ERROR(st); + return st; + } + auto* h = static_cast(it); + auto res = h->iterator->next(); + if (!res.has_value()) { + return MAKE_STATUS(res.error()); + } + if (!res.value()) { + return ok_status(); // end of iteration + } + // Copy the doc: PHP frees documents with zvec_doc_free, which deletes a + // Doc*, so the shared_ptr cannot be handed out. + auto* doc = new Doc(*res.value()); + for (const auto& name : h->nullable_fields) { + if (!doc->has(name)) { + doc->set_null(name); + } + } + *out_doc = static_cast(doc); + return ok_status(); +} + +void zvec_doc_iterator_free(zvec_doc_iterator_t it) { + if (!it) return; + delete static_cast(it); +} + // --- Query --- zvec_status_t zvec_collection_query(zvec_collection_t coll, const char* field_name, diff --git a/ffi/zvec_ffi.h b/ffi/zvec_ffi.h index 4e03fe1..0d612bb 100644 --- a/ffi/zvec_ffi.h +++ b/ffi/zvec_ffi.h @@ -384,6 +384,33 @@ zvec_status_t zvec_collection_fetch(zvec_collection_t coll, const char** pks, in int include_vector, zvec_query_result_t* result); +// Document iterator (zvec v0.7.0 Collection::create_iterator) +// +// Full scan over an isolated snapshot taken at creation time. The iterator +// holds its own reference to the collection, so zvec_collection_free() may be +// called before zvec_doc_iterator_free(). While an iterator is open, +// close/destroy/DDL/optimize on the collection return code 5 +// (FAILED_PRECONDITION) and the collection stays open. +typedef void* zvec_doc_iterator_t; + +// has_output_fields = 0: return all scalar fields (output_fields ignored). +// has_output_fields = 1: return only the listed scalar fields; count 0 (and +// output_fields may be NULL) returns no scalar fields, only the PK. +// include_vector: 1 = include vector fields, 0 = skip them. +// On error *out is set to NULL. +zvec_status_t zvec_collection_create_iterator(zvec_collection_t coll, + int has_output_fields, + const char** output_fields, + int output_field_count, + int include_vector, + zvec_doc_iterator_t* out); +// On success *out_doc is a new document owned by the caller (free with +// zvec_doc_free), or NULL at end of iteration. On error *out_doc is NULL. +zvec_status_t zvec_doc_iterator_next(zvec_doc_iterator_t it, zvec_doc_t* out_doc); +// Closes the iterator and releases its snapshot and its collection +// reference. NULL is a no-op. +void zvec_doc_iterator_free(zvec_doc_iterator_t it); + // Query zvec_status_t zvec_collection_query(zvec_collection_t coll, const char* field_name, const float* query_vector, uint32_t dim, diff --git a/ffi/zvec_ffi_php.h b/ffi/zvec_ffi_php.h index bdd4adb..d7f886b 100644 --- a/ffi/zvec_ffi_php.h +++ b/ffi/zvec_ffi_php.h @@ -270,6 +270,11 @@ zvec_status_t zvec_collection_fetch(zvec_collection_t coll, const char** pks, in int include_vector, zvec_query_result_t* result); +typedef void* zvec_doc_iterator_t; +zvec_status_t zvec_collection_create_iterator(zvec_collection_t coll, int has_output_fields, const char** output_fields, int output_field_count, int include_vector, zvec_doc_iterator_t* out); +zvec_status_t zvec_doc_iterator_next(zvec_doc_iterator_t it, zvec_doc_t* out_doc); +void zvec_doc_iterator_free(zvec_doc_iterator_t it); + zvec_status_t zvec_collection_query(zvec_collection_t coll, const char* field_name, const float* query_vector, uint32_t dim, int topk, int include_vector, diff --git a/src/ZVec.php b/src/ZVec.php index 946927b..f845801 100644 --- a/src/ZVec.php +++ b/src/ZVec.php @@ -18,6 +18,7 @@ require_once __DIR__ . '/ZVecGroupByVectorQuery.php'; require_once __DIR__ . '/ZVecSchema.php'; require_once __DIR__ . '/ZVecDoc.php'; +require_once __DIR__ . '/ZVecDocIterator.php'; require_once __DIR__ . '/ZVecReRanker.php'; require_once __DIR__ . '/ZVecRerankedDoc.php'; require_once __DIR__ . '/ZVecRrfReRanker.php'; @@ -853,6 +854,66 @@ public function fetch(array|string|bool ...$args): array return self::parseQueryResult($result); } + /** + * Iterate over every document in the collection. + * + * Unlike fetch() this needs no primary keys, and unlike queryByFilter() no + * filter and no topk — it is the full-scan path, intended for export, + * backup and migration. Memory use is constant regardless of collection + * size. + * + * foreach ($collection->iterDocs() as $pk => $doc) { ... } + * + * The scan runs over an isolated snapshot taken now, so later writes are + * invisible to it. On a writable collection each call seals the current + * writing segment, which may create a small segment. + * + * While the returned iterator is open, close(), destroy(), schema DDL and + * optimize() throw ZVecException (FAILED_PRECONDITION); the iterator closes + * itself when exhausted, so a completed foreach needs no cleanup. + * + * @param string[]|null $outputFields null = all scalar fields, [] = primary + * key only. Vector fields are not allowed + * here and are rejected upstream. + * @param bool $includeVector Whether vector fields are returned + * + * @throws ZVecException On FFI error or when the collection is closed + */ + public function iterDocs(?array $outputFields = null, bool $includeVector = true): ZVecDocIterator + { + $this->checkClosed(); + if ($outputFields !== null) { + foreach ($outputFields as $field) { + if (!is_string($field) || $field === '') { + throw new ZVecException('outputFields must contain only non-empty strings'); + } + } + } + + $ffi = self::ffi(); + [$ofArr, $ofCount, $ofCStrings] = self::toCStringArray($ffi, $outputFields ?? []); + $out = $ffi->new('zvec_doc_iterator_t'); + try { + self::checkStatus($ffi->zvec_collection_create_iterator( + $this->handle, + $outputFields === null ? 0 : 1, + $ofArr, + $ofCount, + $includeVector ? 1 : 0, + FFI::addr($out), + )); + } finally { + self::freeCStringArray($ofCStrings); + // toCStringArray() allocates the char*[N] array unmanaged but frees + // only the strings, so release the array itself here. + if ($ofArr !== null) { + FFI::free($ofArr); + } + } + + return new ZVecDocIterator($this, $out); + } + /** * Index type: HNSW (Hierarchical Navigable Small World). * @@ -2793,6 +2854,7 @@ class_alias(ZVecIndexParams::class, 'ZVecIndexParams'); \class_alias(ZVecQueryInterface::class, 'ZVecQueryInterface'); class_alias(ZVecVectorQuery::class, 'ZVecVectorQuery'); class_alias(ZVecGroupByVectorQuery::class, 'ZVecGroupByVectorQuery'); +class_alias(ZVecDocIterator::class, 'ZVecDocIterator'); \class_alias(ZVecReRanker::class, 'ZVecReRanker'); class_alias(ZVecRerankedDoc::class, 'ZVecRerankedDoc'); class_alias(ZVecRrfReRanker::class, 'ZVecRrfReRanker'); diff --git a/src/ZVecDocIterator.php b/src/ZVecDocIterator.php new file mode 100644 index 0000000..bbc5a1a --- /dev/null +++ b/src/ZVecDocIterator.php @@ -0,0 +1,160 @@ +iterDocs() as $pk => $doc) { ... } + * + * While an iterator is open, `close()`, `destroy()`, schema DDL and + * `optimize()` on the collection throw ZVecException with code 5 + * (FAILED_PRECONDITION) and the collection stays open and usable. Writes, flush + * and queries are unaffected. The iterator closes itself once exhausted, so a + * completed foreach needs no cleanup; call {@see close()} explicitly when + * breaking out early. + * + * @implements \Iterator + */ +class ZVecDocIterator implements \Iterator +{ + private ?FFI\CData $handle; + private ZVec $collection; + private ?ZVecDoc $current = null; + private int $position = -1; + private bool $closed = false; + + /** + * @internal Create through {@see ZVec::iterDocs()}. + */ + public function __construct(ZVec $collection, FFI\CData $handle) + { + $this->collection = $collection; + $this->handle = $handle; + } + + public function __destruct() + { + try { + $this->close(); + } catch (\Throwable) { + // A destructor must not throw; the handle is released by the + // parent's teardown if this fails. + } + } + + private function __clone() + { + } + + /** + * Releases the snapshot and lets the collection close or run DDL again. + * + * Idempotent. Called automatically when the iterator is exhausted. + */ + public function close(): void + { + if ($this->closed) { + return; + } + ZVec::ffi()->zvec_doc_iterator_free($this->handle); + $this->handle = null; + $this->current = null; + $this->closed = true; + } + + /** Whether the native handle has already been released. */ + public function isClosed(): bool + { + return $this->closed; + } + + /** + * @throws ZVecException If the iterator has already been advanced + */ + public function rewind(): void + { + if ($this->position > 0) { + throw new ZVecException( + 'ZVecDocIterator is forward-only and cannot be rewound; call ZVec::iterDocs() again' + ); + } + if ($this->position < 0) { + $this->fetchNext(); + $this->position = 0; + } + } + + public function valid(): bool + { + if ($this->position < 0) { + $this->rewind(); + } + return $this->current !== null; + } + + /** @throws ZVecException If the iterator is not positioned on a document */ + public function current(): ZVecDoc + { + if ($this->position < 0) { + $this->rewind(); + } + if ($this->current === null) { + throw new ZVecException('No current document'); + } + return $this->current; + } + + /** @throws ZVecException If the iterator is not positioned on a document */ + public function key(): string + { + return $this->current()->getPk(); + } + + public function next(): void + { + if ($this->position < 0) { + $this->rewind(); + return; + } + $this->fetchNext(); + $this->position++; + } + + /** + * Advances to the next document, closing the iterator at end of iteration. + * + * @throws ZVecException On FFI error + */ + private function fetchNext(): void + { + if ($this->closed) { + $this->current = null; + return; + } + $ffi = ZVec::ffi(); + $out = $ffi->new('zvec_doc_t'); + try { + ZVec::checkStatus($ffi->zvec_doc_iterator_next($this->handle, FFI::addr($out))); + } catch (ZVecException $e) { + $this->close(); + throw $e; + } + if (FFI::isNull($out)) { + $this->close(); + $this->current = null; + return; + } + $this->current = new ZVecDoc($out, true); + } +} diff --git a/tests/test_doc_iterator.phpt b/tests/test_doc_iterator.phpt new file mode 100644 index 0000000..e29e88d --- /dev/null +++ b/tests/test_doc_iterator.phpt @@ -0,0 +1,193 @@ +--TEST-- +DocIterator: iterDocs() full scan, options, snapshot, deletes, validation +--SKIPIF-- + +--FILE-- +addInt64('id', nullable: false, withInvertIndex: true) + ->addString('name', nullable: false) + ->addFloat('weight', nullable: true) + ->addVectorFp32('dense', dimension: 4, metricType: ZVecSchema::METRIC_IP); + +/** @return ZVecDoc[] */ +function makeDocs(int $n, string $prefix = ''): array +{ + $docs = []; + for ($i = 0; $i < $n; $i++) { + $d = new ZVecDoc($prefix . $i); + $d->setInt64('id', $i) + ->setString('name', "name_$i") + ->setVectorFp32('dense', [(float)$i, 0.0, 0.0, 1.0]); + if ($i % 2 === 0) { + $d->setFloat('weight', (float)$i); + } + $docs[] = $d; + } + return $docs; +} + +try { + $c = ZVec::create($path, $schema); + echo 'empty: ' . count(iterator_to_array($c->iterDocs())) . "\n"; + + $c->insert(...makeDocs(50)); + $c->flush(); + + // Document order within a segment is not part of the contract, so sort. + $pks = []; + $fieldsOk = true; + foreach ($c->iterDocs() as $pk => $doc) { + $pks[] = $pk; + if ($pk !== $doc->getPk() || $doc->getInt64('id') !== (int)$pk || $doc->getString('name') !== "name_$pk") { + $fieldsOk = false; + } + } + sort($pks); + echo 'basic: count=' . count($pks) . ' unique=' . count(array_unique($pks)) + . ' fields_ok=' . ($fieldsOk ? 'yes' : 'no') . "\n"; + + $allVectors = true; + foreach ($c->iterDocs() as $doc) { + if (count($doc->getVectorFp32('dense')) !== 4) { + $allVectors = false; + } + } + echo 'vectors: ' . ($allVectors ? 'yes' : 'no') . "\n"; + + $noVectors = true; + foreach ($c->iterDocs(includeVector: false) as $doc) { + if ($doc->hasVector('dense') || $doc->getInt64('id') === null) { + $noVectors = false; + } + } + echo 'no vectors: ' . ($noVectors ? 'yes' : 'no') . "\n"; + + $outputOk = true; + foreach ($c->iterDocs(outputFields: ['id'], includeVector: false) as $doc) { + if (!$doc->hasField('id') || $doc->hasField('name') || $doc->hasField('weight')) { + $outputOk = false; + } + } + echo 'output fields: ' . ($outputOk ? 'yes' : 'no') . "\n"; + + // An empty list means "primary key only" upstream. + $pkOnly = true; + $pkOnlyCount = 0; + foreach ($c->iterDocs(outputFields: [], includeVector: false) as $doc) { + $pkOnlyCount++; + if ($doc->fieldNames() !== []) { + $pkOnly = false; + } + } + echo 'pk only: ' . $pkOnlyCount . ($pkOnly ? '' : ' FAILED') . "\n"; + + // Since #192 fetch() reports an absent nullable field as present-and-null; + // iteration must agree, but only for the fields actually requested. + $nullableOk = true; + foreach ($c->iterDocs() as $pk => $doc) { + if ($doc->getPk() === '1' && (!($doc->isFieldNull('weight')) || $doc->getFloat('weight') !== null)) { + $nullableOk = false; + } + if ($doc->getPk() === '2' && $doc->getFloat('weight') !== 2.0) { + $nullableOk = false; + } + } + foreach ($c->iterDocs(outputFields: ['id'], includeVector: false) as $doc) { + if ($doc->hasField('weight')) { + $nullableOk = false; + } + } + echo 'nullable: ' . ($nullableOk ? 'yes' : 'no') . "\n"; + + $c->delete(...array_map('strval', range(0, 48, 2))); + $c->flush(); + $afterDelete = []; + foreach ($c->iterDocs() as $pk => $doc) { + $afterDelete[] = (int)$pk; + } + echo 'after delete: ' . count($afterDelete) + . (count(array_filter($afterDelete, static fn(int $v): bool => $v % 2 === 0)) === 0 ? '' : ' FAILED') . "\n"; + + // The snapshot is taken when iterDocs() is called, so later writes are invisible. + $snapshot = $c->iterDocs(); + $c->insert(...makeDocs(3, 'late_')); + echo 'snapshot: ' . iterator_count($snapshot) . "\n"; + echo 'fresh: ' . count(iterator_to_array($c->iterDocs())) . "\n"; + + // Exhaustion releases the native handle, so a completed foreach needs no close(). + echo 'auto closed: ' . ($snapshot->isClosed() ? 'yes' : 'no') . "\n"; + + $early = $c->iterDocs(); + $early->rewind(); + $early->close(); + $early->close(); + echo 'early close: ' . (!$early->valid() && $early->isClosed() ? 'yes' : 'no') . "\n"; + + $forward = $c->iterDocs(); + $forward->rewind(); + $forward->next(); + try { + $forward->rewind(); + echo "rewind: NOT REJECTED\n"; + } catch (ZVecException $e) { + echo 'rewind: ' . $e->getMessage() . "\n"; + } + $forward->close(); + + try { + $c->iterDocs(outputFields: ['nope']); + echo "unknown field: NOT REJECTED\n"; + } catch (ZVecException $e) { + echo 'unknown field: ' . $e->getErrorCodeString() . "\n"; + } + + try { + $c->iterDocs(outputFields: ['dense']); + echo "vector field: NOT REJECTED\n"; + } catch (ZVecException $e) { + echo 'vector field: ' . $e->getErrorCodeString() . "\n"; + } + + try { + $c->iterDocs(outputFields: ['']); + echo "PHP validation: NOT REJECTED\n"; + } catch (ZVecException $e) { + echo 'PHP validation: ' . $e->getMessage() . "\n"; + } + + $c->close(); + try { + $c->iterDocs(); + echo "closed collection: NOT REJECTED\n"; + } catch (ZVecException $e) { + echo $e->getMessage() . "\n"; + } +} finally { + exec('rm -rf ' . escapeshellarg($path)); +} +?> +--EXPECT-- +empty: 0 +basic: count=50 unique=50 fields_ok=yes +vectors: yes +no vectors: yes +output fields: yes +pk only: 50 +nullable: yes +after delete: 25 +snapshot: 25 +fresh: 28 +auto closed: yes +early close: yes +rewind: ZVecDocIterator is forward-only and cannot be rewound; call ZVec::iterDocs() again +unknown field: INVALID_ARGUMENT +vector field: INVALID_ARGUMENT +PHP validation: outputFields must contain only non-empty strings +Collection is closed. Open with ZVec::open() to continue. diff --git a/tests/test_doc_iterator_lifecycle.phpt b/tests/test_doc_iterator_lifecycle.phpt new file mode 100644 index 0000000..2295214 --- /dev/null +++ b/tests/test_doc_iterator_lifecycle.phpt @@ -0,0 +1,138 @@ +--TEST-- +DocIterator lifecycle: guards on close/destroy/DDL, collection lifetime, destructor safety +--SKIPIF-- + +--FILE-- +addInt64('id', nullable: false) + ->addVectorFp32('v', dimension: 4, metricType: ZVecSchema::METRIC_IP); + +function makeLifeDocs(int $n, string $prefix = ''): array +{ + $docs = []; + for ($i = 0; $i < $n; $i++) { + $docs[] = (new ZVecDoc($prefix . $i)) + ->setInt64('id', $i) + ->setVectorFp32('v', [(float)$i, 0.0, 0.0, 0.0]); + } + return $docs; +} + +try { + $c = ZVec::create($path, $schema); + $c->insert(...makeLifeDocs(10)); + $c->flush(); + $c->close(); + + $c = ZVec::open($path); + $it = $c->iterDocs(); + $it->rewind(); + + // A rejected destroy() must leave the collection usable -- this is the + // guard #222 had to provide, since erasing the handle on failure would + // leave PHP with a dangling pointer. + try { + $c->destroy(); + echo "destroy: NOT REJECTED\n"; + } catch (ZVecException $e) { + echo 'destroy: ' . $e->getErrorCodeString() . "\n"; + } + echo 'usable after destroy: ' . count($c->fetch('0')) . "\n"; + + try { + $c->close(); + echo "close: NOT REJECTED\n"; + } catch (ZVecException $e) { + echo 'close: ' . $e->getErrorCodeString() . "\n"; + } + $c->flush(); + echo "usable after close: yes\n"; + + try { + $c->addColumnInt64('extra'); + echo "ddl: NOT REJECTED\n"; + } catch (ZVecException $e) { + echo 'ddl: ' . $e->getErrorCodeString() . "\n"; + } + + try { + $c->optimize(); + echo "optimize: NOT REJECTED\n"; + } catch (ZVecException $e) { + echo 'optimize: ' . $e->getErrorCodeString() . "\n"; + } + + // Writes and queries are explicitly not affected. + $c->insert(...makeLifeDocs(1, 'w_')); + echo 'write while open: ' . count($c->fetch('w_0')) . "\n"; + + // 10 rather than 9: iterator_count() calls rewind() first, which is a no-op + // at position 0, so the already-current document is counted too. The + // document written above is not visible -- the snapshot predates it. + echo 'drained: ' . iterator_count($it) . "\n"; + + // Exhaustion returned the slot, so this close is accepted. + $c->close(); + echo "close after drain: ok\n"; + + // Dropping the collection variable must not stop iteration: the iterator + // holds a PHP reference to it. 11 because w_0 was written after the earlier + // iterators existed and is visible to a fresh snapshot. + $holder = ZVec::open($path); + $it2 = $holder->iterDocs(); + $it2->rewind(); + unset($holder); + echo 'iter after unset: ' . iterator_count($it2) . "\n"; + unset($it2); + + // Destructor with an open iterator must not throw, and the iterator must + // keep working. This is where a member-order mistake in the adapter would + // deadlock: ~CollectionImpl blocks until every iterator is closed. + $doomed = ZVec::open($path); + $it3 = $doomed->iterDocs(); + $it3->rewind(); + $doomed->__destruct(); + echo "destruct with open iterator: ok\n"; + try { + $doomed->fetch('0'); + echo "closed: NOT REJECTED\n"; + } catch (ZVecException $e) { + echo 'closed: ' . $e->getMessage() . "\n"; + } + echo 'iter after destruct: ' . iterator_count($it3) . "\n"; + // 11 as above: this snapshot also includes w_0. + // Required: the finished iterator still holds the old ZVec object, and that + // object still holds the collection lock. + unset($it3, $doomed); + + $reopened = ZVec::open($path); + echo "reopen: ok\n"; + $reopened->destroy(); + echo "destroy after release: ok\n"; +} finally { + exec('rm -rf ' . escapeshellarg($path)); +} +?> +--EXPECT-- +destroy: FAILED_PRECONDITION +usable after destroy: 1 +close: FAILED_PRECONDITION +usable after close: yes +ddl: FAILED_PRECONDITION +optimize: FAILED_PRECONDITION +write while open: 1 +drained: 10 +close after drain: ok +iter after unset: 11 +destruct with open iterator: ok +closed: Collection is closed. Open with ZVec::open() to continue. +iter after destruct: 11 +reopen: ok +destroy after release: ok diff --git a/tests/test_doc_iterator_shutdown.phpt b/tests/test_doc_iterator_shutdown.phpt new file mode 100644 index 0000000..b29fe8d --- /dev/null +++ b/tests/test_doc_iterator_shutdown.phpt @@ -0,0 +1,47 @@ +--TEST-- +DocIterator: process shutdown with a collection and an open iterator exits cleanly +--SKIPIF-- + +--FILE-- +addInt64('id', nullable: false) + ->addVectorFp32('v', dimension: 4, metricType: ZVecSchema::METRIC_IP); + +// Recorded so --CLEAN-- can remove the directory in a separate process: the +// collection must still exist when PHP shuts down, so try/finally is not an +// option here. +$path = __DIR__ . '/../test_dbs/iter_shutdown_' . uniqid(); +file_put_contents(sys_get_temp_dir() . '/zvec_iter_shutdown.path', $path); + +$c = ZVec::create($path, $schema); +for ($i = 0; $i < 5; $i++) { + $c->insert( + (new ZVecDoc((string)$i))->setInt64('id', $i)->setVectorFp32('v', [(float)$i, 0.0, 0.0, 0.0]) + ); +} + +$it = $c->iterDocs(); +$it->rewind(); + +echo "done\n"; +// No close(), no cleanup: at shutdown the PHP objects are torn down in an order +// we do not control, with the iterator still open. If the adapter released the +// collection before the iterator, upstream ~CollectionImpl would wait forever +// for an iterator that can never be closed -- an infinite hang rather than a +// crash, which is what makes this case worth its own file. +?> +--CLEAN-- + +--EXPECT-- +done diff --git a/tests/test_ffi_load.phpt b/tests/test_ffi_load.phpt index 135a30c..0848bc4 100644 --- a/tests/test_ffi_load.phpt +++ b/tests/test_ffi_load.phpt @@ -18,6 +18,9 @@ $requiredFunctions = [ 'zvec_collection_create', 'zvec_collection_open', 'zvec_collection_free', + 'zvec_collection_create_iterator', + 'zvec_doc_iterator_next', + 'zvec_doc_iterator_free', 'zvec_collection_close', 'zvec_collection_flush', 'zvec_collection_optimize', @@ -145,7 +148,7 @@ try { echo "DONE\n"; ?> --EXPECT-- -All 73 FFI symbols resolved successfully +All 76 FFI symbols resolved successfully No FFI::cdef() inline string found in src/ZVec.php Header file zvec_ffi_php.h is used as source of truth Basic create/insert/optimize works diff --git a/tests/test_memory_doc_iterator.phpt b/tests/test_memory_doc_iterator.phpt new file mode 100644 index 0000000..7a0cc52 --- /dev/null +++ b/tests/test_memory_doc_iterator.phpt @@ -0,0 +1,93 @@ +--TEST-- +Memory leak: document iteration does not leak C handles or the C string array +--SKIPIF-- + +--FILE-- +setMaxDocCountPerSegment(1000) + ->addInt64('id', nullable: false) + ->addString('name', nullable: true) + ->addVectorFp32('embedding', dimension: 4, metricType: ZVecSchema::METRIC_IP); + + $c = ZVec::create($path, $schema); + for ($i = 0; $i < 20; $i++) { + $doc = new ZVecDoc("doc{$i}"); + $doc->setInt64('id', $i) + ->setString('name', "name_$i") + ->setVectorFp32('embedding', [(float)$i, 1.0, 0.0, 0.0]); + $c->insert($doc); + } + $c->flush(); + + // Warm up so one-off allocations are not counted as a leak. + for ($i = 0; $i < 5; $i++) { + foreach ($c->iterDocs() as $doc) { + // no-op + } + } + + $startHeap = memory_get_usage(); + $startRss = getVmRSS(); + + // Early close: the handle must be released even though iteration stops. + for ($i = 0; $i < 50; $i++) { + $it = $c->iterDocs(); + $it->rewind(); + $it->next(); + $it->next(); + $it->close(); + } + + // Full scan, auto-closed on exhaustion. + for ($i = 0; $i < 50; $i++) { + foreach ($c->iterDocs() as $doc) { + // no-op + } + } + + // Exercises the output-fields path, where toCStringArray() allocates both + // the char*[N] array and each string; iterDocs() frees the array itself. + for ($i = 0; $i < 20; $i++) { + foreach ($c->iterDocs(outputFields: ['id']) as $doc) { + // no-op + } + } + + $heapDelta = memory_get_usage() - $startHeap; + $rssDelta = getVmRSS() - $startRss; + + printf("heap delta: %d bytes\n", $heapDelta); + printf("VmRSS delta: %d kB\n", $rssDelta); + + $ok = $heapDelta < $THRESHOLD; + if ($startRss > 0) { + $ok = $ok && $rssDelta < $THRESHOLD; + } + echo $ok ? "PASS: memory delta within threshold\n" : "FAIL: memory delta above threshold\n"; +} finally { + exec('rm -rf ' . escapeshellarg($path)); +} +?> +--EXPECTF-- +heap delta: %d bytes +VmRSS delta: %d kB +PASS: memory delta within threshold diff --git a/tests/test_null_handle_collection.phpt b/tests/test_null_handle_collection.phpt index 7968844..a148ab5 100644 --- a/tests/test_null_handle_collection.phpt +++ b/tests/test_null_handle_collection.phpt @@ -51,6 +51,20 @@ $ffi->zvec_query_result_free(\FFI::addr($result)); // ensure clean $status = $ffi->zvec_collection_query_vector(null, null, \FFI::addr($result)); echo "PASS: zvec_collection_query_vector(null) returned code={$status->code}\n"; +// 10. zvec_collection_create_iterator(null, ...) — should return error +$it = $ffi->new('zvec_doc_iterator_t'); +$status = $ffi->zvec_collection_create_iterator(null, 0, null, 0, 1, \FFI::addr($it)); +echo "PASS: zvec_collection_create_iterator(null) returned code={$status->code}\n"; + +// 11. zvec_doc_iterator_next(null, ...) — should return error +$doc = $ffi->new('zvec_doc_t'); +$status = $ffi->zvec_doc_iterator_next(null, \FFI::addr($doc)); +echo "PASS: zvec_doc_iterator_next(null) returned code={$status->code}\n"; + +// 12. zvec_doc_iterator_free(null) — no-op +$ffi->zvec_doc_iterator_free(null); +echo "PASS: zvec_doc_iterator_free(null) is no-op\n"; + echo "PASS\n"; ?> --EXPECTF-- @@ -64,4 +78,7 @@ PASS: zvec_collection_schema(null) returned code=%d PASS: zvec_collection_path(null) returned code=%d PASS: zvec_collection_stats(null) returned code=%d PASS: zvec_collection_query_vector(null) returned code=%d +PASS: zvec_collection_create_iterator(null) returned code=%d +PASS: zvec_doc_iterator_next(null) returned code=%d +PASS: zvec_doc_iterator_free(null) is no-op PASS From 124eec4a7822e60d2814ff509427d9d89fffdac5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=C5=82as=20Piotr?= Date: Tue, 29 Sep 2026 11:41:06 +0200 Subject: [PATCH 2/2] test(ivf-rabitq): assert the top hit is near the target, not an exact PK RaBitQ is a lossy quantizer and IVF is approximate, so the exact top hit is not guaranteed. Measured 9/10 for the exact primary key and a 1-off otherwise, which made this test fail roughly 1 run in 10 in CI and forced a rerun on an unrelated PR. Asserts the winner is within 1 of the target instead, which is what the index actually promises. Both query paths are checked the same way. 12 consecutive runs green afterwards. --- tests/test_ivf_rabitq_index.phpt | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/test_ivf_rabitq_index.phpt b/tests/test_ivf_rabitq_index.phpt index 3df5f12..0b1b165 100644 --- a/tests/test_ivf_rabitq_index.phpt +++ b/tests/test_ivf_rabitq_index.phpt @@ -37,17 +37,23 @@ try { echo 'index type: ' . $c->getFieldSchema('v')->getIndexType() . "\n"; + // RaBitQ is a lossy quantizer and IVF is approximate, so the exact top hit is + // not guaranteed: measured 9/10 for the exact PK and a 1-off otherwise. + // Assert the winner is in a small neighbourhood of the target instead, which + // is what the index contract actually promises. + $near = static fn(string $pk): bool => abs((int)$pk - 42) <= 1; + $target = array_fill(0, 128, 42.0); $query = (new ZVecVectorQuery('v', $target))->setTopk(10)->setIvfRabitqParams(nprobe: 16); $hits = array_map(static fn(ZVecDoc $d): string => $d->getPk(), $c->queryVector($query)); - echo 'queryVector top: ' . $hits[0] . "\n"; + echo 'queryVector top: ' . ($near($hits[0]) ? 'near target' : 'WRONG: ' . $hits[0]) . "\n"; // The legacy query() path goes through zvec_collection_query_ex, which needs // its own IVF_RABITQ branch in validate_query_param_type() and // apply_query_params() to work at all. $legacy = (new ZVecVectorQuery('v', $target))->setTopk(10)->setIvfRabitqParams(nprobe: 16); $legacyHits = array_map(static fn(ZVecDoc $d): string => $d->getPk(), $c->query($legacy)); - echo 'query top: ' . $legacyHits[0] . "\n"; + echo 'query top: ' . ($near($legacyHits[0]) ? 'near target' : 'WRONG: ' . $legacyHits[0]) . "\n"; // Radius only, with no setIvfRabitqParams(): this goes through // ensure_query_params_for_field(), which without an IVF_RABITQ case would @@ -92,8 +98,8 @@ try { ?> --EXPECT-- index type: 7 -queryVector top: 42 -query top: 42 +queryVector top: near target +query top: near target radius-only query ok hnsw params rejected (code 3) PASS: IVF-RaBitQ index works