dotnet/efcore

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

smitpatel opened this issue ยท 6 comments

See https://docs.microsoft.com/en-us/azure/cosmos-db/sql-query-system-functions, https://docs.microsoft.com/en-us/azure/cosmos-db/sql-query-linq-to-sql

Done:

  • LENGTH, CONTAINS, ENDSWITH, LOWER, LTRIM, RTRIM, STARTSWITH, TRIM, UPPER, SUBSTRING, CONCAT (using + operator), INDEX_OF, REPLACE, case-insensitive versions of STRINGEQUAL
  • ABS, ACOS, ASIN, ATAN, ATN2, CEILING, COS, EXP, FLOOR, LOG, LOG10, POWER, ROUND, SIGN, SIN, SQRT, TAN, TRUNC, RAND,
  • GetCurrentDateTime

Questionable:

  • COT - no direct Math function, equivalent to 1/TAN, should we recognize the pattern?
  • DEGREES, RADIANS - no direct Math function. we could recognize the mathematical pattern, e.g r = (Math.PI/180) * d, but it seems too complicated. Dedicated EF.Functions seems the most reasonable.
  • SQUARE - no direct Math function, we already translate Math.Pow. We could recognize the pattern Math.Pow(x, 2) but it would only work for constant. Probably better to create a decicated EF.Function, if anything at all.
  • PI - impossible to encounter this in expression tree because its const
  • REPLICATE, REVERSE - complex translation, probably better suited for EF.Functions translation (see #25470)
  • StringToArray, StringToBoolean, StringToNull, StringToNumber, StringToObject - EF.Functions? (StringToBoolean could be mapped to Convert though)
  • ToString - we could do it, but there are differences between c# and cosmos output, potential can of worms
  • RIGHT, LEFT - maps cleanly only to VB method, no clear c# counterpart. LEFT is similar although we do translate some Substring scenarios into it
  • GetCurrentTimestamp - could correspond to new DateTimeOffset(DateTime.UtcNow)..ToUnixTimeMilliseconds() but that seems to complicated and not very discoverable. EF.Functions seems like a better option.
  • IS_DEFINED, IS_OBJECT, IS_PRIMITIVE - doesn't map directly to bcl
  • IS_ARRAY - could lead to discrepancies (collection in the model but array on the database)
  • IS_BOOL, IS_STRING, IS_NUMBER - marginal utility, everything is already strongly typed
  • IS_NULL - should we map to this instead of x = null ?
  • ARRAY_CONCAT, ARRAY_CONTAINS, ARRAY_LENGTH, ARRAY_SLICE (#20441) - no straightforward translation
  • ST_DISTANCE, ST_INTERSECTS, ST_ISVALID, ST_ISVALIDDETAILED, ST_WITHIN (#17317) - blocked on spatial support for cosmos

Didn't see it in the list but are there plans to also map the case insensitive versions of Contains, EndsWith, and StartsWith?

While writing dotnet/EntityFramework.Docs#3307, the absence of string.Length stuck out to me a lot. Especially since we use LENGTH in the translation of Substring.

Can anyone suggest, functions like StringToArray, StringToBoolean, StringToNull, StringToNumber should be implemented as EF.Functions?

@Marusyk StringToBool could arguably be mapped to Convert.ToBoolean(string), although there are some differences, for others I think it makes sense to make them EF.Functions. (probably makes sense for StringToBool also, for consistency)

roji commented

StringToNumber could also correspond to int.Parse?

Agree with @maumar that it's probably good to have them on EF.Functions for consistency, but if it makes sense we could do both (having native .NET constructs just work is very helpful for discoverability)

@Marusyk thanks for your contributing to the Cosmos DB provider. I'd like to send you a Cosmos DB drink coaster and a sticker. If you could DM me on Twitter @markjbrown and send me your address, I'll get those shipped out to you. I'll also DM you as well in case you don't see this comment.

Thanks again.