Comparing a property with a value converter against null throws exception in EF7 InMemory

[alitas]

Since upgrading to EF7, unit tests utilizing EF InMemory provider started failing. Upon investigating, I was able to narrow it down to null checks not being handled correctly. Digging into the source code, it looks like changes in #27654 this commit might have caused the regression: 3ad8b2e.

Specifically, in this code: https://github.com/dotnet/efcore/blob/main/src/EFCore.InMemory/Query/Internal/InMemoryExpressionTranslatingExpressionVisitor.cs#L286-L302, the IsAssignable checks on leftType and rightType do not always guarantee that the newLeft and newRight can be passed to the Replace method, as they can be a ConstantExpression with null value.

You can find the relevant code snippets below, or find the whole reproduction at https://github.com/alitas/ef7-inmemory-converter-null-bug.

Relevant code (Sample from https://learn.microsoft.com/en-us/ef/core/modeling/value-comparers)

public class Post
{
    public int Id { get; set; }
    public string? Title { get; set; }
    public string? Contents { get; set; }
    public ICollection<string>? Tags { get; set; }
}

  modelBuilder.Entity<Post>()
      .Property(e => e.Tags)
      .HasConversion(
          v => JsonSerializer.Serialize(v, (JsonSerializerOptions?) null),
          v => JsonSerializer.Deserialize<List<string>>(v, (JsonSerializerOptions?) null),
          new ValueComparer<ICollection<string>>(
              (c1, c2) => (c1 == null && c2 == null) || c1 != null && c2 != null && c1.SequenceEqual(c2),
              c => c.Aggregate(0, (a, v) => HashCode.Combine(a, v.GetHashCode())),
              c => (ICollection<string>) c.ToList()));

ICollection<string>? nullTags = null;

// This query runs successfully
var postsWithoutTags = await dbContext.Posts.CountAsync(p => p.Tags != nullTags && p.Tags.Count > 0);
Console.WriteLine($"There are {postsWithoutTags} posts without tags according to the first query.");

// This query fails with the following exception
// System.ArgumentException: Expression of type 'System.Object' cannot be used for parameter of type 'System.String' of
// method 'System.Collections.Generic.List`1[System.String] Deserialize[List`1](System.String, System.Text.Json.JsonSerializerOptions)' (Parameter 'arg0')
postsWithoutTags = await dbContext.Posts.CountAsync(p => p.Tags != null && p.Tags.Count > 0);
Console.WriteLine($"There are {postsWithoutTags} posts without tags according to the second query.");

Stack trace

Unhandled exception. System.ArgumentException: Expression of type 'System.Object' cannot be used for parameter of type 'System.String' of method 'System.Collections.Generic.List`1[System.String] Deserialize[List`1](System.String, System.Text.Json.JsonSerializerOptions)' (Parameter 'arg0')
   at System.Dynamic.Utils.ExpressionUtils.ValidateOneArgument(MethodBase method, ExpressionType nodeKind, Expression arguments, ParameterInfo pi, String methodParamName, String argumentParamName, Int32 index)
   at System.Linq.Expressions.Expression.ValidateOneArgument(MethodBase method, ExpressionType nodeKind, Expression arg, ParameterInfo pi, String methodParamName, String argumentParamName)
   at System.Linq.Expressions.Expression.Call(MethodInfo method, Expression arg0, Expression arg1)
   at System.Linq.Expressions.MethodCallExpression2.Rewrite(Expression instance, IReadOnlyList`1 args)
   at System.Linq.Expressions.ExpressionVisitor.VisitMethodCall(MethodCallExpression node)
   at Microsoft.EntityFrameworkCore.Query.ReplacingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at System.Linq.Expressions.MethodCallExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.ReplacingExpressionVisitor.Visit(Expression expression)
   at Microsoft.EntityFrameworkCore.Query.ReplacingExpressionVisitor.Replace(Expression original, Expression replacement, Expression tree)
   at Microsoft.EntityFrameworkCore.InMemory.Query.Internal.InMemoryExpressionTranslatingExpressionVisitor.VisitBinary(BinaryExpression binaryExpression)
   at System.Linq.Expressions.BinaryExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.InMemory.Query.Internal.InMemoryExpressionTranslatingExpressionVisitor.VisitBinary(BinaryExpression binaryExpression)
   at System.Linq.Expressions.BinaryExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.InMemory.Query.Internal.InMemoryExpressionTranslatingExpressionVisitor.TranslateInternal(Expression expression)
   at Microsoft.EntityFrameworkCore.InMemory.Query.Internal.InMemoryExpressionTranslatingExpressionVisitor.Translate(Expression expression)
   at Microsoft.EntityFrameworkCore.InMemory.Query.Internal.InMemoryQueryableMethodTranslatingExpressionVisitor.TranslateExpression(Expression expression, Boolean preserveType)
   at Microsoft.EntityFrameworkCore.InMemory.Query.Internal.InMemoryQueryableMethodTranslatingExpressionVisitor.TranslateLambdaExpression(ShapedQueryExpression shapedQueryExpression, LambdaExpression lambdaExpression, Boolean preserveType)
   at Microsoft.EntityFrameworkCore.InMemory.Query.Internal.InMemoryQueryableMethodTranslatingExpressionVisitor.TranslateWhere(ShapedQueryExpression source, LambdaExpression predicate)
   at Microsoft.EntityFrameworkCore.InMemory.Query.Internal.InMemoryQueryableMethodTranslatingExpressionVisitor.TranslateCount(ShapedQueryExpression source, LambdaExpression predicate)
   at Microsoft.EntityFrameworkCore.Query.QueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at Microsoft.EntityFrameworkCore.InMemory.Query.Internal.InMemoryQueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at System.Linq.Expressions.MethodCallExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   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__DisplayClass12_0`1.<ExecuteAsync>b__0()
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.ExecuteAsync[TResult](Expression query, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.ExecuteAsync[TResult](Expression expression, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ExecuteAsync[TSource,TResult](MethodInfo operatorMethodInfo, IQueryable`1 source, Expression expression, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ExecuteAsync[TSource,TResult](MethodInfo operatorMethodInfo, IQueryable`1 source, LambdaExpression expression, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.CountAsync[TSource](IQueryable`1 source, Expression`1 predicate, CancellationToken cancellationToken)

Provider and version information

EF Core version: 7.0
Database provider: Microsoft.EntityFrameworkCore.InMemory
Target framework: .NET 6 and 7

[adamhathcock]

Have this exact problem in some tests. Thanks for the workaround.

Is this not going to be fixed until 8?

Just want to know how much adjustment I need to do make. Thanks.

[ajcvickers]

@adamhathcock (and others): make sure to vote (👍) for this issue. In order for us to get approval to patch an issue we have to provide evidence to the powers-that-be that the issue is highly impacting to customers.

[adamhathcock]

Thanks!

I'm not sure how it's not more impactful as it's a common practice to have Memory DB for some testing. I guess it's the magic combination of in-memory usage with a null check that way that might be rarer.

Unfortunately, the fix requires me to change the tested code and not tests.

[ajcvickers]

it's a common practice to have Memory DB for some testing

It is, but it's not reecommend. See Testing EF Core Applications.

[adamhathcock]

I do full integraiton tests with a real DB as well. Might just switch the in-memory tests to the SQLite in-memory to avoid this too.

[roji]

@adamhathcock if you're already doing full integration tests with a real DB, you've gone to the trouble of setting up your test infrastructure and hopefully testing efficiently and in isolation. Assuming that's the case, we recommend considering just doing all your tests against your real database, as you've already gone through that effort and can avoid all the problems associated with both InMemory and SQLite in-memory (e.g. different query results).

[adamhathcock]

there's a variety of tests for history reasons, this might prompt the need to rewrite the memory ones but we'll see.

the full integration tests against mysql in a container are great but not all areas have them yet

[jochenjonc]

@ajcvickers today while trying to upgrade to .NET 7 and EF Core 7 I encountered this issue. For the moment the only workaround I found was using 8.0.0-preview.1.23111.4 of Microsoft.EntityFrameworkCore.InMemory.
Are there any plans to fix this in EF Core 7? I cannot find it in the 7.0.x milestone.

[ajcvickers]

@jochenjonc We looked into patching this, but the code changes are too extensive to include in a patch release.

[analogrelay]

Is there any workaround available? We are currently blocked from upgrading to EF 7 because of this issue.

I recognize that the In-Memory Provider isn't intended for testing, but is it not a supported part of Entity Framework? What is the supported use case for the In-Memory Provider? If there isn't one that would warrant patching a significant breaking change like this, perhaps the package should no longer be published.

[ajcvickers]

@analogrelay Workarounds are:

• Disable the tests for now
• Convert the tests to use a different provider, possibly SQLite in-memory

What is the supported use case for the In-Memory Provider?

It's not recommended for any customer use case.

If there isn't one that would warrant patching a significant breaking change like this, perhaps the package should no longer be published.

Indeed; that has been considered. Would that be a better solution for you?

[analogrelay]

Removing the package or making some clear statement that it's not part of the Entity Framework Core product itself would certainly help clarify things. As it is, while it is listed as not "intended for production" in the docs, the implication is that it's still a part of Entity Framework, and thus covered by the same support expectations. In fact the docs say "The provider is maintained by Microsoft as part of the Entity Framework Core Project." If it is no longer covered by the support expectations of the product, it would be good to clarify that.

Removing all of our code using the in memory provider isn't feasible for a while so we'll be remaining on EF Core 6.

Thanks for clarifying the support policy here.