feat(schema): declare an FTS or invert index in addString() (#223) - #232
Merged
Merged
Conversation
zvec picks an I/O backend for DiskANN disk reads on first use: on Linux
it tries io_uring, then libaio, then falls back to synchronous pread();
macOS arm64 always uses pread(). The choice dominates DiskANN
throughput -- pread on Linux usually means libaio simply is not installed
-- and until now there was no way to observe it from PHP.
Three static getters on ZVec, next to getVersion() and following the
same pattern the Python SDK uses for its module-level zvec.io_backend_type():
ZVec::getIoBackendType(): int
ZVec::getIoBackendTypeName(int $type): string
ZVec::getIoBackendDescription(): string
plus IO_BACKEND_PREAD / IO_BACKEND_LIBAIO / IO_BACKEND_IO_URING (0/1/2),
the same values as the upstream C ABI. The description is where upstream
explains how to install an async backend when pread is in use.
These are deliberately static rather than per-collection: the value is
process-wide, it is not part of GlobalConfig, and upstream probes it
lazily, so the getters work without ZVec::init() at all. The test asserts
that.
Adhering to the #215 singleton rule: the adapter includes only the public
zvec/ailego/io/io_backend.h and calls upstream's exported
current_io_backend_type() / current_io_backend_description(), which are
compiled inside libzvec, so the IOBackend singleton is created only there.
The internal io_backend_def.h is not part of the SDK and IOBackend::Instance()
is never referenced from this module. The comment in ffi/zvec_ffi.cc records
why, so it does not get "fixed" later.
$ nm -C ffi/build/libzvec_ffi.so | grep -c 'IOBackend::Instance'
0
$ nm -D --defined-only ffi/build/libzvec_ffi.so | grep io_backend
zvec_get_io_backend_description
zvec_get_io_backend_type
zvec_get_io_backend_type_name
No status to check: these return plain values, matching getVersion(), so
self::checkStatus() is not involved and FFI::free() is not needed -- the
C side owns the memory (a static literal for the name, a thread-local
buffer for the description).
Tests: 192/192, 0 skipped, 0 failed, 2 expected fail. The new
tests/test_io_backend.phpt covers the constants, the name mapping including
the "unknown" fallback, that the value is valid before init(), that the
description mentions the reported backend, the macOS pread rule, and that
the cached value is stable across repeated calls and across init().
tests/test_ffi_load.phpt gained the three symbols (60 -> 63).
Exposes two upstream GlobalConfig::ConfigData fields that had no PHP equivalent. jiebaDictDir is the folder holding jieba.dict.utf8 and hmm_model.utf8 for the jieba FTS tokenizer; ftsBruteForceByKeysRatio (0.0-1.0, default 0.05) is where an FTS query stops walking posting lists and starts scoring candidates one by one, which pays off when the scalar filter is very selective. Read back with getJiebaDictDir() and getFtsBruteForceByKeysRatio(); with no option given the bundled zvec_data/jieba_dict is still found automatically. null rather than 0.0 as the ratio default, because 0.0 is a real value upstream accepts rather than a "use the default" marker like the other ratios. Both values are validated in PHP before any FFI call, which is the only place the check can live. Upstream GlobalConfig::initialize() sets its initialized flag *before* validating, so a value it rejects still leaves the library marked initialized, with defaults, and every later init() returns OK without applying anything -- a silent misconfiguration. NAN also passes the upstream range test, since NAN < 0 and NAN > 1 are both false. A bad jiebaDictDir is rejected for a second, harder reason: cppjieba calls abort() rather than returning a Status, so a wrong folder kills the process with exit 134 later, when a jieba FTS index is created, with no catchable error. The check therefore requires both dictionary files, not just the directory. (Confirmed: the same abort is reachable through per-field extraParams and through ZVEC_JIEBA_DICT_DIR, which is a separate gap.) The getters use global_config_ptr() rather than GlobalConfig::Instance(), per the #215 singleton rule. The ratio message uses var_export() because interpolating NAN emits a PHP warning; the message is the same shape as the other init() validation errors. Tests: 194/194, 0 skipped, 0 failed, 2 expected fail. The two new tests run in separate processes because upstream config is applied only once per process: - test_init_fts_options.phpt: six rejection cases (1.5, -0.1, NAN, empty dir, missing dir, existing dir without the dictionary files) each asserting isInitialized() is still false afterwards, then a successful init against a *copy* of the bundled dictionary plus an end-to-end jieba FTS query that returns j1, proving a custom folder is the one actually used. - test_init_fts_defaults.phpt: the upstream defaults, 0.05 and the bundled dictionary, with no new options passed.
Declaring a full-text index on a STRING field took two steps: create the
collection, then call createIndex(). Upstream puts the index params on the
field itself, so the index can exist from the first insert:
$schema->addString('body', indexParams: ZVecIndexParams::forFts());
$schema->addString('tag', indexParams: ZVecIndexParams::forInvert());
This is what the Python SDK does with FieldSchema(index_param=...), and
what upstream's own hybrid tests do. The new argument is last and
optional, so existing calls are unaffected.
Two validation cases the older signature could not express. Passing both
$withInvertIndex and $indexParams is ambiguous and now throws. And
unlike the other add*() methods, a duplicate field name is reported here
rather than silently dropped: upstream returns AlreadyExists from
add_field(), and zvec_schema_add_field_* discards that Status, so
addString('body') twice currently loses a field without a word. This
function propagates it because it has to build index params anyway.
A mismatched index type is deliberately *not* rejected here. Upstream
reports it at ZVec::create() time with a message naming the field
("scalar field[body] does not support vector index params, but got
index_type ..."), which is clearer than anything the adapter could
produce, so there is one source of truth.
Upstream's FieldSchema constructor clones the index params, so the
ZVecIndexParams object keeps owning its handle and is freed by its own
destructor as usual; nothing new to release on the PHP side.
FFI: zvec_schema_add_field_string_with_index(), declared next to
zvec_collection_create_index() in both headers because the
zvec_index_params_t typedef comes after the schema block.
Tests: 195/195, 0 skipped, 0 failed, 2 expected fail. The new
tests/test_schema_fts_index.phpt queries for a term without any
createIndex() call, checks getIndexType() is 11 before and after a
reopen, covers invert via params (10), both validation errors, and the
pre-existing argument combinations. test_ffi_load.phpt gained the symbol
(67 -> 68).
Note: tests/test_diskann_index.phpt was seen failing once with only
doc2 returned instead of doc1,doc2,doc3, and passed on five subsequent
single runs and four subsequent full-suite runs. Not reproduced and not
caused by this change; a flaky DiskANN query result on a 3-document
collection, worth a separate look if it shows up again.
…ndex # Conflicts: # CHANGELOG.md # README.md # tests/test_ffi_load.phpt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #223.
Summary
A full-text index on a STRING field used to need two steps — create the collection, then
createIndex(). Upstream puts index params on the field, so it can now be one:The index exists from the first insert. This matches the Python SDK's
FieldSchema(index_param=...)and how upstream's own hybrid tests build collections. The new argument is last and optional, so existing calls are unchanged.Design notes
A mismatched index type is deliberately not rejected in PHP. Upstream reports it at
ZVec::create()time with a message naming the field:That is clearer than anything the adapter could produce, so there is one source of truth rather than two. The test asserts the upstream path rejects it.
A duplicate field name is now reported here, unlike in the other
add*()methods.add_field()returnsAlreadyExists, and everyzvec_schema_add_field_*function discards thatStatus— soaddString('body')twice currently loses a field silently. This one propagates it because it already has to build index params and return a status. Worth noting as a pre-existing bug in the other setters that is out of scope here (changing them is a BC-sensitive sweep).No new memory handling. Upstream's
FieldSchemaconstructor clones the index params, so theZVecIndexParamsobject keeps owning its handle and is freed by its own destructor as usual.The FFI function is declared next to
zvec_collection_create_index()in both headers rather than next tozvec_schema_add_field_string(), because thezvec_index_params_ttypedef comes after the schema block.Verification
Full suite from a clean build:
tests/test_schema_fts_index.phptcovers the query withoutcreateIndex(),getIndexType()before and after a reopen, invert via params, both validation errors, and the pre-existing argument combinations.test_ffi_load.phptgained the symbol (67 → 68).test_dbs/is left with only.gitignore.One thing to watch
tests/test_diskann_index.phptfailed once during this work, returningdoc2where the expectation wasdoc1,doc2,doc3. It then passed on five consecutive single runs and four consecutive full-suite runs, and the failure did not reproduce with the change stashed. Not caused by this PR as far as I can tell — most likely a flaky DiskANN result on a 3-document collection — but flagging it rather than leaving it unexplained.🤖 Generated with Claude Code