dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.78k stars 3.19k forks source link

Incorrect apply projection for complex properties #32911

Closed tNRevan closed 8 months ago

tNRevan commented 9 months ago

When I combine ComplexProperty and AsSplitQuery with properties mapped to column with equal names, I get wrong values when reading the complex properties. Their values are equal (but they are different instances). If I don't use the AsSplitQuery or rename the columns, the results are correct. See the snippet below.

The code is using Microsoft.EntityFrameworkCore.Sqlite 8.0.1 package.

#nullable enable

void Main()
{
    using (var ctx = new MyContext())
    {
        ctx.Database.EnsureDeleted();
        ctx.Database.EnsureCreated();
        ctx.Add(new Offer 
        {
            Variations = [
                new Variation {
                    Payment = new Payment(120, 100),
                    Nested = new NestedEntity
                    {
                        Payment = new Payment(12, 10),
                    },
                    Nested2 = new NestedEntity2
                    {
                        Payment = new Payment(24, 20),
                    },
                }
            ]
        });
        ctx.SaveChanges();
        ctx.ChangeTracker.Clear(); // If I don't clear the change tracker, it also works.

        var entity = ctx.Offers
            .Include(e => e.Variations!)
                .ThenInclude(v => v.Nested)
            .Include(e => e.Variations!)
                .ThenInclude(v => v.Nested2)
            .AsSplitQuery() // If I comment this out, it works
            .Single(e => e.Id == 1);

        //  The values are equal even when they shouldn't be.
        Console.WriteLine($"Entity {entity.Variations?.Single().Payment}");
        Console.WriteLine($"Nested Entity {entity.Variations?.Single().Nested?.Payment}");
        Console.WriteLine($"Nested Entity2 {entity.Variations?.Single().Nested2?.Payment}");
    }
}

public class MyContext : DbContext
{
    public DbSet<Offer> Offers => Set<Offer>();

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseSqlite("DataSource=Test.db");
    }

    protected override void OnModelCreating(ModelBuilder builder)
    {
        builder.Entity<Variation>().ComplexProperty(p => p.Payment, price =>
        {
            price.Property(p => p.Netto).HasColumnName("payment_netto"); // If I change the column name to a different than the others, it also works
            price.Property(p => p.Brutto).HasColumnName("payment_brutto");
        });
        builder.Entity<NestedEntity>().ComplexProperty(p => p.Payment, price =>
        {
            price.Property(p => p.Netto).HasColumnName("payment_netto");
            price.Property(p => p.Brutto).HasColumnName("payment_brutto");
        });
        builder.Entity<NestedEntity2>().ComplexProperty(p => p.Payment, price =>
        {
            price.Property(p => p.Netto).HasColumnName("payment_netto");
            price.Property(p => p.Brutto).HasColumnName("payment_brutto");
        });
    }
}

public abstract class EntityBase 
{
    [System.ComponentModel.DataAnnotations.Key]
    public int Id {get; set; }
}

public class Offer : EntityBase
{   
    public ICollection<Variation>? Variations { get; set; }
}

public class Variation : EntityBase
{
    public Payment Payment {get; set; } = new Payment(0, 0);

    public NestedEntity? Nested {get; set; }
    public NestedEntity2? Nested2 {get; set; }
}

public class NestedEntity : EntityBase
{
    public Payment Payment {get; set; } = new Payment(0, 0);
}

public class NestedEntity2 : EntityBase
{
    public Payment Payment {get; set; } = new Payment(0, 0);
}

public record Payment(decimal Netto, decimal Brutto);

Include provider and version information

EF Core version: 8.0.1 Database provider: Microsoft.EntityFrameworkCore.Sqlite 8.0.1 Target framework: .NET 8.0 Operating system: Windows 10 IDE: Visual Studio 2022 17.8.5

ajcvickers commented 9 months ago

Still repros on latest daily. /cc @roji @maumar

maumar commented 9 months ago

Simplified:


#nullable enable

    [ConditionalFact]
    public void Repro32911()
    {
        using var ctx = new MyContext();
        ctx.Database.EnsureDeleted();
        ctx.Database.EnsureCreated();
        ctx.Add(new Offer
        {
            Variations = [
                new Variation
                {
                    Payment = new Payment(120, 100),
                    Nested = new NestedEntity
                    {
                        Payment = new Payment(12, 10),
                    },
                }
            ]
        });

        ctx.SaveChanges();
        ctx.ChangeTracker.Clear();

        var entity = ctx.Offers
            .Include(e => e.Variations!)
                .ThenInclude(v => v.Nested)
            .AsSplitQuery()
            .Single(e => e.Id == 1);

        var one = entity.Variations?.Single().Payment;
        var two = entity.Variations?.Single().Nested?.Payment;
        if (one == two) throw new InvalidOperationException("wrong");
    }

    public class MyContext : DbContext
    {
        public DbSet<Offer> Offers => Set<Offer>();

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder.UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=Repro32911;Trusted_Connection=True;MultipleActiveResultSets=true");
        }

        protected override void OnModelCreating(ModelBuilder builder)
        {
            builder.Entity<Variation>().ComplexProperty(p => p.Payment, price =>
            {
                price.Property(p => p.Netto).HasColumnName("payment_netto");
                price.Property(p => p.Brutto).HasColumnName("payment_brutto");
            });
            builder.Entity<NestedEntity>().ComplexProperty(p => p.Payment, price =>
            {
                price.Property(p => p.Netto).HasColumnName("payment_netto");
                price.Property(p => p.Brutto).HasColumnName("payment_brutto");
            });
        }
    }

    public abstract class EntityBase
    {
        [System.ComponentModel.DataAnnotations.Key]
        public int Id { get; set; }
    }

    public class Offer : EntityBase
    {
        public ICollection<Variation>? Variations { get; set; }
    }

    public class Variation : EntityBase
    {
        public Payment Payment { get; set; } = new Payment(0, 0);

        public NestedEntity? Nested { get; set; }
    }

    public class NestedEntity : EntityBase
    {
        public Payment Payment { get; set; } = new Payment(0, 0);
    }

    public record Payment(decimal Netto, decimal Brutto);

#nullable disable
maumar commented 9 months ago

query we generate is wrong:

SELECT TOP(2) [o].[Id]
FROM [Offers] AS [o]
WHERE [o].[Id] = 1
ORDER BY [o].[Id]

SELECT [s].[Id], [s].[NestedId], [s].[OfferId], [s].[payment_brutto], [s].[payment_netto], [s].[Id0], [o0].[Id]
FROM (
    SELECT TOP(1) [o].[Id]
    FROM [Offers] AS [o]
    WHERE [o].[Id] = 1
) AS [o0]
INNER JOIN (
    SELECT [v].[Id], [v].[NestedId], [v].[OfferId], [v].[payment_brutto], [v].[payment_netto], [n].[Id] AS [Id0]
    FROM [Variation] AS [v]
    LEFT JOIN [NestedEntity] AS [n] ON [v].[NestedId] = [n].[Id]
) AS [s] ON [o0].[Id] = [s].[OfferId]
ORDER BY [o0].[Id]

we only project brutto and netto values from Variation entity and don't project those from NestedEntity.

As comparison, single query generates this:

SELECT [o0].[Id], [s].[Id], [s].[NestedId], [s].[OfferId], [s].[payment_brutto], [s].[payment_netto], [s].[Id0], [s].[payment_brutto0], [s].[payment_netto0]
FROM (
    SELECT TOP(2) [o].[Id]
    FROM [Offers] AS [o]
    WHERE [o].[Id] = 1
) AS [o0]
LEFT JOIN (
    SELECT [v].[Id], [v].[NestedId], [v].[OfferId], [v].[payment_brutto], [v].[payment_netto], [n].[Id] AS [Id0], [n].[payment_brutto] AS [payment_brutto0], [n].[payment_netto] AS [payment_netto0]
    FROM [Variation] AS [v]
    LEFT JOIN [NestedEntity] AS [n] ON [v].[NestedId] = [n].[Id]
) AS [s] ON [o0].[Id] = [s].[OfferId]
ORDER BY [o0].[Id], [s].[Id]
maumar commented 9 months ago

problem is during apply projection. In GenerateComplexPropertyShaperExpression we loop through properties of the complex type to generate corresponding column, but we don't have enough information to create the correct column. We just take table alias and name from IColumn. However in our case the same column is projected multiple times (and aliased accoringly):

SELECT v.Id, v.NestedId, v.OfferId, v.payment_brutto, v.payment_netto, n.Id AS Id0, n.payment_brutto AS payment_brutto0, n.payment_netto AS payment_netto0
    FROM Variation AS v
    LEFT JOIN NestedEntity AS n ON v.NestedId == n.Id

so when we generate projection for the second complex type NestedEntity.Payment we associate it with payment_brutto and payment_netto columns rather than payment_brutto0 and payment_netto0. If the columns for both types have different names we generate distinct ones and everything works.

maumar commented 9 months ago

reopening for possible patch

solistice commented 8 months ago

Does this fix include occurrences where AsSplitQuery is not used? We encountered this bug as well without using AsSplitQuery. I looked at the commit but couldn't see if this was specific to the AsSplitQuery case, but all the comments and pointers state only that case. Happy to create and share some runnable code to show what I mean

roji commented 8 months ago

@solistice yeah, it should. Can you please try the latest daily build and confirm that it fixes your case? If it doesn't, can you please open a new issue with a minimal, runnable repo?

solistice commented 8 months ago

I can confirm that the latest daily build fixes our issue as well! Thanks!

roji commented 8 months ago

Thanks @solistice!

maumar commented 8 months ago

This issue has been approved for patch and will be available in 8.0.4, closing