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.79k stars 3.19k forks source link

Wrong SQL generated when using Include in a Many-To-Many relationship after migration from EF6 #21358

Closed dglozano closed 2 years ago

dglozano commented 4 years ago

We have recently decided to port an existing application using .NET Framework 4.7 Web API and EF6 to .NET Core and EF Core 3.1.

As a first step, we are going to move to EF Core. Once that's done, we will move later to .NET Core.

We are almost done with the porting to EF Core, but unfortunately we are experiencing an incorrect behavior when using .Include() statements in the Entities we had to create to map N-to-N relationships. The SQL generated is trying to read non existing columns which ends up in a "Invalid column name" error when executing the query in the database.

For example, we have a Project entity which has a n-to-n relationship with Department. When porting to EF Core, we needed to add the intermediate entity ProjectDepartment to do the mapping.

Project.cs

using System;
using System.Collections.Generic;
using System.Linq;

namespace Company.DI.Core.Entity
{
    public class Project
    {
        public int ProjectId { get; set; }

        ...
        public virtual ICollection<ProjectDepartment> ProjectDepartments { get; set; }

    }
}

Department.cs

using System.Collections.Generic;

namespace Company.DI.Core.Entity
{
    public class Department
    {
        public int DepartmentId { get; set; }

        public virtual ICollection<ProjectDepartment> ProjectDepartments { get; set; }

        ...

    }
}

ProjectDepartment.cs

namespace Company.DI.Core.Entity
{
    public class ProjectDepartment
    {
        public int ProjectId { get; set; }

        public Project Project { get; set; }

        public int DepartmentId { get; set; }

        public Department Department { get; set; }
    }
}

ProjectConfiguration.cs

using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Metadata.Builders;

namespace Company.DI.Core.DataAccess.Sql.EntityConfigurations
{
    public class ProjectConfiguration : IEntityTypeConfiguration<Project>
    {
        public void Configure(EntityTypeBuilder<Project> builder)
        {
            builder.ToTable("Project");
            builder.HasKey(k => k.ProjectId);

           // Nothing related to Department or ProjectDepartment here...

            builder.Property(p => p.ProjectId).IsRequired();
            builder.Property(p => p.Created).IsRequired();
            builder.Property(p => p.Modified).IsRequired();
            builder.Property(p => p.CreatedByUserId).IsRequired();
            builder.Property(p => p.ModifiedByUserId).IsRequired();
            builder.Property(p => p.FieldId).IsRequired();
            builder.Property(p => p.FacilityId).IsRequired();
            builder.Property(p => p.WellboreId).IsRequired();
            builder.Property(p => p.Status);
            builder.Property(p => p.PlannedStartDate);
            builder.Property(p => p.PlannedEndDate);
            builder.Property(p => p.StartTime);
            builder.Property(p => p.EndTime);
            builder.Property(p => p.ReferenceNumber);
            builder.Property(p => p.EstimatedStart).HasMaxLength(20).IsRequired();
            builder.Property(p => p.EstimatedEnd).HasMaxLength(20).IsRequired();
            builder.Property(p => p.ScopeId);
            builder.Property(p => p.Comments).HasMaxLength(500);
            builder.Property(p => p.FailReason);

            builder
                .HasMany(x => x.Files)
                .WithOne(x => x.Project)
                .HasForeignKey(x => x.ProjectId);

            builder
                .HasMany(x => x.Threads)
                .WithOne(x => x.Project).IsRequired()
                .HasForeignKey(x => x.ProjectId);

            builder
                .HasMany(x => x.ThreadMessages)
                .WithOne(x => x.Project).IsRequired()
                .HasForeignKey(x => x.ProjectId)
                .OnDelete(DeleteBehavior.Restrict);

            builder
                .HasMany(x => x.ProjectMembers)
                .WithOne(x => x.Project).IsRequired()
                .HasForeignKey(x => x.ProjectId);

            builder
                .HasMany(x => x.Tasks)
                .WithOne(x => x.Project).IsRequired()
                .HasForeignKey(x => x.ProjectId);

            builder
                .HasMany(x => x.Activities)
                .WithOne(x => x.Project).IsRequired()
                .HasForeignKey(x => x.ProjectId)
                .OnDelete(DeleteBehavior.Restrict);

            builder
                .HasMany(x => x.ProjectTimers)
                .WithOne(x => x.Project).IsRequired()
                .HasForeignKey(x => x.ProjectId);

            builder
                .HasMany(x => x.Lessons)
                .WithOne(x => x.Project).IsRequired()
                .HasForeignKey(x => x.ProjectId);
        }
    }
}

DepartmentConfiguration.cs

using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Metadata.Builders;

namespace Company.DI.Core.DataAccess.Sql
{
    public class DepartmentConfiguration : IEntityTypeConfiguration<Department>
    {
        public void Configure(EntityTypeBuilder<Department> builder)
        {
            builder.ToTable("Department");
            builder.HasKey(k => k.DepartmentId);
            builder.Property(p => p.Created).IsRequired();
            builder.Property(p => p.Modified).IsRequired();
            builder.Property(p => p.Name).HasMaxLength(100).IsRequired();
            builder.Property(p => p.IsEnabled).IsRequired();
            builder.HasIndex(i => i.Name).IsUnique(true);
        }
    }
}

DepartmentConfiguration.cs

using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Metadata.Builders;

namespace Company.DI.Core.DataAccess.Sql
{
    public class DepartmentConfiguration : IEntityTypeConfiguration<Department>
    {
        public void Configure(EntityTypeBuilder<Department> builder)
        {
            builder.ToTable("Department");
            builder.HasKey(k => k.DepartmentId);
            builder.Property(p => p.Created).IsRequired();
            builder.Property(p => p.Modified).IsRequired();
            builder.Property(p => p.Name).HasMaxLength(100).IsRequired();
            builder.Property(p => p.IsEnabled).IsRequired();
            builder.HasIndex(i => i.Name).IsUnique(true);
        }
    }
}

ProjectDepartment.Configurationcs

using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Metadata.Builders;

namespace Company.DI.Core.DataAccess.Sql
{
    public class ProjectDepartmentConfiguration : IEntityTypeConfiguration<ProjectDepartment>
    {
        public void Configure(EntityTypeBuilder<ProjectDepartment> builder)
        {
            builder
                .HasKey(pd => new { pd.ProjectId, pd.DepartmentId });

            builder
                .HasOne(pd => pd.Project)
                .WithMany(p => p.ProjectDepartments)
                .HasForeignKey(pd => pd.ProjectId);

            builder
                .HasOne(pd => pd.Department)
                .WithMany(d => d.ProjectDepartments)
                .HasForeignKey(pd => pd.DepartmentId);
        }
    }
}

Now the problem is that whenever we try to use an Include from the "join" entity we would get an invalid SQL translation. For example, these will fail:

The SQL generated is trying to SELECT a ProjectId column in the Department table or a DepartmentId in the Project table when doing those includes, columns which of course don't exist.

Here is a proper example:

IQueryable

Context.Project
                .Include(x => x.CreatedBy)
                .Include(x => x.ModifiedBy)
                .Include(x => x.Field)
                .Include(x => x.Facility)
                .Include(x => x.Wellbore)
                .Include(x => x.ProjectDepartments)
                .ThenInclude(x => x.Department)
                .Include(x => x.ProjectServices)
                .ThenInclude(x => x.Service)
                .Include(x => x.Scope)
                .Include(x => x.MeasurementPreference)
                .Include(x => x.ProjectMembers)
                .Where(x => x.ProjectMembers.Any(m => m.UserId == userId))
                .OrderBy(x => x.ProjectId)
                .ToList();

Generated SQL

SELECT [p].[ProjectId], [p].[Comments], [p].[Created], [p].[CreatedByUserId], [p].[Description], [p].[EndTime], [p].[EstimatedEnd], [p].[EstimatedStart], [p].[FacilityId], [p].[FailReason], [p].[FieldId], [p].[MeasurementPreferenceId], 
[p].[MeasurementSystem], [p].[Modified], [p].[ModifiedByUserId], [p].[PlannedEndDate], [p].[PlannedStartDate], [p].[ReferenceNumber], [p].[ScopeId], [p].[StartTime], [p].[Status], [p].[WellboreId], [u].[UserId], [u].[AccountName], [u].[Avatar], [u].[Created], 
[u].[DirectoryId], [u].[FirstName], [u].[FullName], [u].[IsDeleted], [u].[JobTitle], [u].[LastName], [u].[Mail], [u].[MeasurementPreferenceId], [u].[MeasurementSystem], [u].[MobilePhone], [u].[Modified], [u].[OrganizationId], [u].[UserType], [u0].[UserId], [u0].[AccountName], [u0].[Avatar], [u0].[Created], [u0].[DirectoryId], [u0].[FirstName], [u0].[FullName], [u0].[IsDeleted], [u0].[JobTitle], [u0].[LastName], [u0].[Mail], [u0].[MeasurementPreferenceId], [u0].[MeasurementSystem], [u0].[MobilePhone], [u0].[Modified], [u0].[OrganizationId], [u0].[UserType], [f].[FieldId], [f].[Created], [f].[Modified], [f].[NPDFieldId], [f].[Name], [f].[Status], [f].[StatusFrom], [f].[StatusTo], [f0].[FacilityId], [f0].[Created], [f0].[FieldId], [f0].[Modified], [f0].[NPDFacilityId], [f0].[NPDFieldId], [f0].[Name], [f0].[Operator], [f0].[Status], [f0].[WaterDepth], [w].[WellboreId], [w].[CompletedDate], [w].[Content], [w].[Created], [w].[EnteredDate], [w].[FieldId], [w].[KellyBushingElevation], [w].[LocationEW], [w].[LocationNS], [w].[MeasuredDepth], [w].[Modified], [w].[NPDFieldId], [w].[NPDWellboreId], [w].[Purpose], [w].[Status], [w].[VerticalDepth], [w].[WaterDepth], [w].[WellName], [w].[WellboreName], [s].[ScopeId], [s].[Created], [s].[IsEnabled], [s].[Modified], [s].[Name], [m].[MeasurementPreferenceId], [m].[Created], [m].[CreatedByUserId], [m].[Length], [m].[Mass], [m].[Modified], [m].[ModifiedByUserId], [m].[Pressure], [m].[Speed], [m].[Temperature], [m].[VolumeFlowRate], [t].[ProjectId], [t].[DepartmentId], [t].[DepartmentId0], [t].[Created], [t].[IsEnabled], [t].[Modified], [t].[Name], [t].[ProjectId0], [t].[Sequence], [t0].[ProjectId], [t0].[ServiceId], [t0].[ServiceId0], [t0].[Created], [t0].[IsVisible], [t0].[Modified], [t0].[Name], [t0].[ProjectId0], [p2].[ProjectMemberId], [p2].[Created], [p2].[CreatedByUserId], [p2].[Modified], [p2].[ModifiedByUserId], [p2].[ProjectId], [p2].[ProjectMemberRole], [p2].[UserId]
FROM [Project] AS [p]
INNER JOIN [User] AS [u] ON [p].[CreatedByUserId] = [u].[UserId]
INNER JOIN [User] AS [u0] ON [p].[ModifiedByUserId] = [u0].[UserId]
INNER JOIN [Field] AS [f] ON [p].[FieldId] = [f].[FieldId]
INNER JOIN [Facility] AS [f0] ON [p].[FacilityId] = [f0].[FacilityId]
INNER JOIN [Wellbore] AS [w] ON [p].[WellboreId] = [w].[WellboreId]
INNER JOIN [Scope] AS [s] ON [p].[ScopeId] = [s].[ScopeId]
LEFT JOIN [MeasurementPreference] AS [m] ON [p].[MeasurementPreferenceId] = [m].[MeasurementPreferenceId]
LEFT JOIN (
    SELECT [p0].[ProjectId], [p0].[DepartmentId], [d].[DepartmentId] AS [DepartmentId0], [d].[Created], [d].[IsEnabled], [d].[Modified], [d].[Name],

 ---WRONG---> [d].[ProjectId] AS [ProjectId0] <---WRONG----,

 [d].[Sequence]
    FROM [ProjectDepartment] AS [p0]
    INNER JOIN [Department] AS [d] ON [p0].[DepartmentId] = [d].[DepartmentId]
) AS [t] ON [p].[ProjectId] = [t].[ProjectId]
LEFT JOIN (
    SELECT [p1].[ProjectId], [p1].[ServiceId], [s0].[ServiceId] AS [ServiceId0], [s0].[Created], [s0].[IsVisible], [s0].[Modified], [s0].[Name], [s0].[ProjectId] AS [ProjectId0]
    FROM [ProjectService] AS [p1]
    INNER JOIN [Service] AS [s0] ON [p1].[ServiceId] = [s0].[ServiceId]
) AS [t0] ON [p].[ProjectId] = [t0].[ProjectId]
LEFT JOIN [ProjectMember] AS [p2] ON [p].[ProjectId] = [p2].[ProjectId]
WHERE EXISTS (
    SELECT 1
    FROM [ProjectMember] AS [p3]
    WHERE ([p].[ProjectId] = [p3].[ProjectId]) AND ([p3].[UserId] = 15))
ORDER BY [p].[ProjectId], [u].[UserId], [u0].[UserId], [f].[FieldId], [f0].[FacilityId], [w].[WellboreId], [s].[ScopeId], [t].[ProjectId], [t].[DepartmentId], [t].[DepartmentId0], [t0].[ProjectId], [t0].[ServiceId], [t0].[ServiceId0], [p2].[ProjectMemberId]

If it didn't try to SELECT that column, then the query would be correct.

Just to point out, that query would have the same problem even if it was simpler (i.e. Context.Project .Include(x => x.ProjectDepartments) .ThenInclude(x => x.Department).ToList();)

The weird thing is that EVERYTHING else seems to be working fine, the only problem is when using Includes/ThenIncludes in the new entities we had to create to map the N-to-N relationships (for which we had the corresponding DB table already, but we didn't need an entity in EF6).

This is the only thing that's stopping us to finally move to EF Core and we can't figure out what we are doing wrong. I have been trying to find a similar issue but couldn't find anything relevant... this is the only one that was quite similar: https://github.com/dotnet/efcore/issues/4093

It's probably an error on our side, but I don't know where else to look to be honest, so any help will be really appreciated it 😄

DB tables

image

image

image

Further technical details

EF Core version: 3.1.5 Database provider: Microsoft.EntityFrameworkCore.SqlServer Target framework: NET Framework 4.7 Operating system: Windows 10 IDE: Visual Studio 2019 16.3

dglozano commented 4 years ago

Well the problem was on our side. We had some convenience lambda properties in the Project and Department defined like this:

Project.cs

...
public List<Department> Departments => ProjectDepartments?.Select(x => x.Department).ToList() ?? Enumerable.Empty<Department>();
...

Department.cs

...
public List<Project> Projects => ProjectDepartments?.Select(x => x.Project).ToList() ?? Enumerable.Empty<Project>();
...

For some reason, those lambda expressions were trying to add the wrong columns to the model. Adding [NotMapped] to those lambda properties solved our problem.

However, as far as I understand, EF6 ignored lambda properties and didn't map them at all. From what I can see in the EF Core documentation, it seems that get only properties (which I believe are equivalent to lambda properties) shouldn't be mapped either in EF Core. Am I understanding this wrong?

https://docs.microsoft.com/en-us/ef/core/modeling/entity-properties?tabs=data-annotations%2Cwithout-nrt image image

ajcvickers commented 4 years ago

@dglozano These are collection navigation properties. If I remember correctly, read-only collection navigations should be discovered used by EF6 as well as EF Core. That being said, if they should not be mapped, then using [NotMapped] is the right thing to do.

dglozano commented 4 years ago

@ajcvickers I have double checked and I can confirm that in EF6 collection lambda properties are also discovered. Didn't know there was a distinction between collection lambda properties and lambda properties. Thanks for the help 😄 I am closing this.