Nullable<Guid>.GetValueOrDefault is not translated

[NRKirby]

I get an InvalidOperationException when I try filtering a DbSet on a property that is a nullable Guid.

Steps to reproduce

When I execute the following query on an Orders DbSet where MachineId is a nullable Guid.

var machineIds // a list of Guids

var orders = _context.Orders.Where(x => machineIds.Contains(x.MachineId.GetValueOrDefault()));

I get the following exception

Message: 
    System.InvalidOperationException : The LINQ expression 'DbSet<Order>
        .Where(m => __machineIds_0
            .Contains((Nullable<Guid>)m.MachineId.GetValueOrDefault()))' could not be translated. Either rewrite the query in a form that can be translated, or switch to client evaluation explicitly by inserting a call to either AsEnumerable(), AsAsyncEnumerable(), ToList(), or ToListAsync(). See https://go.microsoft.com/fwlink/?linkid=2101038 for more information.
  Stack Trace: 
    QueryableMethodTranslatingExpressionVisitor.<VisitMethodCall>g__CheckTranslated|8_0(ShapedQueryExpression translated, <>c__DisplayClass8_0& )
    QueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
    RelationalQueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
    MethodCallExpression.Accept(ExpressionVisitor visitor)
    QueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
    RelationalQueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
    MethodCallExpression.Accept(ExpressionVisitor visitor)
    QueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
    RelationalQueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
    MethodCallExpression.Accept(ExpressionVisitor visitor)
    QueryCompilationContext.CreateQueryExecutor[TResult](Expression query)
    Database.CompileQuery[TResult](Expression query, Boolean async)
    QueryCompiler.CompileQueryCore[TResult](IDatabase database, Expression query, IModel model, Boolean async)
    <>c__DisplayClass9_0`1.<Execute>b__0()
    CompiledQueryCache.GetOrAddQueryCore[TFunc](Object cacheKey, Func`1 compiler)
    CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func`1 compiler)
    QueryCompiler.Execute[TResult](Expression query)
    EntityQueryProvider.Execute[TResult](Expression expression)
    Queryable.FirstOrDefault[TSource](IQueryable`1 source, Expression`1 predicate)
    QueryHandler.GetProducts(Query request, CancellationToken cancellationToken, String normalisedCultureCode, IList`1 allVideos, IList`1 allImages, IList`1 allBrochures, IList`1 allSpecs, IList`1 allMachines) line 385
    QueryHandler.Handle(Query request, CancellationToken cancellationToken) line 127
    RequestPostProcessorBehavior`2.Handle(TRequest request, CancellationToken cancellationToken, RequestHandlerDelegate`1 next)
    RequestPreProcessorBehavior`2.Handle(TRequest request, CancellationToken cancellationToken, RequestHandlerDelegate`1 next)
    TestContext.ExecuteScopeAsync[T](Func`2 action) line 156
    GetByHubShould.ReturnCorrectFeaturedSpecifications() line 372
    <>c.<ThrowAsync>b__139_0(Object state)

-->

Further technical details

EF Core version: 3.1 Database provider: Microsoft.EntityFrameworkCore.SqlServer Target framework: NET Core 3.1 Operating system: Windows IDE: Visual Studio 2019 16.7.0

[smitpatel]

@roji to write the work-around query.

[roji]

Until we implement this, you can simply use the coalescing operator to work around it:

var orders = _context.Orders.Where(x => machineIds.Contains(x.MachineId ?? default));

[roji]

Note: consider regular COALESCE vs. IS_NULL here (see #32519, though that discussion should apply both to the regular C# coalescing operator and to GetValueOrDefault)

[John0King]

IMO, EF should translte all COALESCE to ISNULL .

EF current tanslate ?? operator to COALESCE , COALESCE is actrully the CASE WHEN expression, For field scenario (eg. COALESCE(x.FieldA, 0) ) , COALESCE should have the same performance than ISNULL() ; for expression scenario (eg. COALESCE((SELECT TOP 1 FIELD FROM TABLE), 0)), COALESCE is much slower ,because it need evaluate the expression twice , because CASE WHEN condition evaluate it first and then the THEN/ELSE return value evaluate it after. eg.

CASE WHEN  
    (SELECT TOP 1 FIELD FROM TABLE) IS NOT NULL
THEN 
    (SELECT TOP 1 FIELD FROM TABLE)
ELSE
    0
END 

and in real test , ISNULL is faster for both filed and expression

[roji]

for expression scenario [...] COALESCE is much slower

@John0King do you have any links/resources to back that up, or better yet, a small benchmark that shows this performance difference?

Note this useful post on COALESCE vs. ISNULL, according to which the performance is the same - although it indeed doesn't test the expression/subquery case.

[John0King]

@roji
I don't have a professional benchmarks, just with SSMS + Sqlserver 2022 in docker with a large table

name	1st run	2nd run	3rd run	4th run
Filed_ISNULL	00:00:10.593	00:00:10.122	00:00:10.550	00:00:09.980
Filed_COALESCE	00:00:10.330	00:00:10.623	00:00:10.708	00:00:10.235
expression_COALESCE	00:00:28.083	00:00:29.340	00:00:27.289	00:00:29.223
expression_ISNULL	00:00:12.194	00:00:12.587	00:00:12.624	00:00:12.827

and I also have an another compare about expression case (but the isnull case is not reporting the real time execution plan until all output done, I think this benchmark is not a fair compare)	name	1st run	2nd run	3rd run	4th run
expression_COALESCE	00:00:23.286	00:00:21.499	00:00:21.042	00:00:21.443
expression_ISNULL	00:00:29.659	00:00:29.874	00:00:31.610	00:00:31.610

and there is stackoverflow answer about them tow. https://stackoverflow.com/questions/7408893/using-isnull-vs-using-coalesce-for-checking-a-specific-condition

and this result prove that COALESCE indeed evaluate twice:

[roji]

and this result prove that COALESCE indeed evaluate twice:

~I'm unable to reproduce this: wouldn't COALESCE returning NULL in this case be a violation of its behavior, regardless of any performance considerations? I tried to reproduce with the following code (please don't post screenshots in github issues - always post actual text):~

CREATE TABLE Foo (A int, B int);
INSERT INTO Foo (A, B) VALUES (1, NULL), (2, 2), (3, NULL);
SELECT COALESCE((SELECT TOP 1 B FROM Foo ORDER BY NEWID() ASC), 0);

~This outputs 0, not NULL.~

After running the above several times, I could actually observe a NULL being returned - so you're right.

[roji]

@John0King in any case, I've opened #32519 to track the COALESCE vs. ISNULL question. I don't see this as relevant for translating GetValueOrDefault() (this issue) in any way: if we end up deciding that ISNULL is better, then we translate to that also when the normal C# coalescing operator is used. Whether or not we translate GetValueOrDefault() is unrelated to that.