dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.75k stars 3.18k forks source link

Query: add translation for string.Compare for Cosmos #20910

Open maumar opened 4 years ago

maumar commented 4 years ago

both instance and static version

ajcvickers commented 4 years ago

@AndriySvyryd Can't find a duplicate that contains this.

AndriySvyryd commented 4 years ago

https://github.com/dotnet/efcore/issues/16143, but we can keep this one separate

yosoyhabacuc commented 3 years ago

Hi, I was thinking on translating this function by using string1>string2 comparison in Cosmos, as there is not Compare equivalent function, but we could get the same result by just using > and <. Therefore the basic comparison cold be translated, and also the overloads with starting positions. For overloads with case sensitiveness , we could use UPPER to ignore casing, although it could create create performance issues as it doesn't use the indexes (https://docs.microsoft.com/lv-lv/azure/cosmos-db/sql/sql-query-upper), Other overloads including CompareOptions, CultureInfo and StringComparisson would not be translated. What do you think of the proposal? @ajcvickers @maumar

roji commented 3 years ago

We could indeed translate .NET string.Compare(a, b) > 0 to a > b in Cosmos (and similar).

I agree that we should avoid translating overloads accepting StringComparison, if the only way to implement those would be to use UPPER, negating index use - we've discussed this thoroughly in the context of relational databases, and I think the logic here is the same. Note #25250, which discusses translating Contains, including case-insensitivity, which apparently can be done with Cosmos CONTAINS while maintaining index usage.

pebo commented 2 years ago

I'm working with an ODATA api over Cosmos DB and ran into this issue when trying to filter with gt/ltoperators on string attributes, e.g. $filter=modified_at gt '2022-09-01T00:00:00Z' and status lt '100'.

The error I get is:

System.InvalidOperationException: The LINQ expression 'DbSet<Entity>()
    .Where(s => string.Compare(
        strA: (string)s.modified_at, 
        strB: __TypedProperty_0) > 0)' could not be translated. Additional information: Translation of method 'string.Compare' failed. If this method can be mapped to your custom function, see https://go.microsoft.com/fwlink/?linkid=2132413 for more information. Either rewrite the query in a form that can be translated, or switch to client evaluation explicitly by inserting a call to 'AsEnumerable', 'AsAsyncEnumerable', 'ToList', or 'ToListAsync'. See https://go.microsoft.com/fwlink/?linkid=2101038 for more information.

Packages used:

    <PackageReference Include="Microsoft.AspNetCore.OData" Version="8.0.11" />
    <PackageReference Include="Microsoft.Azure.Cosmos" Version="3.30.1" />
    <PackageReference Include="Microsoft.EntityFrameworkCore.Cosmos" Version="6.0.9" />

Using 'ToList' and performing the filtering on the client side works but the should really be done in the database.

Is there a workaround for this issue?

roji commented 2 years ago

@pebo until this is implemented, you can a SQL query to write exactly the SQL you want.

pebo commented 2 years ago

@pebo until this is implemented, you can a SQL query to write exactly the SQL you want.

Thanks! So currently there's no way to translate a generic string.Compare(a, b) > 0 criteria into a > b in CosmosDb?

As a partial workaround, i.e. it only supports one predefined filter, I tested adding an ODATA function which can be called with /api/foos/GetModifiedSince(modified_since=2022-09-01). Tested using FromSqlRaw from the controller and this seems to work:

    [HttpGet("GetModifiedSince(modified_since={modified_since})")]
    public IActionResult GetModifiedSince(string modified_since)
    {
      var result = CosmosQueryableExtensions.FromSqlRaw(_dbContext.Foo, 
              "SELECT * FROM c WHERE c.modified_at > {0}", modified_since);
      return Ok(result);
    }
roji commented 2 years ago

Thanks! So currently there's no way to translate a generic string.Compare(a, b) > 0 criteria into a > b in CosmosDb?

No, that's what this issue is about.

maumar commented 2 years ago

Note: OData $skiptoken uses string.Compare when the token is string based, effectively blocking that feature on Cosmos

clazarr commented 1 year ago

Similarly, expressions with IComparable<T>.CompareTo(v) > 0 and its variations are not translated to Cosmos' comparison operators (>, <, >=, etc.). CompareTo is used in expressions to compare lots of different data types in .NET including string, boolean, Guid, and Enum among others listed in the docs. Note that CompareTo is correctly translated for other EF Core database providers such as SqlServer, Sqlite, InMemory (that I've tested).

The Cosmos DB .NET SDK for NoSQL includes translation of CompareTo in its LINQ provider. When will Entity Framework Core's Cosmos DB provider translate CompareTo?

Also note that a lack of support for CompareTo blocks some higher-level extensions of EF Core when used with the Cosmos DB provider including keyset paging where we construct custom where predicates to execute a query to retrieve the next page (without using offset paging/skip).

Are there any workarounds until this is supported in the query translation layer for the Cosmos DB provider? For example, would HasDbFunction/HasTranslation be able to workaround this?

roji commented 1 year ago

@clazarr thanks for point that out - yes, I think it makes sense to add CompareTo support to the Cosmos provider along with string.Compare.

Are there any workarounds until this is supported in the query translation layer for the Cosmos DB provider? For example, would HasDbFunction/HasTranslation be able to workaround this?

Yes, that should be possible.