feat(iterator): add DocIterator for full collection traversal by YongqiYin · Pull Request #597 · alibaba/zvec

YongqiYin · 2026-07-16T12:53:07Z

Add streaming full-collection traversal across C++/C/Python:

C++: Collection::CreateIterator + DocIterator (isolated Flush+snapshot scan, ConcatenatingReader across segments, FilteringReader for deletes, batch-prefetched vectors)
C API: zvec_collection_create_iterator/next/close + iterator options
Python: collection.iter_docs() generator (constant memory)
Tests: C++ (unit/integration/concurrency/perf), C API, Python

Relates to #380

Copilot

Pull request overview

This PR introduces a full-collection document iterator (“DocIterator”) across the C++ core, C API, and Python bindings to enable streaming traversal/export without relying on large topk queries (relates to #380).

Changes:

Add C++ Collection::CreateIterator() and DocIterator with snapshot isolation, segment concatenation, and delete filtering.
Expose the iterator via the C API (zvec_collection_create_iterator/next/close) and Python (Collection.iter_docs() generator).
Add C++/C/Python tests covering basic iteration, delete filtering, field selection, and concurrency/isolation.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
tests/db/iterator_test.cc	New C++ unit/integration/concurrency/perf tests for full traversal.
tests/c/c_api_test.c	Adds C API iterator tests and option handling coverage.
src/include/zvec/db/options.h	Introduces `IteratorOptions` (output_fields/include_vector).
src/include/zvec/db/doc_iterator.h	Public C++ `DocIterator` interface.
src/include/zvec/db/collection.h	Adds `Collection::CreateIterator()` API.
src/include/zvec/c_api.h	Adds public C iterator and iterator-options API.
src/db/index/segment/filtering_reader.h	New Arrow reader wrapper to filter deleted docs.
src/db/index/segment/concatenating_reader.h	New Arrow reader to concatenate readers across segments.
src/db/doc_iterator.cc	Implements row-by-row Doc materialization + optional vector prefetch.
src/db/doc_iterator_internal.h	Internal `DocIterator::Impl` definition (lifetime ordering, caches).
src/db/collection.cc	Implements iterator creation, snapshot scan, and reader chain construction.
src/binding/python/model/python_collection.cc	Exposes `_DocIterator` + `Collection.CreateIterator()` to Python.
src/binding/python/include/python_collection.h	Declares `bind_iterator()` hook.
src/binding/c/c_api.cc	Implements iterator options + iterator handles for the C API.
python/zvec/model/collection.py	Adds `Collection.iter_docs()` streaming generator.
python/tests/test_iter_docs.py	New Python tests for iterator behavior and isolation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

YongqiYin · 2026-07-16T13:01:36Z

@copilot resolve the merge conflicts in this pull request

CLAassistant · 2026-07-16T13:13:01Z

All committers have signed the CLA.

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Add streaming full-collection traversal across C++/C/Python: - C++: Collection::CreateIterator + DocIterator (isolated Flush+snapshot scan, ConcatenatingReader across segments, FilteringReader for deletes, batch-prefetched vectors) - C API: zvec_collection_create_iterator/next/close + iterator options - Python: collection.iter_docs() generator (constant memory) - Tests: C++ (unit/integration/concurrency/perf), C API, Python Relates to alibaba#380

- iterate segment-by-segment: one FilteringReader per segment (drop ConcatenatingReader), so each batch's owning segment is known directly - hide CollectionImpl from the public doc_iterator.h (Pimpl fwd-decl + public ctor, no friend/Passkey in the public header) - propagate errors to callers instead of logging: Segment::scan failure, missing vector indexer, vector fetch failure, buffer conversion failure - fetch vectors via the segment-local row id column (LOCAL_ROW_ID) instead of g_doc_id - min_doc_id arithmetic (safe for compacted segments with non-contiguous doc ids) - use has_record() for the writable flush condition (aligns with alibaba#618) - drop stale comments and unused includes

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Address review comments on shared conversion helpers: - Add db/index/common/doc_field_converter with two shared helpers: ConvertVectorDataBufferToDocField (dense + sparse vector buffers) and ConvertArrowRowToDocField (all 9 scalar + 9 array data types). - DocIterator, SegmentImpl::Fetch and sqlengine fill_doc_field now share the same row-level conversion, removing ~650 lines of duplicated type-switch code. - SegmentImpl::Fetch boxes its single-value scalars via arrow::MakeArrayFromScalar to reuse the row converter, keeping its lenient log-and-continue contract unchanged. - fill_doc_vector/fill_doc_sparse_vector stay in sqlengine: they decode a query-result-specific Arrow encoding with a single consumer.

egolearner · 2026-08-03T08:52:51Z

+
+  std::pair<std::vector<IndexType>, std::vector<ValueType>> sparse_vector_pair(
+      std::move(indices_vector), std::move(values_vector));
+  doc->set(field->name(), sparse_vector_pair);


egolearner · 2026-08-03T08:53:02Z

+  const T *data_ptr = reinterpret_cast<const T *>(buffer.data.data());
+  size_t data_size = buffer.data.size() / sizeof(T);
+  std::vector<T> vector_data(data_ptr, data_ptr + data_size);
+  doc->set(field->name(), vector_data);


egolearner · 2026-08-03T08:54:47Z

+template <typename ArrowArrayT>
+Status SetScalarField(const std::shared_ptr<arrow::Array> &array, int64_t row,
+                      const std::string &name, Doc *doc) {
+  auto typed_array = std::dynamic_pointer_cast<ArrowArrayT>(array);


保持指针的方式，可以减少不必要的shared_ptr创建和析构

已改为裸指针入参 + static_cast

egolearner · 2026-08-03T08:55:01Z

+template <typename ArrowArrayT, typename T>
+Status SetListField(const std::shared_ptr<arrow::Array> &array, int64_t row,
+                    const std::string &name, Doc *doc) {
+  auto list_array = std::dynamic_pointer_cast<arrow::ListArray>(array);


已改，同上

egolearner · 2026-08-03T09:01:58Z

+  if (!typed_array) {
+    return Status::InternalError("Arrow array type mismatch for field: ", name);
+  }
+  if constexpr (std::is_same_v<ArrowArrayT, arrow::StringArray> ||


需要检查是否null，以string为例，""和null是不一样的

已加，ConvertArrowRowToDocField 入口已有array->IsNull(row) 检查，此处加了第二道，保证若未来直接调SetScalarField 行为正确

egolearner · 2026-08-03T09:24:14Z

+    impl_->vector_cache_.clear();
+    impl_->segments.clear();
+    impl_->delete_store.reset();
+    impl_->schema.reset();


可以直接reset impl_吗？简单一点

如果不行的话，最后也reset一下impl_吧，当前虽然没问题，但万一后面逻辑有修改引入问题

已在最后impl_.reset()
没有直接impl_.reset()是将现在的析构顺序固化，相对更安全
若进一步保证未来安全可以加一个单测，我觉得目前不需要做到这程度

egolearner · 2026-08-03T09:30:41Z

+    auto uid_array =
+        std::dynamic_pointer_cast<arrow::StringArray>(batch.column(uid_col));
+    if (uid_array) {
+      // GetView avoids the per-row Scalar allocation of GetScalar()->ToString()


这里现场马上构造了一个string，使用这个注释没什么意义，可以去掉。

egolearner · 2026-08-03T09:34:16Z

+  if (impl_->include_vector && impl_->schema) {
+    for (const auto &field : impl_->schema->vector_fields()) {
+      auto it = impl_->vector_cache_.find(field->name());
+      if (it == impl_->vector_cache_.end()) continue;


这两行加上报错吧

egolearner · 2026-08-03T09:38:50Z

+
+namespace zvec {
+
+class DocIterator {


rebase最新代码，加上ZVEC_API，其他类似问题一并改下

egolearner · 2026-08-03T09:43:10Z

+/**
+ * @brief Opaque handle for iterator options.
+ *
+ * Follows the same pattern as zvec_collection_options_t (opaque type + setters)


这个描述是开发者的内心独白，作为注释对用户没什么意义，可以精简下

egolearner · 2026-08-03T09:54:05Z

+            >>> for doc in collection.iter_docs(include_vector=False):
+            ...     print(doc.id, doc.field("title"))
+        """
+        iterator = self._obj.CreateIterator(output_fields, include_vector)


Python snapshot 仍延迟到第一次 next()。 [collection.py (line 407)]位于包含 yield 的 generator body 中，因此 iter_docs() 调用时不会执行 CreateIterator。it = iter_docs(); insert(...); next(it) 会看到调用后写入的数据，与“snapshot taken at call time”不符。现有测试先执行 next(it) 再写入，未覆盖该问题。

参考

def iter_docs(...): iterator = self._obj.CreateIterator(output_fields, include_vector) def generate(): try: for core_doc in iterator: py_doc = convert_to_py_doc(core_doc, self.schema) if py_doc is not None: yield py_doc finally: iterator.close() return generate()

已参考修复，并补了测试

egolearner · 2026-08-03T11:02:08Z

+    bool loaded = false;
+    while (impl_->current_segment_index < impl_->readers.size()) {
+      auto &reader = impl_->readers[impl_->current_segment_index];
+      auto status = reader->ReadNext(&impl_->current_batch);


一次会读多少数据回来？

egolearner · 2026-08-03T11:03:51Z

+  std::vector<RecordBatchReaderPtr> readers;
+  readers.reserve(segments.size());
+  for (const auto &seg : segments) {
+    auto scalar_reader = seg->scan(scan_columns);


可以一次只打开一个reader，用完了释放，减少整体的资源需求

…view feedback Per review feedback on the shared-lock model: a maintenance operation waiting for a running Optimize became a pending exclusive acquirer of the schema lock, blocking new readers on typical shared_mutex implementations. Changes: - Replace optimize_mtx_ with maintenance_mtx_ (acquired by Optimize, schema DDLs, Flush, Close and Destroy before the schema lock). Lock order: maintenance -> schema -> write -> SegmentManager. - Restructure Optimize into three phases: exclusive seal, lock-free compact (MoveDirectory + Segment::Open moved to lock-free phase 2 tail, so an open failure aborts before the manifest is persisted), and short exclusive commit that restores in-place reload_vector_index with eager file removal (no reader can hold indexers under exclusive schema lock). - Downgrade read-only Stats()/Schema()/Options() to shared locking. - Remove replace_segments() and the deferred index-file removal machinery introduced earlier in this PR; they are only needed by lock-free consumers (DocIterator, alibaba#597) and will land there. - Keep and adapt the two regression UTs (concurrent read-write and superseded index file count).

- doc_field_converter: pass raw pointers instead of shared_ptr, add per-row null check, move vectors into Doc::set, direct return per case - filtering_reader: static_cast instead of dynamic_pointer_cast - collection: validate output_fields (schema membership + no duplicates), build_scan_columns returns Result, move scalar_reader into reader - doc_iterator: reset impl_ in Close, report vector cache errors instead of silently skipping, drop stale comment - doc_iterator.h: add ZVEC_API export macro - c_api.h: trim implementation-detail comment - collection.py: take the snapshot eagerly at iter_docs() call time

Extend the reviewer's shared_ptr-reduction feedback to the remaining hot paths: - ConvertArrowRowToDocField now takes a raw Array* (callers pass batch.columns()[i].get() / chunk.get()), removing per-row atomic ref-count traffic. - DocIterator PK/doc_id/row-id extraction uses type_id() + static_cast instead of per-row dynamic_pointer_cast. - Update the Scan comment after alibaba#614: snapshot consistency comes from the write_mtx_ atomic snapshot + shared_ptr keep-alive, not from blocking Optimize (which now takes the schema lock in shared mode).

Copilot

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.

Suppressed comments (12)

tests/db/iterator_test.cc:183

The iterator is still alive when collection->Destroy() is called, which can leave Arrow/segment resources open during destruction and cause flaky cleanup. Close the iterator before destroying the collection.

  EXPECT_EQ(count, N - static_cast<int>(pks_to_delete.size()));

  collection->Destroy();

tests/db/iterator_test.cc:271

The iterator is still alive when collection->Destroy() is called, which can keep segment/Arrow resources open during destruction. Close the iterator before destroying the collection to avoid flaky cleanup.

  EXPECT_EQ(count, N);
  collection->Destroy();

tests/db/iterator_test.cc:317

The iterator is still alive when collection->Destroy() is called, which can keep segment/Arrow resources open during destruction. Close the iterator before destroying the collection to avoid flaky cleanup.

  EXPECT_EQ(count, 5);
  collection->Destroy();

tests/db/iterator_test.cc:391

The iterator is not closed before collection->Destroy(). Keeping the iterator alive can keep file handles open and make Destroy() flaky on some platforms. Close the iterator before destroying the collection.

  EXPECT_EQ((*a_s)[0], "value_" + std::to_string(kId));

  collection->Destroy();

tests/db/iterator_test.cc:447

The iterator is not closed before collection->Destroy(). If the iterator still holds segment/Arrow resources, destroying the collection can be flaky (especially on Windows). Close the iterator before destroying the collection.

  EXPECT_EQ(count, N);
  EXPECT_EQ(seen_pks.size(), (size_t)N);
  collection->Destroy();

tests/db/iterator_test.cc:509

Both iterators (iter and iter2) are still alive when collection->Destroy() is called. Close them before destroying the collection to ensure underlying segment/Arrow resources are released and cleanup is reliable.

  EXPECT_EQ(count2, N + 200);

  collection->Destroy();

tests/db/iterator_test.cc:556

The iterator is still alive when collection->Destroy() is called, which can keep resources open during destruction and cause flaky cleanup. Close the iterator before destroying the collection.

  EXPECT_EQ(count, N);
  collection->Destroy();
}

tests/db/iterator_test.cc:607

The iterator is still alive when collection->Destroy() is called. Close it before destroying the collection to ensure segment/Arrow resources are released and test cleanup is reliable.

            << std::endl;

  collection->Destroy();
}

tests/db/iterator_test.cc:16

std::cout is used later in this test file (Performance100k), but <iostream> is not included. This can break compilation depending on transitive includes.

#include <chrono>

tests/db/iterator_test.cc:126

The iterator is still alive when collection->Destroy() is called. On platforms with strict file-handle semantics (and per DocIterator's internal comment about releasing Arrow handles before segment cleanup), this can make Destroy() fail or become flaky. Close the iterator (or let it go out of scope) before destroying the collection.

This issue also appears in the following locations of the same file:

line 181
line 270
line 316
line 389
line 445
...and 3 more

  EXPECT_EQ(r.value(), nullptr) << "Expected EOF on empty collection";

  collection->Destroy();

src/binding/c/c_api.cc:7333

zvec_collection_create_iterator always maps C++ iterator creation failures to ZVEC_ERROR_INTERNAL_ERROR, even when the underlying Status is INVALID_ARGUMENT (e.g., unknown/duplicate output fields). This loses actionable error information for C callers; use the existing status_to_error_code() helper for consistent mapping.

        SET_LAST_ERROR(ZVEC_ERROR_INTERNAL_ERROR,
                       "Failed to create iterator: " +
                           result.error().message());
        return ZVEC_ERROR_INTERNAL_ERROR;

src/binding/c/c_api.cc:7361

zvec_doc_iterator_next converts all iterator failures into ZVEC_ERROR_INTERNAL_ERROR, which hides the underlying StatusCode and is inconsistent with other C API wrappers that use status_to_error_code(). Map the error code from result.error() so callers can distinguish invalid arguments vs internal failures.

      if (!result.has_value()) {
        SET_LAST_ERROR(ZVEC_ERROR_INTERNAL_ERROR,
                       "Iterator next failed: " + result.error().message());
        return ZVEC_ERROR_INTERNAL_ERROR;
      }

…ue patterns - Gate per-element IsNull checks behind an O(1) null_count() precheck (the common null-free case skips validity checks entirely, matching the optimization the reviewer asked to preserve). - Unify element extraction on Value() for all list element types (string/binary Value returns string_view, emplace_back converts it to std::string in place), removing the if-constexpr type branch.

Regression test for the deferred-snapshot issue: creating the iterator without consuming it must already freeze the snapshot, so documents inserted between iter_docs() and the first next() stay invisible. Verified red on the lazy-generator version, green on the fix.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

- Fetch reads Scalars directly again and drops MakeArrayFromScalar boxing; its LIST branch now shares value extraction with the iterator/SQL-engine path via ExtractTypedArrayValues . - Open one segment reader lazily and release it when the segment is exhausted, so at most one segment's files stay open. - Prefetch vectors in bounded windows (kIteratorVectorPrefetchWindow) so a large Parquet row group cannot cache a million vectors. - Skip empty batches defensively; fail instead of silently emitting docs when uid/g_doc_id columns are missing; cache column indices per batch to avoid per-row GetFieldIndex lookups.

…pshot side effects - output_fields accepts only forward fields and rejects duplicates; validation runs before the writing segment is sealed so invalid options fail fast with no side effect. - Map CreateIterator/Next failures through status_to_error_code in the C API so callers see INVALID_ARGUMENT instead of INTERNAL_ERROR. - Document the seal side effect and the close-before-destroy contract in the C++/C/Python API docs.

- Add OutputFieldsSelection and InvalidOutputFieldsRejected. - Add ParquetVectorPrefetchWindows: a row group larger than the prefetch window verifies window-refill alignment. - Close iterators before Destroy() and add the missing <iostream> include.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI lite review requested due to automatic review settings July 16, 2026 12:53

YongqiYin requested review from Cuiyus, chinaux, feihongxu0824 and zhourrr as code owners July 16, 2026 12:53

github-actions Bot assigned YongqiYin Jul 16, 2026

Copilot started reviewing on behalf of YongqiYin July 16, 2026 12:53 View session

Copilot AI reviewed Jul 16, 2026

View reviewed changes

YongqiYin force-pushed the feat/doc-iterator branch from b596564 to 4aca7cb Compare July 16, 2026 13:01

YongqiYin force-pushed the feat/doc-iterator branch from 4aca7cb to feef1d7 Compare July 16, 2026 16:53

YongqiYin requested a review from Copilot July 17, 2026 03:15

Copilot started reviewing on behalf of YongqiYin July 17, 2026 03:15 View session

Copilot AI reviewed Jul 17, 2026

View reviewed changes

Comment thread src/db/collection.cc Outdated

Comment thread src/db/doc_iterator.cc

Comment thread src/db/doc_iterator.cc Outdated

YongqiYin force-pushed the feat/doc-iterator branch 2 times, most recently from 96273fc to 4f542e0 Compare July 17, 2026 06:04

egolearner requested changes Jul 20, 2026

View reviewed changes

YongqiYin force-pushed the feat/doc-iterator branch from 4f542e0 to 16c0bd2 Compare July 21, 2026 09:15

Merge branch 'main' into feat/doc-iterator

fff802a

feihongxu0824 reviewed Jul 24, 2026

View reviewed changes

Comment thread src/include/zvec/db/doc_iterator.h Outdated

YongqiYin added 2 commits July 29, 2026 15:12

chore: merge upstream main into feat/doc-iterator

86cff17

Copilot AI review requested due to automatic review settings July 29, 2026 07:14

Copilot AI reviewed Jul 29, 2026

Copilot AI review requested due to automatic review settings July 30, 2026 11:53

This comment was marked as spam.

Sign in to view

egolearner requested changes Aug 3, 2026

View reviewed changes

egolearner reviewed Aug 3, 2026

View reviewed changes

YongqiYin added 3 commits August 5, 2026 17:30

Merge remote-tracking branch 'upstream/main' into feat/doc-iterator

959be61

Copilot AI review requested due to automatic review settings August 5, 2026 09:47

Copilot started reviewing on behalf of YongqiYin August 5, 2026 09:47 View session

Copilot AI reviewed Aug 5, 2026

View reviewed changes

YongqiYin added 2 commits August 6, 2026 11:23

Copilot AI review requested due to automatic review settings August 6, 2026 12:54

Copilot AI reviewed Aug 6, 2026

YongqiYin added 3 commits August 7, 2026 17:47

Copilot AI review requested due to automatic review settings August 7, 2026 09:48

Copilot AI reviewed Aug 7, 2026


		namespace zvec {

		class DocIterator {

Uh oh!

Conversation

YongqiYin commented Jul 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

YongqiYin commented Jul 16, 2026

Uh oh!

CLAassistant commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

CLAassistant commented Jul 16, 2026 •

edited

Loading