Suboptimal SQL generation for query with optional navigation, its collection navigation and lateral join

[imb590]

Consider the following code:

using System.Collections.Generic;
using System.ComponentModel.DataAnnotations.Schema;
using System.Linq;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;

using var db = new Context();

var result = db.Set<EntityOne>()
    .Select(o => new
    {
        Ids = o.Two!.Children!.Select(c => new { ChildId = c.Id, ParentId = o.Two.Id }),
    });

System.Console.WriteLine(result.ToQueryString());

public class Context : DbContext
{
    private static ILoggerFactory LoggerFactory => new ServiceCollection().AddLogging(l => l.AddConsole().SetMinimumLevel(LogLevel.Trace)).BuildServiceProvider().GetRequiredService<ILoggerFactory>();

    public DbSet<EntityOne> Ones => Set<EntityOne>();
    public DbSet<EntityTwo> Twos => Set<EntityTwo>();

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder
            .UseNpgsql("Host=stub")
            .EnableSensitiveDataLogging()
            .UseLoggerFactory(LoggerFactory);
    }
}

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

    public int? TwoId { get; set; }

    [ForeignKey(nameof(TwoId))]
    public EntityTwo? Two { get; set; }
}

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

    public int? ParentId { get; set; }

    [ForeignKey(nameof(ParentId))]
    public EntityTwo? Parent { get; set; }

    [InverseProperty(nameof(Parent))]
    public ICollection<EntityTwo>? Children { get; set; }
}

The generated SQL is:

SELECT o."Id", t."Id", t0."ChildId", t0."ParentId"
FROM "Ones" AS o
LEFT JOIN "Twos" AS t ON o."TwoId" = t."Id"
LEFT JOIN LATERAL (
    SELECT t1."Id" AS "ChildId", t."Id" AS "ParentId"
    FROM "Twos" AS t1
    WHERE (t."Id" = t1."ParentId") OR ((t."Id" IS NULL) AND (t1."ParentId" IS NULL))
) AS t0 ON TRUE
ORDER BY o."Id", t."Id"

It is very suboptimal, since due to the unnecessary OR ((t."Id" IS NULL) AND (t1."ParentId" IS NULL)) clause in the inner query, it will iterate over all EntityTwo having ParentId = null for every EntityOne with TwoId = null, creating a new output row on every iteration.

It's possible to get rid of the iteration in the database with a workaround like:

db.Set<EntityOne>()
    .Select(o => new
    {
        Ids = o.Two!.Children!.Where(c => c.ParentId != null).Select(c => new { ChildId = c.Id, ParentId = o.Two.Id }),
    });

provider and version information

EF Core version: 6.0.1
Database provider: Npgsql.EntityFrameworkCore.PostgreSQL 6.0.2
Target framework: .NET 6

[roji]

Can you please reopen this in https://github.com/dotnet/efcore? This is a general SQL generation question rather than anything specific to Npgsql.

[imb590]

isn't this already in https://github.com/dotnet/efcore?

[roji]

Oh apologies, I misread the repo name, going a bit too quickly here.

[smitpatel]

cc: @maumar

[maumar]

when nav expansion is rewriting the collection navigation we add a null check for the outer Id being not null

DbSet<EntityOne>()
    .LeftJoin(
        inner: DbSet<EntityTwo>(), 
        outerKeySelector: e => EF.Property<int?>(e, "TwoId"), 
        innerKeySelector: e0 => EF.Property<int?>(e0, "Id"), 
        resultSelector: (o, i) => new TransparentIdentifier<EntityOne, EntityTwo>(
            Outer = o, 
            Inner = i
        ))
    .Select(e => new { Ids = DbSet<EntityTwo>()
        .Where(e1 => EF.Property<int?>(e.Inner, "Id") != null && object.Equals(
            objA: (object)EF.Property<int?>(e.Inner, "Id"), 
            objB: (object)EF.Property<int?>(e1, "ParentId")))
        .Select(e1 => new { 
            ChildId = e1.Id, 
            ParentId = e.Inner.Id
         }) })

but we get rid of it (as a redundant null check) when we translate to JOIN/APPLY. So when null semantics rewriter processes the query it applies the OR ((t."Id" IS NULL) AND (t1."ParentId" IS NULL)) term because both sides are nullable. If we kept the null check as it was, we won't need to add them (since we do simplified null semantics here) and the result query would look something like this:

SELECT [o].[Id], [t].[Id], [t0].[ChildId], [t0].[ParentId]
FROM [Ones] AS [o]
LEFT JOIN [Twos] AS [t] ON [o].[TwoId] = [t].[Id]
OUTER APPLY (
    SELECT [t1].[Id] AS [ChildId], [t].[Id] AS [ParentId]
    FROM [Twos] AS [t1]
    WHERE [t].[Id] IS NOT NULL AND [t].[Id] = [t1].[ParentId]
) AS [t0]
ORDER BY [o].[Id], [t].[Id]

suggestion: we should keep the null check if we can't convert from APPLY to JOIN

[maumar]

note: this is data corruption, but not a regression (ef5 had the bug already). @ajcvickers @smitpatel thoughts on patching this?

[smitpatel]

It should be easy fix by just remembering the earlier predicate rather than recreating it from join predicate extraction. Due to low risk and data corruption, we should patch this. Let's discuss in team meeting.