fix: do not mutate a reused vector_index_config when adding a quantizer - #2173
Open
joaquinhuigomez wants to merge 1 commit into
Open
joaquinhuigomez wants to merge 1 commit into
joaquinhuigomez wants to merge 1 commit into
Conversation
`_IndexWrappers.single()` and `.multi()` set the quantizer, the multivector config and the encoding on the caller's object. Two vectors built from one `Configure.VectorIndex.hnsw(...)` therefore ended up holding the same pydantic instance, so a quantizer requested for one vector was also sent for the other, and the caller's own object was mutated as a side effect. That is hard to undo: `_CollectionConfigUpdate` refuses to change a quantizer after the collection has been created, so the collection has to be dropped and recreated. Copy the config objects before touching them, as weaviate#2143 did for `_FilterBase._target_path`. `multi()` copies `multi_vector_config` too, since it assigns the encoding onto it in the same way.
![orca-security-eu[bot]](https://avatars.githubusercontent.com/in/276244?s=60&v=4){kind=small}
There was a problem hiding this comment.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
 Passed |
Infrastructure as Code |  0  0  0  0 |
View in Orca |
| Passed |
SAST | 0 0 0 0 |
View in Orca |
| Passed |
Secrets | 0 0 0 0 |
View in Orca |
| Passed |
Vulnerabilities | 0 0 0 0 |
View in Orca |
|
To avoid any confusion in the future about your contribution to Weaviate, we work with a Contributor License Agreement. If you agree, you can simply add a comment to this PR that you agree with the CLA so that we can merge. |
This branch has not been deployed
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
3 participants
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.
Reusing one
Configure.VectorIndex.hnsw(...)object across severalConfigure.Vectors.*calls leaks the quantizer requested for one vector onto all of them:renders
pqon bothcompressedandraw, andtuneditself is mutated._IndexWrappers.singleand.multiassign the quantizer (and, for multi-vectors, the encoding) directly onto the caller's object, so every vector built from the same variable shares one pydantic instance. It is silent, and because a quantizer cannot be changed after creation (__check_quantizersrefuses), the collection has to be recreated. Same class of aliasing that #2143 fixed in_FilterBase._target_path.Both wrappers now deep-copy
vector_index_configbefore touching it;multi()also copiesmulti_vector_config, which had the identical problem withencoding. Four tests cover HNSW, dynamic (the quantizer fans out to bothhnswandflat), multi-vector, and a shared multi-vector config — each asserts the unquantized vector stays clean and the caller's object is unchanged. All fail onmain.test/collection/test_config.py211 passed; ruff, flake8 and pyright clean.