-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Cosmos Full Text Search support #35868
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
src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Internal/Expressions/SqlFunctionExpression.cs
Outdated
Show resolved
Hide resolved
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 adds support for full-text search in Cosmos by introducing new full-text index types, translators, model validations, and extensions.
- Introduces full-text indexing support in container creation and client wrappers.
- Adds a new CosmosFullTextSearchTranslator and corresponding DbFunctions extensions.
- Updates model validation and Fluent API extensions to include full-text search configuration.
Reviewed Changes
Copilot reviewed 23 out of 26 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs | Adds support for full-text indexing in container creation, but the new full-text index assignment may override vector index settings. |
src/EFCore.Cosmos/Query/Internal/Translators/CosmosVectorSearchTranslator.cs | Modifies method checking logic that may prematurely reject valid method calls by changing && to |
src/EFCore.Cosmos/Query/Internal/Translators/CosmosFullTextSearchTranslator.cs | Implements a new translator for full-text search functions. |
src/EFCore.Cosmos/Query/Internal/Expressions/SqlFunctionExpression.cs | Introduces an extra constructor parameter to flag scoring functions. |
src/EFCore.Cosmos/Query/Internal/Expressions/SelectExpression.cs | Adds a RankOrdering property and methods to apply rank-based ordering. |
src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs | Adjusts ordering logic to support scoring functions and rank ordering. |
src/EFCore.Cosmos/Query/Internal/CosmosQuerySqlGenerator.cs | Updates SQL generation to include rank ordering handling. |
src/EFCore.Cosmos/Metadata/Internal/CosmosAnnotationNames.cs | Defines new annotations for full-text search configuration. |
src/EFCore.Cosmos/Infrastructure/Internal/CosmosModelValidator.cs | Validates full-text index configuration, ensuring only single-property indexes with language annotation are defined. |
src/EFCore.Cosmos/Extensions/CosmosPropertyExtensions.cs, CosmosPropertyBuilderExtensions.cs, CosmosIndexExtensions.cs, CosmosIndexBuilderExtensions.cs, CosmosDbFunctionsExtensions.cs | Adds Fluent API and extension methods for configuring full-text search on properties and indexes. |
Files not reviewed (3)
- Directory.Packages.props: Language not supported
- src/EFCore.Cosmos/EFCore.Cosmos.csproj: Language not supported
- src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs: Language not supported
Comments suppressed due to low confidence (2)
src/EFCore.Cosmos/Query/Internal/Translators/CosmosVectorSearchTranslator.cs:33
- Changing the logical operator from '&&' to '||' alters the intended logic and causes the translator to return null for methods declared on CosmosDbFunctionsExtensions that are not named 'VectorDistance'. Revisit the condition to ensure it only returns null when both conditions are unmet.
if (method.DeclaringType != typeof(CosmosDbFunctionsExtensions) || method.Name != nameof(CosmosDbFunctionsExtensions.VectorDistance))
src/EFCore.Cosmos/Extensions/CosmosPropertyExtensions.cs:137
- [nitpick] The Experimental attribute for full-text search methods is using 'CosmosVectorSearchExperimental' instead of the more appropriate 'CosmosFullTextSearchExperimental'. This may cause confusion regarding the feature's experiment status.
[Experimental(EFDiagnostics.CosmosVectorSearchExperimental)]
0f39e44
to
a836b63
Compare
src/EFCore.Cosmos/Query/Internal/Expressions/RankOrderingExpression.cs
Outdated
Show resolved
Hide resolved
a836b63
to
898cc5f
Compare
@maumar just to note that I'm holding back from reviewing since this is still in draft (you seemed to maybe want people to take a look in yesterday's meeting). Let me know if you want a review etc. |
898cc5f
to
cee74fd
Compare
@roji feel free to hold off on reviewing a draft. I'm pretty close to marking it for review I think |
a45f839
to
245066a
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.
Ended up doing a quick review even though the PR is still in draft... Here are some comments.
src/EFCore.Cosmos/Infrastructure/Internal/CosmosModelValidator.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
.../CosmosShapedQueryCompilingExpressionVisitor.InExpressionValuesExpandingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Internal/Expressions/SelectExpression.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Internal/Translators/CosmosFullTextSearchTranslator.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Internal/Translators/CosmosFullTextSearchTranslator.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Internal/Translators/CosmosFullTextSearchTranslator.cs
Outdated
Show resolved
Hide resolved
73b0297
to
30e0bd9
Compare
30e0bd9
to
ebebcfc
Compare
src/EFCore.Cosmos/Query/Internal/Translators/CosmosFullTextSearchTranslator.cs
Outdated
Show resolved
Hide resolved
ebebcfc
to
68ca715
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.
/cc @AndriySvyryd in case you want to take a quick look for the metadata side and Cosmos container building part.
src/EFCore.Cosmos/Query/Internal/Expressions/SelectExpression.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Internal/Expressions/SelectExpression.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Internal/Expressions/SelectExpression.cs
Outdated
Show resolved
Hide resolved
.../CosmosShapedQueryCompilingExpressionVisitor.InExpressionValuesExpandingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Internal/Translators/CosmosFullTextSearchTranslator.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Internal/Translators/CosmosFullTextSearchTranslator.cs
Outdated
Show resolved
Hide resolved
test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs
Outdated
Show resolved
Hide resolved
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.
@maumar see batch of comments.
I think we should maybe discuss the FTS property vs. index question in design...
src/EFCore.Cosmos/Extensions/CosmosPropertyBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
.../CosmosShapedQueryCompilingExpressionVisitor.InExpressionValuesExpandingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
.../CosmosShapedQueryCompilingExpressionVisitor.InExpressionValuesExpandingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
.../CosmosShapedQueryCompilingExpressionVisitor.InExpressionValuesExpandingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Internal/Translators/CosmosFullTextSearchTranslator.cs
Show resolved
Hide resolved
64a2808
to
0933839
Compare
Fixes #35476 Fixes #35853 (need to fix this one, otherwise vector translator will try to translate full text methods and fail) Description This PR enables full-text search queries using EF Core 9 when targeting Azure Cosmos Db. This is one of the flagship new features for Cosmos and the aim here is to help with it's adoption. This is very limited port of the full feature we are adding in EF 10 Preview 4. We are only adding querying capabilities: function stubs for FullTextContains, FullTextContainsAll, FullTextContainsAny, FullTextScore and RRF as well as logic translating these signatures to built-in Cosmos functions. No model building or data manipulation - containers need to be created outside EF (using Cosmos SDK or in the Data Explorer). Customer impact Customers will be able to use the upcoming full text search capabilities when working with Azure Cosmos Db without the need to upgrade to EF 10 preview. How found Partner team ask. Regression No Testing Extensively tested on EF 10, manual testing on EF9. End-to-end testing is not possible because we can't create containers programmatically (no support for it inside EF Core itself, and the Cosmos SDK which supports it is currently only available in beta, so we can't take dependency on it). Instead, we created containers and data using EF 10, ported all the query tests from EF 10 and ran them using the EF9 bits. Risk Low. Code here is purely additive and actually localized to only a handful of places in the code: validation in ApplyOrdering/AppendOrdering, FTS method translator, parameter inliner and sql generator. Feature is marked as experimental and quirks have been added.
d9fa4f3
to
b554e29
Compare
@roji @AndriySvyryd I updated the PR addressing the Property/Index API and fixing a few small errors. You can review just the second commit - all the changes are there.
|
b554e29
to
65cc1e5
Compare
src/EFCore.Cosmos/Extensions/CosmosPropertyBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Infrastructure/Internal/CosmosModelValidator.cs
Outdated
Show resolved
Hide resolved
8d8771f
to
993bc56
Compare
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
5c2c1ea
to
49bedd3
Compare
…35909) * Limited port of #35868 Fixes #35476 Fixes #35853 (need to fix this one, otherwise vector translator will try to translate full text methods and fail) Description This PR enables full-text search queries using EF Core 9 when targeting Azure Cosmos Db. This is one of the flagship new features for Cosmos and the aim here is to help with it's adoption. This is very limited port of the full feature we are adding in EF 10 Preview 4. We are only adding querying capabilities: function stubs for FullTextContains, FullTextContainsAll, FullTextContainsAny, FullTextScore and RRF as well as logic translating these signatures to built-in Cosmos functions. No model building or data manipulation - containers need to be created outside EF (using Cosmos SDK or in the Data Explorer). Customer impact Customers will be able to use the upcoming full text search capabilities when working with Azure Cosmos Db without the need to upgrade to EF 10 preview. How found Partner team ask. Regression No Testing Extensively tested on EF 10, manual testing on EF9. End-to-end testing is not possible because we can't create containers programmatically (no support for it inside EF Core itself, and the Cosmos SDK which supports it is currently only available in beta, so we can't take dependency on it). Instead, we created containers and data using EF 10, ported all the query tests from EF 10 and ran them using the EF9 bits. Risk Low. Code here is purely additive and actually localized to only a handful of places in the code: validation in ApplyOrdering/AppendOrdering, FTS method translator, parameter inliner and sql generator. Feature is marked as experimental and quirks have been added. * fix FTContainsAll/Any for constants, change TFScore signature to accept params, fixed SqlFunctionExpression to use the new ctor internally, added mandatory property overrides for FragmentExpression (vector search) fixed parameter inliner to not match parameter of type string, but only string[] when processing ContainsAll/Any
src/EFCore.Cosmos/Extensions/CosmosPropertyBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
49bedd3
to
31e670c
Compare
- Adding model building API to configure property as full-text search enabled, as well as setup the index for it, - Adding model validation (e.g. FTS index not matching FTS property), - Adding EF.Functions stubs and translations for FullTextContains, FullTextContainsAll, FullTextContainsAny, FullTextScore and RRF (for hybrid), - Adding logic in SelectExpression to produce ORDER BY RANK when necessary, - Adding validation when attempting to mix with ORDER BY RANK with regular ORDER BY, - Rewrite OFFSET/LIMIT from parameter to constant when ORDER BY RANK is present. - Adding model building support for default language on the container level Also fixed / added support for vector search on owned types (since it shares logic with FTS) and added some tests. outstanding work: - support for FTS Container building using Azure.ResourceManager.CosmosDb (currently blocked on updated package being released) Fixes #35476 Fixes #35853 Fixes #35867 Fixes #35852
31e670c
to
b987e7f
Compare
Also fixed / added support for vector search on owned types (since it shares logic with FTS) and added some tests.
outstanding work:
Fixes #35476
Fixes #35851
Fixes #35852
Fixes #35853
Fixes #35867