"Argument types do not match" when using ternary operator on Guids in Select statement

[isaacdontjelindell]

Issue and Code

I'm pretty sure this is a similar issue to #23309 and #23556, but that's marked as fixed for 5.0.1, which we're already on. Similar to #23556, this came up right after we upgraded to EF Core 5 (from 3.1.3 -> 5.0.1)

Basically, this line in a select statement causes "Argument types do not match" to be thrown when .ToList() is called on the query:

select new ClaimViewModel
{
    ...
    ProviderID = supervisor != null ? supervisor.SupervisorID : provider.ID,
    ...
}

This is a significantly-simplified version of the whole query:

var ourQuery = from e in _encountersContext

    let provider = e.ScheduledEvent.CounselorID == null ? null : e.ScheduledEvent.Counselor
    let supervisor = e.ScheduledEvent.Counselor.Supervisors.FirstOrDefault(y => y.Billable && y.StartDate.Date <= e.ScheduledEvent.EventDate.Date && (y.EndDate == null || y.EndDate.Value.Date >= e.ScheduledEvent.EventDate.Date))

    where
        ...
        && e.ScheduledEvent.CounselorID != null
        ...

    select new ClaimViewModel
    {
        ...
        ProviderID = supervisor != null ? supervisor.SupervisorID : provider.ID,
        ...
    };

var ourList = ourQuery.ToList(); // Exception is thrown here

The ProviderID property of ClaimViewModel is a non-nullable Guid: public Guid ProviderID { get; set; } . Both supervisor.SupervisorID and provider.ID are also non-nullable Guids.

In all cases, provider.ID will be set, guaranteed by the null check in the where clause.

Workaround

I can get everything to work properly by doing this in the select:

ProviderID = supervisor != null ? new Guid(supervisor.SupervisorID.ToString()) : new Guid(provider.ID.ToString()),

However, I don't think that should be necessary. I figured it out by guessing based on the workaround found in #23556.

Stack trace

System.ArgumentException: Argument types do not match
   at System.Linq.Expressions.Expression.Condition(Expression test, Expression ifTrue, Expression ifFalse, Type type)
   at System.Linq.Expressions.ExpressionVisitor.VisitMemberAssignment(MemberAssignment node)
   at System.Linq.Expressions.ExpressionVisitor.VisitMemberBinding(MemberBinding node)
   at System.Linq.Expressions.ExpressionVisitor.Visit[T](ReadOnlyCollection`1 nodes, Func`2 elementVisitor)
   at System.Linq.Expressions.ExpressionVisitor.VisitMemberInit(MemberInitExpression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitLambda[T](Expression`1 node)
   at System.Linq.Expressions.ExpressionVisitor.VisitUnary(UnaryExpression node)
   at System.Dynamic.Utils.ExpressionVisitorUtils.VisitArguments(ExpressionVisitor visitor, IArgumentProvider nodes)
   at System.Linq.Expressions.ExpressionVisitor.VisitMethodCall(MethodCallExpression node)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryOptimizingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at Microsoft.EntityFrameworkCore.Query.QueryTranslationPreprocessor.Process(Expression query)
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryTranslationPreprocessor.Process(Expression query)
   at Microsoft.EntityFrameworkCore.Query.QueryCompilationContext.CreateQueryExecutor[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Storage.Database.CompileQuery[TResult](Expression query, Boolean async)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileQueryCore[TResult](IDatabase database, Expression query, IModel model, Boolean async)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass9_0`1.<Execute>b__0()
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.Execute[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.Execute[TResult](Expression expression)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1.GetEnumerator()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at blpOps.Web.DataServices.ClaimsDataService.FindPotentialClaims(ClaimsSearchRequestViewModel claimsSearchRequestViewModel) in C:\Projects\BLPOpsPlatform\blpops-portal\blpOps.Web\DataServices\ClaimsDataService.cs:line 556
   at blpOps.Web.DataServices.ClaimsDataService.FindClaims(ClaimsSearchRequestViewModel claimsSearchRequestViewModel) in C:\Projects\BLPOpsPlatform\blpops-portal\blpOps.Web\DataServices\ClaimsDataService.cs:line 132
   at blpOps.Web.DataServices.ClaimsDataService.FindForIndex(ClaimsSearchRequestViewModel claimsSearchRequestViewModel) in C:\Projects\BLPOpsPlatform\blpops-portal\blpOps.Web\DataServices\ClaimsDataService.cs:line 178
   at blpOps.Web.Controllers.ClaimsController.Index() in C:\Projects\BLPOpsPlatform\blpops-portal\blpOps.Web\Controllers\ClaimsController.cs:line 110

Provider and version information

EF Core version: 5.0.1
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 5.0
Operating system: Win 10 Pro (also on Azure App Service)
IDE: Visual Studio 2019 16.8.3

[smitpatel]

Issue here is that provider can take null value hence provider.ID can be nullable. Hence in ternary, the return type of true/false parts being different, exception is thrown. If ternary worked fine then it would throw in binding to non-nullable property in initialization. It is true that the values which can give null provider.ID are being filtered out but EF Core translates the query to be processed on server. Evaluating above is basically understanding data. While some optimizations are performed by EF Core as long as they are local and not correlated to server, something like this, which is easy to see for human eyes is much more complex and fragile to do in expression tree because of components being in different places.
Few options

• Since CounselorID is FK to Counselor navigation, both of them will have null value at the same time so null check is redundant. (Navigations which are not loaded being null in client is just client behavior. When it is used inside query, it will get the appropriate value from database so it can only be null if FK is null)
• If you are writing a check and later filtering out a branch of it, then check can be skipped too (even without the above point). All accesses on server side are null-safe.
• If neither of above possible then the way user figures out that provider.ID cannot be null due to filters above, an explicit cast to Guid before provider.ID will also make the query work.

For fix in 6.0, we should try to introduce explicit cast iff without it expression tree building is an error. So rather than failing to build tree, it would fail for null data at runtime and if the query doesn't return null results (due to whatever filter), it would work just fine.

[smitpatel]

Fixed by #23877

Put a repro together from above data which throws in 5.0.x and works on 6.0 preview7.