Wrong mapping of nested complex property with generic types and same property name, resulting in InvalidCastException

[lucahost]

File a bug

I stumbled upon a bug in the RelationalSqlTranslatingExpressionVisitor.CreatePropertyAccessExpression
I have a .Where() query comparing 2 ComplexProperties of a given type:

Repro-Repo

I set up a repository to reproduce the error: https://github.com/lucahost/ef-complex-bug-repro
Just adjust the connection string in Program.cs and run the sample.

Setup

var orderPosition = // load orderPosition

dbContext.DbSet<Shipment>()
  .Where(shipmnet => shipment.Recipient == orderPosition.Recipient)
  .ToList();
...

public record Recipient
{
    public required Address Address { get; init; }
    public required Customer Customer { get; init; }
    public required Person BuyerPerson { get; init; }
}

public sealed record Address
{
    public required Id<Address> Id { get; init; }
}
public sealed record Customer
{
    public required Id<Customer> Id { get; init; }
}
public sealed record Person
{
    public required Id<Person> Id { get; init; }
}

builder.ComplexProperty(
    shipment => shipment.Recipient,
    complexPropertyBuilder =>
    {
        complexPropertyBuilder
            .ComplexProperty(
                recipient => recipient.Address,
                addressBuilder => addressBuilder.Property(address => address.Id)
                    .HasConversion<IdConverter<Address>>()
                    .IsRequired())
            .ComplexProperty(
                recipient => recipient.Customer,
                customerBuilder => customerBuilder.Property(customer => customer.Id)
                    .HasConversion<IdConverter<Customer>>()
                    .IsRequired())
            .ComplexProperty(
                recipient => recipient.BuyerPerson,
                buyerPersonBuilder => buyerPersonBuilder.Property(person => person.Id)
                    .HasConversion<IdConverter<Person>>()
                    .IsRequired());
    });

// Same Mapping for OrderPosition

Now in the RelationalSqlTranslatingExpressionVisitor in CreatePropertyAccessExpression the SQL statements parameterName for OrderPosition.Recipient.Address.Id of Type Id<Address> is generated as following: __entity_equality_orderPosition_Recipient_0_Id

Exception in RelationalTypeMapping.CreateParameter

Then in the conversion of the parameter to the provider, the wrong cast is being made.
The SQL-parameter @__entity_equality_orderPosition_Recipient_0_Id of type Id<Customer> would be converted to Id<Address>

Assumption

As you see, the Id property on the complex types are all called the same. I think this is the culprit which results in the parameter not being added correctly.

Workarounds

One workaround was to get rid of the nesting -> OrderPosition.Recipient.AddressId where Recipient is configured as ComplexProperty() and AddresId as .Property().

Another one I found out while stepping through the code was by using a distinct parameter-name in RelationalSqlTranslatingExpressionVisitor.CreatePropertyAccessExpression. Something along the lines

// get generic class as propertyName
var propName = $"{property.ClrType.GenericTypeArguments[0].Name}_{property.Name}";

var newParameterName =
    $"{RuntimeParameterPrefix}"
    + $"{sqlParameterExpression.Name[QueryCompilationContext.QueryParameterPrefix.Length..]}_{propName }";

Later is not a solution for everything, but was working in my case.
I think the parameter-name should be generated something like this: __entity_equality_orderPosition_Recipient_0_Address_Id including the nested properties name.

EF Core version: 8.0.3
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 8.0
Operating system:
IDE: Rider 2024.1 RC

[roji]

Thanks, confirmed the bug.

Minimal repro

await using var context = new BlogContext();
await context.Database.EnsureDeletedAsync();
await context.Database.EnsureCreatedAsync();

var container = new ComplexContainer
{
    Id = 1,
    Containee1 = new() { Id = 2 },
    Containee2 = new() { Id = 3 }
};

_ = await context.Blogs.Where(b => b.ComplexContainer == container).ToListAsync();

public class BlogContext : DbContext
{
    public DbSet<Blog> Blogs { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            .UseSqlServer("Server=localhost;Database=test;User=SA;Password=Abcd5678;Connect Timeout=60;ConnectRetryCount=0;Encrypt=false")
            .LogTo(Console.WriteLine, LogLevel.Information)
            .EnableSensitiveDataLogging();

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<Blog>().ComplexProperty(b => b.ComplexContainer, x =>
        {
            x.ComplexProperty(c => c.Containee1);
            x.ComplexProperty(c => c.Containee2);
        });
    }
}

public class Blog
{
    public int Id { get; set; }
    public ComplexContainer ComplexContainer { get; set; }
}

public class ComplexContainer
{
    public int Id { get; set; }

    public ComplexContainee1 Containee1 { get; set; }
    public ComplexContainee2 Containee2 { get; set; }
}

public class ComplexContainee1
{
    public int Id { get; set; }
}

public class ComplexContainee2
{
    public int Id { get; set; }
}

[roji]

Backporting discussion: data corruption bug, plus customer impact is probably common enough. In addition, the fix is trivial and very low-risk.