Nullable navigation properties lead to unexpected outer join criteria

[DerPeit]

Bug description

The attempt to use a nullable navigation property to iterate over related records seems to work only when unfiltered.

Based on the provided code, EF seems to translate the code

context.People
    .Where(person => person.Name == "Name")
    .Select(subject => new
    {
        Name = subject.Name,
        Coworkers = subject.Employer!.Employees!
            .Select(coworker => new { coworker.Name })
            .ToList(),
    })
    .ToList();

to something which narrows down to

SELECT *
FROM People people
LEFT JOIN Employers employers ON employers.EmployerId = people.EmployerId
LEFT JOIN People coworkers ON coworkers.EmployerId = employers.EmployerId
WHERE people.Name = N'Name'

However, when you apply a filter, like

context.People
    .Where(person => person.Name == "Name")
    .Select(subject => new
    {
        Name = subject.Name,
        Coworkers = subject.Employer!.Employees!
            .Where(employee => employee != subject) // changed, compared to above
            .Select(coworker => new { coworker.Name })
            .ToList(),
    })
    .ToList();

this tranlates to a different join criteria:

SELECT *
FROM People people
LEFT JOIN Employers employers ON employers.EmployerId = people.EmployerId
LEFT JOIN People coworkers ON (coworkers.EmployerId = employers.EmployerId OR (coworkers.EmployerId IS NULL AND employers.EmployerId IS NULL)) AND coworkers.PersonId != people.PersonId
WHERE people.Name = N'Name'

Expectation:

• People without employer have an empty list of coworkers even with additional filter criteria.

Observation:

• People without employer have all other people without employer listed as their coworkers.

This is not subject to the SQL Server provider. I was able to reproduce this with the most recent PostgreSQL provider, as well.

Your code

using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Migrations;

public class Employer
{
    public int EmployerId { get; set; }

    public required string Name { get; set; }

    public IList<Person>? Employees { get; set; }
}

public class Person
{
    public int PersonId { get; set; }

    public required string Name { get; set; }

    public int? EmployerId { get; set; }

    public Employer? Employer { get; set; }
}

public sealed class TestContext : DbContext
{
    public DbSet<Employer> Employers { get; set; }

    public DbSet<Person> People { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseSqlServer("Data Source=localhost; Initial Catalog=Test; Integrated Security=True");
    }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        var employerBuilder = modelBuilder.Entity<Employer>();
        var peopleBuilder = modelBuilder.Entity<Person>();

        peopleBuilder.HasKey(x => x.PersonId);
        employerBuilder.HasKey(x => x.EmployerId);
        employerBuilder.HasMany(x => x.Employees).WithOne(x => x.Employer).HasForeignKey(x => x.EmployerId).HasPrincipalKey(x => x.EmployerId);

        employerBuilder.HasData(
            new Employer { EmployerId = 1, Name = "Microsoft" },
            new Employer { EmployerId = 2, Name = "Apple" },
            new Employer { EmployerId = 3, Name = "Google" });

        peopleBuilder.HasData(
            new Person { PersonId = 1, Name = "Satya", EmployerId = 1 },
            new Person { PersonId = 2, Name = "Brad", EmployerId = 1 },
            new Person { PersonId = 3, Name = "Amy", EmployerId = 1 },
            new Person { PersonId = 4, Name = "Tim", EmployerId = 2 },
            new Person { PersonId = 5, Name = "Katherine", EmployerId = 2 },
            new Person { PersonId = 6, Name = "Eddy", EmployerId = 2 },
            new Person { PersonId = 7, Name = "Sundar", EmployerId = 3 },
            new Person { PersonId = 8, Name = "Ruth", EmployerId = 3 },
            new Person { PersonId = 9, Name = "Anat", EmployerId = 3 },
            new Person { PersonId = 10, Name = "Donald", Employer = null },
            new Person { PersonId = 11, Name = "Elon", Employer = null });
    }
}

[TestClass]
public sealed class Reproduction
{
    [TestMethod]
    public void Employees_have_coworkers()
    {
        using var context = new TestContext();

        context.Database.EnsureCreated();

        var satya = context.People
            .Where(person => person.Name == "Satya")
            .Select(satya => new
            {
                Name = satya.Name,
                Coworkers = satya.Employer!.Employees!
                    .Where(employee => employee != satya)
                    .Select(coworker => new { coworker.Name })
                    .ToList(),
            })
            .Single();

        Assert.IsTrue(satya.Coworkers.Count == 2);
        Assert.IsTrue(satya.Coworkers.Any(coworker => coworker.Name == "Brad"));
        Assert.IsTrue(satya.Coworkers.Any(coworker => coworker.Name == "Amy"));
    }

    [TestMethod]
    public void Unemployed_people_do_not_have_coworkers()
    {
        using var context = new TestContext();

        context.Database.EnsureCreated();

        var donald = context.People
            .Where(person => person.Name == "Donald")
            .Select(donald => new
            {
                Name = donald.Name,
                Coworkers = donald.Employer!.Employees!
                    .Where(employee => employee != donald)
                    .Select(coworker => new { coworker.Name })
                    .ToList(),
            })
            .Single();

        Assert.IsTrue(donald.Coworkers.Count == 0); // This fails. Donald and Elon are coworkers.
    }
}

public class TestMigration : Migration
{
    protected override void Up(MigrationBuilder migrationBuilder)
    {
        migrationBuilder.CreateTable(
            name: "Employers",
            columns: table => new
            {
                EmployerId = table.Column<int>("int", false).Annotation("SqlServer:Identity", "1, 1"),
                Name = table.Column<string>("nvarchar(max)", false)
            },
            constraints: table =>
            {
                table.PrimaryKey("PK_Employers", x => x.EmployerId);
            });

        migrationBuilder.CreateTable(
            name: "People",
            columns: table => new
            {
                PersonId = table.Column<int>("int", false).Annotation("SqlServer:Identity", "1, 1"),
                Name = table.Column<string>("nvarchar(max)", false),
                EmployerId = table.Column<int>("int", true)
            },
            constraints: table =>
            {
                table.PrimaryKey("PK_People", x => x.PersonId);
                table.ForeignKey("FK_People_Employers_EmployerId", x => x.EmployerId, "Employers", "EmployerId");
            });

        migrationBuilder.InsertData(
            table: "Employers",
            columns: ["EmployerId", "Name"],
            values: new object[,]
            {
                    { 1, "Microsoft" },
                    { 2, "Apple" },
                    { 3, "Google" }
            });

        migrationBuilder.InsertData(
            table: "People",
            columns: ["PersonId", "EmployerId", "Name"],
            values: new object?[,]
            {
                    { 10, null, "Donald" },
                    { 11, null, "Elon" },
                    { 1, 1, "Satya" },
                    { 2, 1, "Brad" },
                    { 3, 1, "Amy" },
                    { 4, 2, "Tim" },
                    { 5, 2, "Katherine" },
                    { 6, 2, "Eddy" },
                    { 7, 3, "Sundar" },
                    { 8, 3, "Ruth" },
                    { 9, 3, "Anat" }
            });
    }
}

Stack traces

Verbose output

EF Core version

9.0.2

Database provider

Microsoft.EntityFrameworkCore.SqlServer 9.0.2

Target framework

.net 9.0

Operating system

Windows 10

[maumar]

I'm able to reproduce this. Initially we generate the correct condition

WHERE ((e.EmployerId != NULL) && (e.EmployerId == p2.EmployerId)) && (p2.PersonId != p.PersonId)

but in the apply projection step, when adding a join we go through RemoveRedundantNullChecks, which removes the null check.

When we get to nullability processor we have:

SELECT s.Name, s.PersonId, s.EmployerId, p0.Name, p0.PersonId
FROM 
(
    SELECT TOP(2) p.Name, p.PersonId, e.EmployerId
    FROM People AS p
    LEFT JOIN Employers AS e ON p.EmployerId == e.EmployerId
    WHERE p.Name == N'Donald'
) AS s
LEFT JOIN People AS p0 ON (s.EmployerId == p0.EmployerId) && (s.PersonId != p0.PersonId)
ORDER BY s.PersonId ASC, s.EmployerId ASC

in null semantics when we process nullability for join predicate, if the binary expression is not Equal, we expand to c# null semantics. This is to mimic joins on anonymous types, stuff like:

select x from table1
join y from table2 on { Foo = x.Id, Bar = null } equals { Foo = 1, Bar = null }
select ...

When there is no extra condition (.Where(employee => employee != subject) from the example), we end up with a simple join predicate, which should follow relational null semantics and is therefore not expanded.

[maumar]

we could improve the way we process composite join predicates in nullability processor - e.g. we could slash matching IS NULL on both sides (but that leaves comparison to a null-value column, which we can't simplify) We could also make sure that predicate has a structure corresponding to anonymous object comparison ( a = b AND c = d AND ...), but that also doesn't cover all the cases.

alternatively we could keep the initial "redundant" null check and only slash it after null semantics, but need to investigate more whether it is safe