HasConversion no longer supports captured Object in EF Core 9, when using projection

[TheConstructor]

As mentioned in #12205 (comment) and as requested by @roji:

When trying to update our project to EF Core 9 RC2, we encountered a NullReferenceException, were EF Core 8 did not throw one. The trouble seems to arise from the attempt to optimize a projection during query execution. If the conversion relies on a static object-instance no closure is created, and the optimization succeeds, but in all other cases it does not.

Short reproduction

<Project Sdk="Microsoft.NET.Sdk">

    <PropertyGroup>
        <TargetFramework>net8.0</TargetFramework>
        <ImplicitUsings>disable</ImplicitUsings>
        <Nullable>enable</Nullable>
    </PropertyGroup>

    <ItemGroup>
      <PackageReference Include="FluentAssertions" Version="6.12.1" />
<!--      <PackageReference Include="Microsoft.EntityFrameworkCore.Sqlite" Version="8.0.10" />-->
      <PackageReference Include="Microsoft.EntityFrameworkCore.Sqlite" Version="9.0.0-rc.2.24474.1" />
      <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.11.1" />
      <PackageReference Include="xunit" Version="2.9.2" />
    </ItemGroup>

</Project>

using System;
using System.Linq;
using System.Threading.Tasks;
using FluentAssertions;
using Microsoft.Data.Sqlite;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
using Xunit;

namespace EfCoreRepro;

internal sealed class CustomConverter
{
    private const string Dummy = "EncryptedValue:";
    public string Encrypt(string value) => Dummy + value;
    public string Decrypt(string value) => value.Substring(Dummy.Length);
}

public class Entity
{
    public long Id { get; set; }
    public DateTime? ValidUntil { get; set; }
    public string? One { get; set; }
    public string? Two { get; set; }
}

internal class MyDbContext(DbContextOptions<MyDbContext> options) : DbContext(options)
{
    private static readonly CustomConverter Converter = new();
    
    public DbSet<Entity> Entities { get; set; } = null!;
    
    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<Entity>(builder =>
        {
            builder.HasKey(e => e.Id);
            builder.Property(e => e.One)
                .HasConversion(new ValueConverter<string,string>(s => Converter.Encrypt(s), s => Converter.Decrypt(s))!);
            builder.Property(e => e.Two)
                .HasConversion(WithNonStaticExternal(Converter)!);
        });
    }

    private static ValueConverter<string, string> WithNonStaticExternal(CustomConverter customConverter)
    {
        return new ValueConverter<string,string>(s => customConverter.Encrypt(s), s => customConverter.Decrypt(s));
    }
}

public class QueryTest : IAsyncLifetime
{
    private SqliteConnection _connection;

    public async Task InitializeAsync()
    {
        _connection = new SqliteConnection("Filename=:memory:");
        await _connection.OpenAsync();
    }

    public async Task DisposeAsync()
    {
        await _connection.DisposeAsync();
    }

    private async Task<MyDbContext> PrepareDbContext()
    {
        var dbContextOptions = new DbContextOptionsBuilder<MyDbContext>()
            .UseSqlite(_connection)
            .Options;
        var dbContext = new MyDbContext(dbContextOptions);
        await dbContext.Database.EnsureCreatedAsync();
        dbContext.Entities.Add(new Entity {Id = 1, One = "One", Two = "Two"});
        await dbContext.SaveChangesAsync();
        return dbContext;
    }

    [Fact]
    public async Task QueryShouldAlwaysWork()
    {
        await using var dbContext = await PrepareDbContext();
        var id = 1L;
        var now = DateTime.UtcNow;
        var entity = await dbContext.Entities
            .Where(e => e.Id == id && (e.ValidUntil == null || e.ValidUntil > now))
            .FirstOrDefaultAsync();
        entity.Should().NotBeNull();
    }

    [Fact]
    public async Task ProjectingOneShouldAlwaysWork()
    {
        await using var dbContext = await PrepareDbContext();
        var id = 1L;
        var now = DateTime.UtcNow;
        var entity = await dbContext.Entities
            .Where(e => e.Id == id && (e.ValidUntil == null || e.ValidUntil > now))
            .Select(e => new {e.Id, e.One })
            .FirstOrDefaultAsync();
        entity.Should().NotBeNull();
    }

    [Fact]
    public async Task ProjectingTwoOnlyWorksIn8()
    {
        await using var dbContext = await PrepareDbContext();
        var id = 1L;
        var now = DateTime.UtcNow;
        var entity = await dbContext.Entities
            .Where(e => e.Id == id && (e.ValidUntil == null || e.ValidUntil > now))
            .Select(e => new {e.Id, e.Two })
            .FirstOrDefaultAsync();
        /*
        FirstOrDefaultAsync raises this exception in EF Core 9, but not 8:
        System.NullReferenceException: Object reference not set to an instance of an object.

System.NullReferenceException
Object reference not set to an instance of an object.
   at lambda_method83(Closure, QueryContext, DbDataReader, ResultContext, SingleQueryResultCoordinator)
   at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable`1.AsyncEnumerator.MoveNextAsync()
   at Microsoft.EntityFrameworkCore.Query.ShapedQueryCompilingExpressionVisitor.SingleOrDefaultAsync[TSource](IAsyncEnumerable`1 asyncEnumerable, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Query.ShapedQueryCompilingExpressionVisitor.SingleOrDefaultAsync[TSource](IAsyncEnumerable`1 asyncEnumerable, CancellationToken cancellationToken)
   at EfCoreRepro.QueryTest.ProjectingTwoOnlyWorksIn8() in C:\Repos\EfCoreRepo\EfCoreRepo\EfCoreRepro\QueryTest.cs:line 109
   at EfCoreRepro.QueryTest.ProjectingTwoOnlyWorksIn8() in C:\Repos\EfCoreRepo\EfCoreRepo\EfCoreRepro\QueryTest.cs:line 113
   at Xunit.Sdk.TestInvoker`1.<>c__DisplayClass47_0.<<InvokeTestMethodAsync>b__1>d.MoveNext() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestInvoker.cs:line 259
--- End of stack trace from previous location ---
   at Xunit.Sdk.ExecutionTimer.AggregateAsync(Func`1 asyncAction) in /_/src/xunit.execution/Sdk/Frameworks/ExecutionTimer.cs:line 48
   at Xunit.Sdk.ExceptionAggregator.RunAsync(Func`1 code) in /_/src/xunit.core/Sdk/ExceptionAggregator.cs:line 90
         */
        entity.Should().NotBeNull();
    }
}

Include provider and version information

EF Core version: 9.0.0-rc.2.24474.1
Database provider: I first found this using Microsoft.EntityFrameworkCore.SqlServer, but can reproduce it using Sqlite. InMemory is not affected.
Target framework: .NET 8.0
Operating system: Windows 11
IDE: Rider

[Li7ye]

EF Core 8.0 generate the below codes for projection in your demo.

(queryContext, dataReader, resultContext, resultCoordinator) => 
{
    string namelessParameter{0};
    string namelessParameter{1};
    namelessParameter{0} = dataReader.IsDBNull(0) ? default(string) : MyDbContext.Converter.Decrypt(dataReader.GetString(0));
    namelessParameter{1} = dataReader.IsDBNull(1) ? default(string) : <>c__DisplayClass7_0.customConverter.Decrypt(dataReader.GetString(1));
    return new { 
        One = namelessParameter{0}, 
        Two = namelessParameter{1}
     };
}

directly use closured variable for Entity.Two property.

<>c__DisplayClass7_0.customConverter.Decrypt(dataReader.GetString(1));

However, EF Core 9.0 generate the below codes:

(queryContext, dataReader, resultContext, resultCoordinator) => 
{
    string value1;
    string value2;
    value1 = dataReader.IsDBNull(0) ? default(string) : MyDbContext.Converter.Decrypt(dataReader.GetString(0));
    value2 = dataReader.IsDBNull(1) ? default(string) : Invoke(((ValueConverter<string, string>)[LIFTABLE Constant: SqliteStringTypeMapping | Resolver: c => (RelationalTypeMapping)c.Dependencies.TypeMappingSource.FindMapping(
        type: System.String, 
        model: c.Dependencies.Model, 
        elementMapping: null)].Converter).ConvertFromProviderTyped, dataReader.GetString(1));
    return new { 
        One = value1, 
        Two = value2
     };
}

EF 9.0 uses new LiftableConstantExpression to get constant value, but call ITypeMappingSource.FindMapping(type,model,elemntMapping) to get value converter for Entity.Two property, it is not a reliable way. so that always get default string type mapping without converter and throw NullReferenceException.

 c => (RelationalTypeMapping)c.Dependencies.TypeMappingSource.FindMapping(
        type: System.String, 
        model: c.Dependencies.Model, 
        elementMapping: null)].Converter).ConvertFromProviderTyped, dataReader.GetString(1))

efcore/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs

Lines 2912 to 2925 in 60524c9

	// NOTE: this is unreliable way to get type mapping. Only doing this as last resort, hoping to "guess" the right one
	Expression<Func<MaterializerLiftableConstantContext, Type, object>> resolverTemplate =
	(c, _) => (RelationalTypeMapping)c.Dependencies.TypeMappingSource.FindMapping(_, c.Dependencies.Model, null)!;
	var body = ReplacingExpressionVisitor.Replace(
	resolverTemplate.Parameters[1],
	Constant(type),
	resolverTemplate.Body);
	typeMappingExpression = liftableConstantFactory.CreateLiftableConstant(
	typeMapping,
	Lambda<Func<MaterializerLiftableConstantContext, object>>(body, resolverTemplate.Parameters[0]),
	"typeMapping",
	typeof(RelationalTypeMapping));

I think this issue have fixed in main branch already.
Like EF 8.0 directly use closured variable for Entity.Two property.

efcore/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs

Lines 2792 to 2802 in 931a67c

	else
	{
	if (valueExpression.Type != converter.ProviderClrType)
	{
	valueExpression = Convert(valueExpression, converter.ProviderClrType);
	}
	valueExpression = ReplacingExpressionVisitor.Replace(
	converter.ConvertFromProviderExpression.Parameters.Single(),
	valueExpression,
	converter.ConvertFromProviderExpression.Body);

[maumar]

Thanks for the investigation @Li7ye ! @TheConstructor the fix indeed found its way to 9.0 so this should be fixed when we release. You can also try our daily build. Closing as dupe of #34760

[TheConstructor]

Thank you @Li7ye and @maumar it indeed works with 9.0.0-rtm.24514.19. Please excuse the inconvenience caused.