Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate better SQL for table splitting with required dependent #15521

Closed
oroumand opened this issue Apr 29, 2019 · 10 comments
Closed

Generate better SQL for table splitting with required dependent #15521

oroumand opened this issue Apr 29, 2019 · 10 comments

Comments

@oroumand
Copy link

oroumand commented Apr 29, 2019

When load Parent And Child Entity in Table Splitting in EF Core before version 3.0 the select query is simple row select but when upgrade to ef core 3.0 the query contains self join and I think it's odd.
check this code please with EF core 3 and 2

[Table("Employee")]
    public class Employee
    {
        [Key]
        public int Id { get; set; }
        public string Name { get; set; }
        public bool IsDeleted { get; set; }
        [ForeignKey("Id")]
        public EmployeeDetail EmployeeDetail { get; set; }
    }

    [Table("Employee")]
    public class EmployeeDetail
    {
        [Key]
        public int Id { get; set; }
        public string EmailAddress { get; set; }
        public string MobileNumber { get; set; }
        public string AlternateNumber { get; set; }
        public Employee Employee { get; set; }
    }
    public class EntityModelContext : DbContext
    {
        public DbSet<Employee> Employees { get; set; }
        public DbSet<EmployeeDetail> EmployeeDetails { get; set; }
        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Employee>().HasOne(p => p.EmployeeDetail).WithOne(p => p.Employee).HasForeignKey<Employee>(p => p.Id);
            modelBuilder.Entity<EmployeeDetail>().HasOne(p => p.Employee).WithOne(p => p.EmployeeDetail).HasForeignKey<EmployeeDetail>(p => p.Id);
            modelBuilder.Entity<Employee>().ToTable("Employee");

            base.OnModelCreating(modelBuilder);
        }
        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder.UseSqlServer(@"cnn string");

        }
    }
    class Program
    {
        static void Main(string[] args)
        {
            using (EntityModelContext context = new EntityModelContext())
            {
                Console.WriteLine("------------- Employee Entity -------------");
                var data3 = context.Employees.TagWith("V3 Query").Include(p => p.EmployeeDetail).ToList();
                Console.ReadLine();
            }
        }
    }

the sql query for ef core 2 is

SELECT [e].[Id], [e].[IsDeleted], [e].[Name], [e].[Id], [e].[AlternateNumber], [e].[EmailAddress], [e].[MobileNumber]
FROM [Employee] AS [e]

and for ef core 3 is

SELECT [e].[Id], [e].[IsDeleted], [e].[Name], [t].[Id], [t].[AlternateNumber], [t].[EmailAddress], [t].[MobileNumber]
FROM [Employee] AS [e]
LEFT JOIN (
    SELECT [e.EmployeeDetail].*
    FROM [Employee] AS [e.EmployeeDetail]
    WHERE [e.EmployeeDetail].[MobileNumber] IS NOT NULL OR ([e.EmployeeDetail].[EmailAddress] IS NOT NULL OR [e.EmployeeDetail].[AlternateNumber] IS NOT NULL)
) AS [t] ON [e].[Id] = [t].[Id]

Further technical details

EF Core version: 3.0.0-preview4.19216.3
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 10
IDE: Visual Studio 2017 15.7.4 & Visual Studio 2019

@smitpatel
Copy link
Member

This is because of #9005
In EF Core 2, every Employee always had EmployeeDetail. In EF Core 3, it is optional so we have to generate Left join.

@divega
Copy link
Contributor

divega commented Apr 29, 2019

Note from triage: As mentioned above, making dependents optional in table splitting scenarios, required EF Core to start reasoning about the columns mapped to the two entities separately, no matter if they are actually located in the same table.

Also, there are new conditions added to the query to make sure that none of the values of the columns mapped to non-nullable properties is null, which we use to decide whether the dependent entity should be materialized.

In the future we would go back to generating simpler SQL, but we need to implement this as a SQL generation optimization.

Moving to backlog.

@divega divega added this to the Backlog milestone Apr 29, 2019
@smitpatel
Copy link
Member

Note to implementer: Client side materializer has to figure out when to materialize owned entity since Id would be always there when additional join is removed. That may require converting where predicate on client side.

@oroumand
Copy link
Author

When I check Execution Plane of two queries; query cost of first one is about 28% and the second one is 72%.
I add another entity to my table and query cost of first one is about 4% and the second one is 96%. I think this method for implementing optional dependent is not good and we need to implement this requirement when materializing entities not in SQL Query .

@AndriySvyryd
Copy link
Member

AndriySvyryd commented Aug 22, 2019

Blocked by #12100

@AndriySvyryd
Copy link
Member

We can't do the check on the client in the general case as we also need to check whether there are any dependents in the database

@AndriySvyryd AndriySvyryd changed the title Bad query generates for table splitting Ef Core 3.0.0-preview4.19216.3 Generate better SQL for table splitting with required dependent Nov 27, 2019
@AndriySvyryd AndriySvyryd removed this from the Backlog milestone Aug 13, 2020
@AndriySvyryd
Copy link
Member

@smitpatel Is this fixed now?

@smitpatel
Copy link
Member

Generated query in current master

SELECT [e].[Id], [e].[IsDeleted], [e].[Name], [t].[Id], [t].[AlternateNumber], [t].[EmailAddress], [t].[MobileNumber]
FROM [Employee] AS [e]
LEFT JOIN (
    SELECT [e0].[Id], [e0].[AlternateNumber], [e0].[EmailAddress], [e0].[MobileNumber]
    FROM [Employee] AS [e0]
    INNER JOIN [Employee] AS [e1] ON [e0].[Id] = [e1].[Id]
) AS [t] ON [e].[Id] = [t].[Id]

Observations

  • The model configuration is invalid. It is trying to configuration same navigation pair twice with different side being principal. Further the ForeignKeyAttribute also causes more ambiguity.
  • With somewhat correct model and required dependent we generated above SQL.
  • We don't eliminate join in all cases of table splitting. Currently that happens only for owned types.
  • With owned types and required dependent generated SQL is following
SELECT [e].[Id], [e].[IsDeleted], [e].[Name], [e].[EmployeeDetail_AlternateNumber], [e].[EmployeeDetail_EmailAddress], [e].[EmployeeDetail_Id], [e].[EmployeeDetail_MobileNumber]
FROM [Employee] AS [e]");

@AndriySvyryd
Copy link
Member

@smitpatel Join elimination is covered by #18869, right?

@smitpatel
Copy link
Member

Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants