feat(vamana): Add relabelVector, a data-preserving external-ID rename - #383
Open
nonirosenfeldredis wants to merge 1 commit into
Open
feat(vamana): Add relabelVector, a data-preserving external-ID rename#383nonirosenfeldredis wants to merge 1 commit into
nonirosenfeldredis wants to merge 1 commit into
Conversation
MutableVamanaIndex::delete_entries + add_points is the only existing way to change a vector's external label -- and that path re-reads and re-inserts the vector's data, and re-links its graph adjacency, even when the caller only wants a new label for data that hasn't changed. That's the situation a downstream index-update path faces after a document update turns out not to have touched the vector: nothing about the stored vector or its graph position needs to change, only the external ID it's known by. Adds IDTranslator::remap_external_id(from, to), the missing symmetric counterpart to the existing remap_internal_id (used internally by compact()): a pure O(1) hash-map key swap on the external ID, validated the same way insert/delete_* are (checked by default, throws ANNException and leaves the table unmodified on a bad from/to, matching every other public mutator on this class). MutableVamanaIndex::relabelVector(old_id, new_id) exposes this at the index level, alongside the existing translate_external_id/has_id/get_datum methods it's meant to be used with. It does not touch the dataset or graph at all. Tested: a new "Rename External" section (plus two error-path sections) in tests/svs/core/translation.cpp, and a new "MutableVamana Index Relabel" test case in tests/svs/index/vamana/dynamic_index.cpp verifying the internal id, stored data, and every other id are unaffected by the rename. Verified both error-path sections actually exercise the check (not just happen to pass) by temporarily disabling it and confirming all four throw-assertions fail as expected, then restoring it.
nonirosenfeldredis
requested review from
ahuber21,
ibhati,
mihaic and
yuejiaointel
as code owners
September 7, 2026 05:35
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.
Summary
MutableVamanaIndex::delete_entries+add_pointswas the only existing way to change avector's external label -- and that path re-reads and re-inserts the vector's data, and
re-links its graph adjacency, even when the caller only wants a new label for data that
hasn't changed. That's exactly the situation a downstream index-update path faces after a
document update turns out not to have touched the vector: nothing about the stored vector or
its graph position needs to change, only the external ID it's known by.
IDTranslator::remap_external_id(from, to): the missing symmetric counterpart to theexisting
remap_internal_id(used internally bycompact()) -- a pure O(1) hash-map keyswap on the external ID, checked by default the same way
insert/delete_*are.MutableVamanaIndex::relabelVector(old_id, new_id): exposes this at the index level,alongside the existing
translate_external_id/has_id/get_datummethods it's meant to beused with. Touches neither the dataset nor the graph.
Testing
tests/svs/core/translation.cpp.tests/svs/index/vamana/dynamic_index.cpp,verifying the internal id, stored data, and every other id are unaffected by the rename.
temporarily disabling it and confirming all four throw-assertions fail as expected, then
restoring it.