Skip to content
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

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

Closed
smitpatel opened this issue Jun 18, 2019 · 6 comments · Fixed by #24247, #19563, #24203 or #24204
Closed

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

smitpatel opened this issue Jun 18, 2019 · 6 comments · Fixed by #24247, #19563, #24203 or #24204
Assignees
Labels
area-cosmos area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution good first issue This issue should be relatively straightforward to fix. type-enhancement
Milestone

Comments

@smitpatel
Copy link
Contributor

smitpatel commented Jun 18, 2019

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 Add translations for REPLICATE/REVERSE sql functions #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 (EFCore 3.1 + CosmosDb can't translate any() on owned object #20441) - no straightforward translation
  • ST_DISTANCE, ST_INTERSECTS, ST_ISVALID, ST_ISVALIDDETAILED, ST_WITHIN (Cosmos: Spatial types, functions and spatial indexes #17317) - blocked on spatial support for cosmos
@smitpatel smitpatel mentioned this issue Jun 18, 2019
82 tasks
@divega divega added this to the Backlog milestone Jun 20, 2019
@AndriySvyryd AndriySvyryd changed the title Cosmos: Add translators for member/methods which maps to built in functions Cosmos: Add translators for member/methods which map to built-in functions Aug 21, 2019
jviau added a commit to jviau/efcore that referenced this issue Jan 10, 2020
Summary of changes
- Adds initial StringMethodTranslator for Cosmos provider.
- Translates Contains, StartsWith, and EndsWith

Addresses part of dotnet#16143
AndriySvyryd pushed a commit that referenced this issue Jan 11, 2020
Summary of changes
- Adds initial StringMethodTranslator for Cosmos provider.
- Translates Contains, StartsWith, and EndsWith

Addresses part of #16143
svengeance pushed a commit to svengeance/EntityFrameworkCore that referenced this issue Jan 15, 2020
…9563)

Summary of changes
- Adds initial StringMethodTranslator for Cosmos provider.
- Translates Contains, StartsWith, and EndsWith

Addresses part of dotnet#16143
@braidenstiller
Copy link

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

@bricelam
Copy link
Contributor

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.

@Marusyk
Copy link
Member

Marusyk commented Aug 10, 2021

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

@maumar
Copy link
Contributor

maumar commented Aug 10, 2021

@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
Copy link
Member

roji commented Aug 11, 2021

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)

@maumar maumar modified the milestones: 6.0.0, 6.0.0-rc1 Aug 18, 2021
@maumar maumar closed this as completed Aug 18, 2021
@maumar maumar added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Aug 18, 2021
@ajcvickers ajcvickers modified the milestones: 6.0.0-rc1, 6.0.0 Nov 8, 2021
@markjbrown
Copy link

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment