Cosmos: translate string Contains, StartsWith, and EndsWith #19563

jviau · 2020-01-10T22:28:39Z

Addresses part of #16143

This PR introduces translation for string.StartsWith, string.EndsWith, and string.Contains in cosmos queries. They are converted to the built-in cosmos functions STARTSWITH, ENDSWITH, and CONTAINS respectively.

Not sure if this contribution is wanted, but I had added this extension to my code as I was playing around with efcore cosmos and figured I would offer to contribute it back. If it is wanted, I will add tests and make changes as requested.

Summary of changes - Adds initial StringMethodTranslator for Cosmos provider. - Translates Contains, StartsWith, and EndsWith Addresses part of dotnet#16143

src/EFCore.Cosmos/Query/Internal/StringMethodTranslator.cs

AndriySvyryd

There are already tests for this in NorthwindFunctionsQueryCosmosTest, look for "String_Contains", "String_StartsWith", "String_EndsWith". Unskip them and correct the expected SQL.

- Remove infer type mapping from cosmos StringMethodTranslator

- Unskips and improves cosmos functional tests for startswith, endswith, and contains - Adds SqlFunctionExpression support to ApplyDefaultTypeMapping

jviau · 2020-01-11T01:09:27Z

@AndriySvyryd , I unskipped those tests and updated them. I also found that I could not have a null TypeMapping, as that would throw in the tests. I used the ApplyDefaultTypeMapping as you suggested (after adding support for SqlFunctionExpression to it).

test/EFCore.Cosmos.FunctionalTests/Query/NorthwindFunctionsQueryCosmosTest.cs

smitpatel · 2020-01-11T02:07:59Z

src/EFCore.Cosmos/Query/Internal/StringMethodTranslator.cs

+        private SqlExpression TranslateSystemFunction(string function, SqlExpression instance, SqlExpression pattern, Type returnType)
+        {
+            Check.NotNull(instance, nameof(instance));
+            return _sqlExpressionFactory.ApplyDefaultTypeMapping(


ApplyDefaultTypeMapping shouldn't be required. If it does not apply default at later stage automatically then there is bug somewhere.

I see. Can a different issue be opened for that? It may be a test validation issue - it is an error along the lines of "SqlExpression without TypeMapping found in query" and the test fails.

Filed #19567

test/EFCore.Cosmos.FunctionalTests/Query/NorthwindFunctionsQueryCosmosTest.cs

- Inline cosmos string method test variables - Remove fact attributes, they are inherited

AndriySvyryd · 2020-01-11T21:11:16Z

Thanks for your contribution!

…9563) Summary of changes - Adds initial StringMethodTranslator for Cosmos provider. - Translates Contains, StartsWith, and EndsWith Addresses part of dotnet#16143

nickwilsonr · 2020-01-21T17:50:46Z

So when can we expect this to be released to Nuget?

AndriySvyryd · 2020-01-21T17:59:27Z

@nickwilsonr EF Core 5.0 is aligned with .NET 5.0 set to release in November

nickwilsonr · 2020-01-21T18:02:39Z

@nickwilsonr EF Core 5.0 is aligned with .NET 5.0 set to release in November

So this fix cant/wont be included in an upcoming 3.x release? 10 months is a VERY LONG time to wait for something that is pretty basic EF functionality.

AndriySvyryd · 2020-01-21T18:14:48Z

@nickwilsonr Unfortunately there will be no 3.x releases apart from 3.0.x and 3.1.x

nickwilsonr · 2020-01-21T19:42:58Z

@AndriySvyryd, this seems like a pretty big problem.

What are we supposed to do here for the next 10 months? For small projects it may be okay to simply pull the entire collection into memory to do these string comparisons, but that's not going to scale. I'm looking for an enterprise-level solution here, which is kinda the whole point of Cosmos.

AndriySvyryd · 2020-01-21T21:03:34Z

@nickwilsonr You can use daily builds. There are few changes to Cosmos for 5.0 so these should be pretty stable, the downside is that you might need to also upgrade other packages that have common dependencies.

jviau · 2020-01-21T21:12:05Z

@nickwilsonr, if you want to, you can also add this functionality yourself. It does use some internal API's though. I uploaded some sample code here for you: https://github.com/jviau/samples/tree/master/src/efcore. This supports more than what I included in this PR 😄 , but I haven't tested it all.

ajcvickers · 2020-01-21T23:57:09Z

@nickwilsonr We will certainly pass on your feedback to those who handle release schedules. Beyond that we strive to keep a high quality in previews and daily builds so that people can use those builds while waiting for RTM releases. (3.0 was an exception to this due to the query overhaul.)

nickwilsonr · 2020-01-30T00:42:14Z

@jviau
Just gave your code a try, but I'm getting this exception when I try to run my query:
Null TypeMapping in Sql Tree

If I put a breakpoint in CosmosStringMethodCallTranslator on line 90, then arguments[0] (which is the RHS of the CONTAINS operation) does have a null TypeMapping, but I have no idea why or how to resolve this.

The query I'm using for testing is VERY simple:
context.Transactions.Where(t => t.Content.Contains("12345"))

For point of reference, Transactions is the DBSet for the Transaction model, which contains a string property called Content

AndriySvyryd · 2020-01-30T01:48:43Z

@nickwilsonr Can you file this as a new issue?

jviau · 2020-01-30T01:51:36Z

I believe an issue for this is already open. I encountered it during this PR, but we assumed it was a test only issue. It is probably a validation issue then as cosmos provider does work fine when that value is set to even an incorrect value. So long as it's not null.

AndriySvyryd · 2020-01-30T01:59:00Z

@jviau You mean #19567? That one is about addressing the root cause, but current code should use ApplyDefaultTypeMapping to avoid this exception.

@nickwilsonr I think you are referring to the code from https://github.com/jviau/samples/tree/master/src/efcore. If so file an issue on that repo.

jviau · 2020-01-30T02:36:36Z

Should be fixed in the sample now: jviau/samples#1

Translate string Contains, StartsWith, and EndsWith

b58b093

Summary of changes - Adds initial StringMethodTranslator for Cosmos provider. - Translates Contains, StartsWith, and EndsWith Addresses part of dotnet#16143

AndriySvyryd reviewed Jan 10, 2020

View reviewed changes

src/EFCore.Cosmos/Query/Internal/StringMethodTranslator.cs Outdated Show resolved Hide resolved

AndriySvyryd requested changes Jan 10, 2020

View reviewed changes

jviau added 2 commits January 10, 2020 15:46

Correct type mapping

01d5039

- Remove infer type mapping from cosmos StringMethodTranslator

Unskip some cosmos string unit tests

a677c7e

- Unskips and improves cosmos functional tests for startswith, endswith, and contains - Adds SqlFunctionExpression support to ApplyDefaultTypeMapping

AndriySvyryd changed the title ~~[WIP] Cosmos: translate string Contains, StartsWith, and EndsWith~~ Cosmos: translate string Contains, StartsWith, and EndsWith Jan 11, 2020

AndriySvyryd reviewed Jan 11, 2020

View reviewed changes

test/EFCore.Cosmos.FunctionalTests/Query/NorthwindFunctionsQueryCosmosTest.cs Outdated Show resolved Hide resolved

smitpatel reviewed Jan 11, 2020

View reviewed changes

test/EFCore.Cosmos.FunctionalTests/Query/NorthwindFunctionsQueryCosmosTest.cs Outdated Show resolved Hide resolved

Inline cosmos string method test variables

ade0907

- Inline cosmos string method test variables - Remove fact attributes, they are inherited

AndriySvyryd approved these changes Jan 11, 2020

View reviewed changes

AndriySvyryd merged commit 83dccdc into dotnet:master Jan 11, 2020

jviau deleted the cosmos-string-translator branch January 11, 2020 08:38

ajcvickers mentioned this pull request Jan 16, 2020

Entity Framework Weekly Status Updates (2020) #19549

Closed

nickwilsonr mentioned this pull request Jan 21, 2020

CosmosDB - Entity Framework Core - Contains could not be translated #18673

Closed

ajcvickers mentioned this pull request Feb 4, 2020

Cosmos: translate string Contains, StartsWith, and EndsWith #19800

Closed

ajcvickers linked an issue Feb 4, 2020 that may be closed by this pull request

Cosmos: translate string Contains, StartsWith, and EndsWith #19800

Closed

roji linked an issue Feb 25, 2021 that may be closed by this pull request

Cosmos: Add translators for member/methods which map to built-in functions #16143

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cosmos: translate string Contains, StartsWith, and EndsWith #19563

Cosmos: translate string Contains, StartsWith, and EndsWith #19563

jviau commented Jan 10, 2020

AndriySvyryd left a comment

jviau commented Jan 11, 2020

smitpatel Jan 11, 2020

jviau Jan 11, 2020

AndriySvyryd Jan 11, 2020

AndriySvyryd commented Jan 11, 2020

nickwilsonr commented Jan 21, 2020

AndriySvyryd commented Jan 21, 2020

nickwilsonr commented Jan 21, 2020

AndriySvyryd commented Jan 21, 2020

nickwilsonr commented Jan 21, 2020

AndriySvyryd commented Jan 21, 2020

jviau commented Jan 21, 2020 •

edited

Loading

ajcvickers commented Jan 21, 2020

nickwilsonr commented Jan 30, 2020

AndriySvyryd commented Jan 30, 2020

jviau commented Jan 30, 2020

AndriySvyryd commented Jan 30, 2020

jviau commented Jan 30, 2020

Cosmos: translate string Contains, StartsWith, and EndsWith #19563

Cosmos: translate string Contains, StartsWith, and EndsWith #19563

Conversation

jviau commented Jan 10, 2020

AndriySvyryd left a comment

Choose a reason for hiding this comment

jviau commented Jan 11, 2020

smitpatel Jan 11, 2020

Choose a reason for hiding this comment

jviau Jan 11, 2020

Choose a reason for hiding this comment

AndriySvyryd Jan 11, 2020

Choose a reason for hiding this comment

AndriySvyryd commented Jan 11, 2020

nickwilsonr commented Jan 21, 2020

AndriySvyryd commented Jan 21, 2020

nickwilsonr commented Jan 21, 2020

AndriySvyryd commented Jan 21, 2020

nickwilsonr commented Jan 21, 2020

AndriySvyryd commented Jan 21, 2020

jviau commented Jan 21, 2020 • edited Loading

ajcvickers commented Jan 21, 2020

nickwilsonr commented Jan 30, 2020

AndriySvyryd commented Jan 30, 2020

jviau commented Jan 30, 2020

AndriySvyryd commented Jan 30, 2020

jviau commented Jan 30, 2020

jviau commented Jan 21, 2020 •

edited

Loading