-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Add Math translators for Cosmos DB provider #24463
Conversation
@maumar I'm poaching this. |
@bricelam sure, I will add |
@maumar Looks like this still needs your expertise to answer some design questions. |
I will add tests after merge of #18842 |
@Marusyk - For now disable tests which are running into issue of convert node. Looking at the CosmosDb docs, seems like they only have double kind of numeric values. This would make any convert to double node in LINQ for numeric type no-op and translatable but I need to research more on this in order to update conversions. |
src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
@Marusyk - 1 test is failing in SQL assertion (perhaps not getting translated now), can you also disable it and update the PR? |
@smitpatel I've fixed it. Thanks |
@Marusyk - Thank you for contribution. |
Added Math for Cosmos:
ABS, CEILING, FLOOR, POWER, EXP, LOG10, LOG, SQRT, ACOS, ASIN, ATAN, ATN2, COS, SIN, TAN, SIGN, TRUNC, ROUND
Issue: #16143
Need your help with translation methods with converting.
Example:
here
od.Discount
isfloat
.Math.Tan
requiresdouble
. So, the following expression will be generated:In this case the
TryRemoveImplicitConvert
will be called and returnnull
https://github.com/Marusyk/efcore/blob/e23cdc0128851a8760bbcd57696f4001ae3b3a60/src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs#L685
Could you please suggest how to improve that?