GroupBy aggregate over a required navigation with a query filter drops entire groups (Regression in 11.0)

[benedict-odonovan]

Bug description

When an aggregate over a grouping reaches through a required navigation whose principal has a query filter, the group is silently removed from the results — including its Count(), which never touched the navigation.

Lifting the aggregate into a pre-GroupBy join (#38668) moved the navigation from a correlated subquery in the projection to an INNER JOIN beneath the GROUP BY. A correlated subquery cannot remove a source row; it returns NULL for it. An INNER JOIN onto a filtered subquery can, and does — so a row whose principal is filtered out no longer reaches the grouping, and if that was the group's only row, the group itself never forms.

Northwind with the natural soft-delete filter on products:

modelBuilder.Entity<Product>().HasQueryFilter(p => !p.Discontinued);

Order 10248 has one line, for a live product. Order 10249 has one line, for a discontinued one.

// 1 — Count alone
ctx.OrderDetails.GroupBy(od => od.OrderID)
    .Select(g => new { g.Key, Count = g.Count() })

// 2 — Count plus an aggregate that traverses Product
ctx.OrderDetails.GroupBy(od => od.OrderID)
    .Select(g => new { g.Key, Count = g.Count(), MaxStock = g.Max(x => (int?)x.Product.UnitsInStock) })

// 3 — the same question, traversal moved into the GroupBy element selector
ctx.OrderDetails.GroupBy(od => od.OrderID, od => new { od.Product.UnitsInStock, od.Quantity })
    .Select(g => new { g.Key, Count = g.Count(), MaxStock = g.Max(x => (int?)x.UnitsInStock) })

	10.0 (before #38668)	11.0 rc1 / main
1	2 groups — 10248:Count=1, 10249:Count=1	2 groups — unchanged
2	2 groups — 10248:Count=1,Max=39, 10249:Count=1,Max=null	1 group — 10248:Count=1,Max=39; order 10249 is gone
3	2 groups — same as 10.0 query 2	2 groups — unchanged

Two things stand out beyond the regression itself.

Count changes even though it traverses nothing. Queries 1 and 2 differ only by the presence of a second, unrelated aggregate, and adding it removes a group from the result. Whether order 10249 appears is decided by what a sibling aggregate in the same projection does.

Queries 2 and 3 ask the same question and now disagree. The only difference is whether od.Product is reached from the aggregate lambda or from the element selector, which selects a different translation. Before #38668 they agreed.

Expected behaviour

Query 2 returns both orders, with MaxStock = null for order 10249 — the 10.0 behaviour, and what query 3 still returns today. An aggregate materializes no Product; it reads UnitsInStock and folds it. A filtered-out principal should make the aggregate empty, not delete the order.

SQL

Before #38668 — the traversal is isolated in a correlated subquery, so COUNT(*) sees every line:

SELECT "o"."OrderID" AS "Key", COUNT(*) AS "Count", (
    SELECT MAX("p0"."UnitsInStock")
    FROM "OrderDetails" AS "o0"
    INNER JOIN (
        SELECT "p"."ProductID", "p"."UnitsInStock"
        FROM "Products" AS "p"
        WHERE NOT ("p"."Discontinued")
    ) AS "p0" ON "o0"."ProductID" = "p0"."ProductID"
    WHERE "o"."OrderID" = "o0"."OrderID") AS "MaxStock"
FROM "OrderDetails" AS "o"
GROUP BY "o"."OrderID"

11.0 rc1 / main — the filter is now a join condition beneath the GROUP BY, so it removes rows from the grouping:

SELECT "o"."OrderID" AS "Key", COUNT(*) AS "Count", MAX("p"."UnitsInStock") AS "MaxStock"
FROM "OrderDetails" AS "o"
INNER JOIN "Products" AS "p" ON "o"."ProductID" = "p"."ProductID" AND NOT ("p"."Discontinued")
GROUP BY "o"."OrderID"

The same plan shape is generated on SQL Server.

Relationship to #19801

I'm aware that removing a dependent whose required principal is filtered out is deliberate, and that PossibleIncorrectRequiredNavigationWithQueryFilterInteractionWarning exists for exactly this model. I don't think that covers this case:

• The rationale in Query: add validation to warn about required navigations pointing to entity with query filter #19801 is referential integrity — a required principal that isn't there can't be materialized. Nothing is materialized here; the aggregate reads a column.
• It doesn't explain why Count(), which never traverses the navigation, changes its answer.
• It doesn't explain why queries 2 and 3 disagree, nor why query 2's behaviour changed in 11.0 while query 3's did not.
• The suggested remedy — matching filters on both entities — changes the intent, since it also removes those lines from Count. It cannot express "count every line, and report the stock figure as unavailable", which is what 10.0 returned.

Your code

using Microsoft.EntityFrameworkCore;

using var ctx = new NorthwindContext();
ctx.Database.EnsureCreated();

ctx.AddRange(
    new Product { ProductID = 1, ProductName = "Chai", Discontinued = false, UnitsInStock = 39 },
    new Product { ProductID = 2, ProductName = "Mishi Kobe Niku", Discontinued = true, UnitsInStock = 29 },
    new Order { OrderID = 10248 },
    new Order { OrderID = 10249 },
    new OrderDetail { OrderID = 10248, ProductID = 1, Quantity = 12 },
    new OrderDetail { OrderID = 10249, ProductID = 2, Quantity = 9 });
ctx.SaveChanges();
ctx.ChangeTracker.Clear();

var q1 = ctx.OrderDetails.GroupBy(od => od.OrderID)
    .Select(g => new { g.Key, Count = g.Count() })
    .OrderBy(x => x.Key).ToList();

var q2 = ctx.OrderDetails.GroupBy(od => od.OrderID)
    .Select(g => new { g.Key, Count = g.Count(), MaxStock = g.Max(x => (int?)x.Product.UnitsInStock) })
    .OrderBy(x => x.Key).ToList();

var q3 = ctx.OrderDetails.GroupBy(od => od.OrderID, od => new { od.Product.UnitsInStock, od.Quantity })
    .Select(g => new { g.Key, Count = g.Count(), MaxStock = g.Max(x => (int?)x.UnitsInStock) })
    .OrderBy(x => x.Key).ToList();

Console.WriteLine($"1) {q1.Count} groups");   // 2
Console.WriteLine($"2) {q2.Count} groups");   // 1 on 11.0, 2 on 10.0
Console.WriteLine($"3) {q3.Count} groups");   // 2

public class Product
{
    public int ProductID { get; set; }
    public string ProductName { get; set; } = null!;
    public bool Discontinued { get; set; }
    public short UnitsInStock { get; set; }
}

public class Order
{
    public int OrderID { get; set; }
}

public class OrderDetail
{
    public int OrderID { get; set; }
    public int ProductID { get; set; }
    public short Quantity { get; set; }
    public Order Order { get; set; } = null!;
    public Product Product { get; set; } = null!;
}

public class NorthwindContext : DbContext
{
    private readonly Microsoft.Data.Sqlite.SqliteConnection _connection = new("DataSource=:memory:");

    public DbSet<Product> Products => Set<Product>();
    public DbSet<Order> Orders => Set<Order>();
    public DbSet<OrderDetail> OrderDetails => Set<OrderDetail>();

    public NorthwindContext() => _connection.Open();

    protected override void OnConfiguring(DbContextOptionsBuilder b)
        => b.UseSqlite(_connection);

    protected override void OnModelCreating(ModelBuilder mb)
    {
        mb.Entity<Product>().HasQueryFilter(p => !p.Discontinued);

        mb.Entity<OrderDetail>().HasKey(od => new { od.OrderID, od.ProductID });
        mb.Entity<OrderDetail>().HasOne(od => od.Order).WithMany().HasForeignKey(od => od.OrderID);
        mb.Entity<OrderDetail>().HasOne(od => od.Product).WithMany().HasForeignKey(od => od.ProductID);
    }

    public override void Dispose()
    {
        base.Dispose();
        _connection.Dispose();
    }
}

Stack traces

Verbose output

EF Core version

11.0.0-rc.1

Database provider

No response

Target framework

No response

Operating system

No response

IDE

No response

[benedict-odonovan]

The join type is decided in ExpandForeignKey:

var innerJoin = !entityReference.IsOptional
    && !derivedTypeConversion
    && onDependent
    && foreignKey.IsEffectivelyRequired();

A required FK guarantees a matching row; it doesn't guarantee that row survives a query filter, so the inner join isn't safe when the principal is filtered. Either the lift emits an outer join in that case (MAX/SUM/AVG ignore the nulls, so the aggregate values match the correlated subquery, and no rows are lost), or TryLiftAggregatesOverNavigations declines to lift when a filtered principal is in the path and keeps the 10.0 translation.

[benedict-odonovan]

@AndriySvyryd the approach in #38973 looks right for 11.0 — keeping the correlated subquery is the safe answer for a GA regression, and tracking requiredPath across the chain handles the optional-hop case that a per-navigation check would over-bail on.

For main, would you be open to restoring the optimization for this shape rather than giving it up? The join doesn't have to be an inner join: where the navigation is required and the principal is filtered, emitting an outer join keeps every source row, so no group is lost, and MAX/SUM/AVG/COUNT fold the resulting nulls to the same values the correlated subquery produces:

SELECT [l].[OrderId] AS [Key], COUNT(*) AS [Count], MAX([p0].[UnitsInStock]) AS [MaxStock]
FROM [Line] AS [l]
LEFT JOIN (
    SELECT [p].[Id], [p].[UnitsInStock]
    FROM [Product] AS [p]
    WHERE [p].[Discontinued] = CAST(0 AS bit)
) AS [p0] ON [l].[ProductId] = [p0].[Id]
GROUP BY [l].[OrderId]

I compared every aggregate kind that lifts against the correlated-subquery translation on the same data and they agree, SUM's ISNULL(..., 0) for an empty aggregate included.

(One carve-out: All() still has to decline. All() over an empty set is true, so the subquery answers true for a group whose only rows have a filtered-out principal, while any join — inner or outer — answers false.)

I have this working against main with tests, including one pinning that a joined principal's own query filter keeps its inner join: the outer join has to be scoped to the aggregate selectors, or an entity can pass its own filter when reached from an aggregate. Happy to open an issue and put up a PR once #38973 merges forward, if that's worth having.

[AndriySvyryd]

@benedict-odonovan Sounds good, you can open a PR after #38982 is merged. Include the tests for all aggregate kinds and other scenarios that #38668 might have regressed (I am not an expert on GroupBy, so I am having trouble coming up with good test cases for it).