Fix inference issues around convert to object #21125
Conversation
There was a problem hiding this comment.
If this is fixing cases like
(Convert(b.Name, Object) == null) &
(object)c.City == (object)"London"
Then it should normalize it during SqlTranslator.VisitBinary method by removing object convert from both sides (or from one side if other side is null).
Also apply same thing to Cosmos.
src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
| @@ -884,37 +947,6 @@ private SqlExpression BindProperty(EntityReferenceExpression entityReferenceExpr | |||
| throw new InvalidOperationException(CoreStrings.TranslationFailed(methodCallExpression.Print())); | |||
| } | |||
|
|
|||
| private static Expression TryRemoveImplicitConvert(Expression expression) | |||
There was a problem hiding this comment.
Any reason to move this to local function? This is not specific to BinaryExpression.
There was a problem hiding this comment.
Wasn't used anywhere else so I did it... We can always move it out if it's needed no?
There was a problem hiding this comment.
Then we need to remember that we had method like this hiding here.
Local methods which are valid in a context of a particular method makes sense but not used elsewhere hence local could be bit non-productive.
cc: @dotnet/efcore ?
There was a problem hiding this comment.
No need to bother everyone, I'll just move it out if that's what you want.
There was a problem hiding this comment.
Found a usecase, there RemoveObjectConvert in VisitMethodCall too. You can DRY with TryUnwrapConvertToObject
There was a problem hiding this comment.
In VisitBinary we need to know if we unwrapped or not (hence Try*), but in VisitMethodCall we don't (so it returns the unwrapped)... So DRYing would make VisitMethodCall clunkier (calling Try* and testing the result). Let's leave things the way they are, the code fragment is really small...
However I did DRY out RemoveObjectConvert in Relational which is used by both VisitMethodCall and ConvertObjectArrayEqualityComparison.
Fixes #19688