fix(milvus): Fix upsert operations when autoId is false #8501
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.
Description
This PR fixes a bug in the Milvus vector store implementation where upsert operations would fail when
autoId
is set tofalse
. The issue was that the primary field was being incorrectly removed from the fields array during collection initialization, causing documents to be added instead of updated during upsert operations.Root Cause
The bug was in the
grabCollectionFields()
method where fields withautoID: true
were being removed fromthis.fields
regardless of the user'sautoId
setting. This caused the primary field to be missing during document processing in theaddVectors()
method, making upsert operations fail.Solution
Modified the field removal logic in
grabCollectionFields()
to only remove autoID fields when both conditions are true:field.autoID
is true (database schema property)this.autoId
is true (user's preference)This ensures that when
autoId: false
, the primary field remains inthis.fields
and gets processed correctly for upsert operations.Testing
Added comprehensive unit tests covering:
autoId
settingNote for reviewers: The unit tests will show harmless error logs during execution due to the MilvusClient constructor attempting connections during instantiation. This is expected behavior and doesn't affect test validity. The tests use a
TestMilvus
class that extends the main class to expose internal methods for testing without requiring network connections. Consider migrating to Vitest for better ESM mocking capabilities in the future.Fixes #8495