feat(c-api): add nullable setter and per-doc write results

[kdy1]

Summary

• add zvec_doc_set_field_null for explicit nullable writes
• add per-document write result APIs for insert/update/upsert/delete
• add allocator-safe free API (zvec_free_ptr) and tests

Tests

• cmake --build build-capi --target c_api_test -j4
• ctest --output-on-failure -R c_api_test

Greptile Summary

This PR extends the C API with four new _with_results DML variants (insert/update/upsert/delete) that return per-document ZVecWriteResult structs, a zvec_doc_set_field_null setter for explicit nullable writes, a generic zvec_free_ptr helper, and fetch-side normalization of nullable fields. The overall design and API shape are sound and consistent with the existing C API patterns.

Key concerns:

• P0 – Crash on OOM in copy_string: The pre-existing copy_string helper does not check if malloc returns null before calling strcpy, causing a null-pointer dereference. This is now called 2×N times per batch write in build_write_results, making it realistically reachable under memory pressure.
• P1 – Null pk in ZVecWriteResult: copy_string returns nullptr for empty strings, so documents without a PK silently produce a nullptr in results[i].pk. The struct documentation does not call this out, risking unexpected null dereferences in caller code.
• P2 – Silent schema-fetch failure during fetch normalization: If Schema() fails while normalizing nullable fields on fetch, the function silently skips normalization and returns data without null markers, directly undermining the feature's correctness guarantee.
• P2 – zvec_free_ptr test doesn't exercise cross-DLL safety: The test allocates with the caller's malloc rather than with a zvec API, so it does not validate the allocator-symmetry the function is designed to provide.

Confidence Score: 2/5

• Not safe to merge as-is — a null pointer dereference in copy_string can crash the process under OOM, and empty PKs silently produce null fields in the result array.
• The P0 crash path in copy_string (no malloc-failure check before strcpy) is now amplified by the new batch loop in build_write_results. The P1 silent-null-pk issue can cause unexpected null dereferences in callers. Both must be fixed before merging.
• src/c_api/c_api.cc — specifically copy_string (line 793) and build_write_results (lines 820-851)

Important Files Changed

Filename	Overview
src/c_api/c_api.cc	Adds nullable setter, per-doc write result helpers, and fetch-side nullable normalization. Critical: copy_string crashes on malloc failure (null deref via strcpy), and empty PKs silently produce null entries in result arrays.
src/include/zvec/c_api.h	Declares ZVecWriteResult struct and four new _with_results DML APIs plus zvec_free_ptr / zvec_doc_set_field_null. API shape and documentation are clean; the ZVecWriteResult.pk field should note it may be null.
tests/c_api/c_api_test.c	Good coverage for null-field API, nullable roundtrip, and DML result APIs. Minor: zvec_free_ptr test uses caller-allocated buffer instead of API-allocated memory, which doesn't validate the intended cross-DLL safety.

Sequence Diagram

sequenceDiagram
    participant Caller as C Caller
    participant API as C API (c_api.cc)
    participant Coll as zvec::Collection

    Note over Caller,Coll: _with_results DML flow (insert/update/upsert)
    Caller->>API: zvec_collection_upsert_with_results(collection, docs, N, &results, &count)
    API->>API: collect_doc_pks(docs, N) → pks[]
    API->>API: convert_zvec_docs_to_internal(docs, N) → internal_docs[]
    API->>Coll: Upsert(internal_docs)
    Coll-->>API: Expected<vector<Status>>
    API->>API: handle_expected_result(result)
    alt operation failed
        API-->>Caller: ZVEC_ERROR_* (results=nullptr, count=0)
    else operation succeeded
        API->>API: build_write_results(statuses, pks, &results, &count)
        Note over API: calloc(N, sizeof(ZVecWriteResult))<br/>loop: copy_string(pk), copy_string(message)
        API-->>Caller: ZVEC_OK, results[N], count=N
        Caller->>API: zvec_write_results_free(results, count)
        API->>API: free_write_results_internal(results, count)
    end

    Note over Caller,Coll: fetch + nullable normalization flow
    Caller->>API: zvec_collection_fetch(collection, pks, N, &docs, &count)
    API->>Coll: Fetch(pk_vector)
    Coll-->>API: Expected<DocPtrMap>
    API->>Coll: Schema()
    Coll-->>API: Expected<CollectionSchema>
    alt schema available
        API->>API: normalize_nullable_fields_for_fetch(schema, doc_map)
        Note over API: For each nullable field absent from a doc,<br/>call doc->set_null(field_name)
    else schema fetch failed
        API->>API: (silently skip normalization)
    end
    API->>API: convert_fetched_document_results(doc_map, &docs, &count)
    API-->>Caller: ZVEC_OK, docs[N]

Loading

Comments Outside Diff (1)

1. src/c_api/c_api.cc, line 793-799 (link)

   Null dereference crash on malloc failure in copy_string

   When malloc returns nullptr (OOM), strcpy(copy, str.c_str()) is called with a null destination pointer, causing a segfault/undefined behavior instead of returning an error. This is a pre-existing bug, but build_write_results now calls copy_string in a tight loop — 2×N times for a batch of N documents — making this path reachable under memory pressure in production:

   char *copy = static_cast<char *>(malloc(str.length() + 1));
   strcpy(copy, str.c_str());  // UB if malloc returned nullptr

   Fix by checking before dereferencing:

   Additionally, build_write_results should then detect a null pk/message from a non-empty input and clean up the partially-built array before returning ZVEC_ERROR_INTERNAL_ERROR.

_{Last reviewed commit: f137932}

Greptile also left 3 inline comments on this PR.

[greptile-apps]

Null PK in results when doc has empty primary key

copy_string returns nullptr for empty strings (line 795: if (str.empty()) return nullptr;). This means results[i].pk will silently be nullptr whenever a document was written without setting a PK (e.g. when docs[i] was null in collect_doc_pks, which pushes ""). The struct documentation says const char *pk; /**< Primary key (allocated by API) */ which implies it's always non-null.

Callers who rely on results[i].pk being non-null for all entries will silently encounter a null dereference. Consider returning an empty string instead of null to ensure a consistent, non-null result:

Suggested change

	const std::string pk = i < pks.size() ? pks[i] : std::string();
	const std::string message = statuses[i].message();
	(*results)[i].pk = copy_string(pk);
	(*results)[i].message = copy_string(message);
	(*results)[i].code = status_to_error_code(statuses[i]);
	for (size_t i = 0; i < *result_count; ++i) {
	const std::string pk = i < pks.size() ? pks[i] : std::string();
	const std::string message = statuses[i].message();
	// Allocate empty string for pk/message if blank, so callers always get a non-null pointer
	auto alloc_string = [](const std::string &s) -> char * {
	char *copy = static_cast<char *>(malloc(s.length() + 1));
	if (!copy) return nullptr;
	memcpy(copy, s.c_str(), s.length() + 1);
	return copy;
	};
	(*results)[i].pk = alloc_string(pk);
	(*results)[i].message = alloc_string(message);
	(*results)[i].code = status_to_error_code(statuses[i]);
	}

This also fixes the related copy_string malloc-failure issue described below.

[greptile-apps]

Silent schema-fetch failure during nullable normalization

If Schema() returns an error (e.g. I/O failure, corrupted metadata), nullable-field normalization is quietly skipped:

if (schema_result.has_value()) {
    normalize_nullable_fields_for_fetch(schema_result.value(), doc_map);
}
// no else: silently falls through with un-normalized docs

The caller receives documents where nullable fields that are absent are not marked null — directly contradicting the intent of this feature. At minimum, consider logging a warning or propagating the error so callers are aware that the returned data may be inconsistent:

Suggested change

	auto doc_map = result.value();
	auto schema_result = (*coll_ptr)->Schema();
	if (schema_result.has_value()) {
	normalize_nullable_fields_for_fetch(schema_result.value(), doc_map);
	}
	auto doc_map = result.value();
	auto schema_result = (*coll_ptr)->Schema();
	if (schema_result.has_value()) {
	normalize_nullable_fields_for_fetch(schema_result.value(), doc_map);
	} else {
	set_last_error("Failed to fetch schema for nullable normalization: " +
	schema_result.error().message());
	// Proceed with un-normalized data rather than failing the fetch
	}
	return convert_fetched_document_results(doc_map, results, doc_count);)

[greptile-apps]

Test for zvec_free_ptr doesn't reflect actual API usage

The test allocates memory with the caller's own malloc and then frees it via zvec_free_ptr. The documented purpose of zvec_free_ptr is to free memory allocated by the zvec C API to avoid allocator-mismatch across DLL boundaries. Using it on a buffer you malloc'd yourself is technically valid (same allocator), but it doesn't exercise the intended cross-DLL safety story.

A better test would call a zvec API that returns a heap-allocated buffer (e.g. a string returned from zvec_get_last_error_message or similar), and free it with zvec_free_ptr. This would validate the allocator symmetry the API is designed to guarantee.

[chinaux]

Currently, the C++ implementation omits the key from the document if the field is null. However, if you require the document to include all fields regardless, we can certainly accommodate that.

[chinaux]

lgtm