MinBy MaxBy support

[henriquewr]

fixes #25566

MinBy(x => x.Id) is lowered to OrderBy(x => x.Id).First/OrDefault()

MaxBy(x => x.Id) is lowered to OrderByDesceding(x => x.Id).First/OrDefault()

To comply with LINQ contract, the First method is determined by the nullability of the source, if source is not nullable, First would be selected, otherwise FirstOrDefault

new List<int>().MaxBy(x => x); // throws System.InvalidOperationException: 'Sequence contains no elements'

new List<int?>().MaxBy(x => x); // returns null

new List<Type /*just any reference type*/>().MaxBy(x => x); // returns null

The original proposal #25566 (comment), has the idea of supporting anonymous types, but, as discussed bellow, supporting anonymous types for MaxBy/MinBy seems to not be a great idea, primarely because that isn't supported in LINQ to Objects

new List<int>() { 1, 2, 3 }.MaxBy(x => new { x }); // throws System.ArgumentException: 'At least one object must implement IComparable.'

[Copilot]

Pull request overview

This pull request adds support for SQL translation of .NET 6's MinBy/MaxBy LINQ methods to Entity Framework Core, addressing issue #25566. The implementation lowers MinBy/MaxBy operations to OrderBy/OrderByDescending followed by First/FirstOrDefault, with special handling for complex selectors including anonymous types.

Changes:

• Added MinBy/MaxBy query method normalization that transforms them into OrderBy/OrderByDescending + First/FirstOrDefault patterns
• Implemented async extension methods (MinByAsync/MaxByAsync) for IQueryable
• Added comprehensive test coverage across SQL Server, InMemory, and Cosmos providers
• Included support for anonymous type selectors by expanding them into ThenBy/ThenByDescending chains

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/EFCore/Query/Internal/QueryableMethodNormalizingExpressionVisitor.cs	Core implementation that transforms MinBy/MaxBy calls into OrderBy+First patterns with special handling for anonymous type selectors
src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.ExpressionVisitors.cs	Minor optimization to pre-allocate list capacity
src/EFCore/Extensions/EntityFrameworkQueryableExtensions.cs	Added MinByAsync and MaxByAsync extension methods with XML documentation
src/EFCore/Query/QueryableMethods.cs	Added MinBy and MaxBy MethodInfo properties
src/Shared/EnumerableMethods.cs	Added MinBy and MaxBy MethodInfo properties
test/EFCore.Specification.Tests/TestUtilities/QueryAsserter.cs	Added AssertMinBy and AssertMaxBy test assertion helpers (contains dead code issue)
test/EFCore.Specification.Tests/TestUtilities/ExpectedQueryRewritingVisitor.cs	Added query rewriting logic for expected results with anonymous type selectors
test/EFCore.Specification.Tests/Query/QueryTestBase.cs	Added AssertMinBy and AssertMaxBy helper methods
test/EFCore.Specification.Tests/Query/NorthwindGroupByQueryTestBase.cs	Added specification tests for MinBy/MaxBy in GroupBy scenarios and fixed #region formatting
test/EFCore.Specification.Tests/Query/NorthwindAggregateOperatorsQueryTestBase.cs	Added comprehensive specification tests for MinBy/MaxBy including edge cases
test/EFCore.SqlServer.FunctionalTests/Query/NorthwindGroupByQuerySqlServerTest.cs	Added SQL Server specific tests and SQL baselines (contains test method call bug)
test/EFCore.SqlServer.FunctionalTests/Query/NorthwindAggregateOperatorsQuerySqlServerTest.cs	Added SQL Server specific tests and SQL baselines (contains test method call bug)
test/EFCore.InMemory.FunctionalTests/Query/NorthwindAggregateOperatorsQueryInMemoryTest.cs	Added InMemory provider specific tests
test/EFCore.Cosmos.FunctionalTests/Query/NorthwindAggregateOperatorsQueryCosmosTest.cs	Added Cosmos provider specific tests (contains test method call bug)

[roji]

Thanks, this looks pretty close to merge. See mainly thoughts about the anonymous type keys.

[roji]

Assuming we remove support for anonymous type keys (see comment above), this can all go away I think.

[henriquewr]

I just removed the support for anonymous type keys (see comment above)

[roji]

Isn't this scenario always going to fail with this error, not just for relational providers (where this currently is)? If so move the Assert.ThrowsAsync to the base implementation.

[henriquewr]

Yes, it would fail for every provider

But the exception is different between InMemory ("Sequence contains no elements") and Relational ("Nullable object must have a value.")

Actually, I just followed the same pattern as the Min/Max tests in the subqueries.

public virtual Task Min_no_data_subquery(bool async)
    => AssertQueryScalar(
        async,
        ss => ss.Set<Customer>().Select(c => c.Orders.Where(o => o.OrderID == -1).Min(o => o.OrderID)));

Relational overrides for

public override async Task Min_no_data_subquery(bool async)
    => Assert.Equal(
        "Nullable object must have a value.",
        (await Assert.ThrowsAsync<InvalidOperationException>(() => base.Min_no_data_subquery(async))).Message);

InMemory overrides for

    public override async Task Min_no_data_subquery(bool async)
        => Assert.Equal(
            "Sequence contains no elements",
            (await Assert.ThrowsAsync<InvalidOperationException>(() => base.Min_no_data_subquery(async))).Message);

[roji]

Hmm, OK - we should probably standardize here a bit but if Min/Max are doing the same we can continue that pattern for now.

[roji]

Thanks, LGTM!

One note... We'll soon likely start moving away from this sort preprocessing normalization approach, where we transform LINQ operators as you're doing; the main issue with this approach is that it requires MakeGenericMethod(), which is incompatible with NativeAOT.

The other theoretical problem with this approach is that if there's ever a provider which happens to support translating Min/MaxBy directly (not as OrderBy+FirstOrDefault), that provider now needs to pattern-match a more complex, lowered version of the original, higher-level concept expressed by the user.

The alternative normalization technique would move this to the translation phase, in QueryableMethodTranslatingExpressionVisitor: rather than transform the tree in advance, we'd leave things as is, and handle Min/MaxBy in translation by calling TranslateOrderBy and then TranslateFirst. That would be compatible with NativeAOT as we don't need to create intermediate pre-translation nodes, and would also allow providers to override TranslateMaxBy if they wish to do a special translation.

But all that is for the future; in the meantime the approach in this PR aligns with how we do things elsewhere and show be completely fine.