-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Cleanup around vector in Cosmos #35960
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR cleans up Cosmos vector and full-text search APIs by removing experimental flags and renaming vector-related API methods.
- Renames vector index configuration methods from ForVectors to IsVectorIndex and similarly updates full-text index test names.
- Removes #pragma warning disable/restore blocks related to experimental features and shifts some validation logic into container creation.
Reviewed Changes
Copilot reviewed 17 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
test/EFCore.Cosmos.Tests/Infrastructure/CosmosModelValidatorTest.cs | Updates test methods to use IsFullTextIndex/IsVectorIndex and removes unnecessary pragma disables. |
test/EFCore.Cosmos.FunctionalTests/VectorSearchCosmosTest.cs | Renames vector index configuration calls and removes pragma warnings. |
test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs | Removes pragma disables for experimental warnings during container creation. |
test/EFCore.Cosmos.FunctionalTests/HybridSearchCosmosTest.cs | Updates hybrid search tests to use new vector and full-text index method names. |
test/EFCore.Cosmos.FunctionalTests/AddHocVectorSearchCosmosTest.cs | Introduces ad hoc tests for composite vector index and vector property on collection navigation. |
test/EFCore.Cosmos.FunctionalTests/AddHocFullTextSearchCosmosTest.cs | Adjusts error messages for full text index validations and removes obsolete tests. |
src/Shared/EFDiagnostics.cs | Adds a new diagnostic constant for CosmosFullTextSearchExperimental. |
src/EFCore.Cosmos/Storage/Internal/CosmosVectorTypeMapping.cs | Removes the Experimental attribute from the CosmosVectorTypeMapping class. |
src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs | Refactors composite vector index validation and updates error messages. |
src/EFCore.Cosmos/Metadata/Internal/CosmosVectorType.cs | Removes the Experimental attribute from the CosmosVectorType record. |
src/EFCore.Cosmos/Metadata/Internal/CosmosAnnotationNames.cs | Removes the Experimental attribute from vector related annotations. |
src/EFCore.Cosmos/Infrastructure/Internal/CosmosModelValidator.cs | Adjusts vector and full text index validation logic and comment descriptions. |
src/EFCore.Cosmos/Extensions/* | Replaces Experimental attributes and renames vector index methods accordingly across property and index extension methods. |
src/EFCore.Cosmos/Extensions/CosmosDbFunctionsExtensions.cs | Removes Experimental attributes on vector distance functions. |
Files not reviewed (3)
- src/EFCore.Cosmos/EFCore.Cosmos.csproj: Language not supported
- src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs: Language not supported
- src/EFCore.Cosmos/Properties/CosmosStrings.resx: Language not supported
03c3d87
to
65759e1
Compare
src/EFCore.Cosmos/Infrastructure/Internal/CosmosModelValidator.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Infrastructure/Internal/CosmosModelValidator.cs
Outdated
Show resolved
Hide resolved
e75f2b1
to
8cc7136
Compare
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
8cc7136
to
be92392
Compare
new version up @roji |
- remove experimental from vector APIs, - renamed vector index model builder method from ForVector to IsVectorIndex, - renamed vector property model builder method from IsVector to IsVectorProperty, - use helper methods rather than extracting annotations directly in validation, - move some validation to container creation, so that queries are not blocked when Cosmos starts supporting new things (e.g. composite indexes) - split CosmosVectorType into two annotations (distance function and dimensions) so that we can keep CVT pubternal Fixes #35895 Fixes #35886 Fixes #35897 Fixes #35903 Fixes #35965
be92392
to
ecba8a8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @maumar, LGTM.
@AndriySvyryd may have more feedback to add but we can address that in a separate PR (I'm thinking possibly about the separate of vector dimensions and distance function). But I don't think there's anything blocking merging.
Fixes #35895
Fixes #35886
Fixes #35897
Fixes #35903
Fixes #35965