From 120a2695d201e7fdecf53bd502b75999d5e27ccf Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Wed, 4 Nov 2026 11:41:11 +1100 Subject: [PATCH] Order collection navigations loaded by a Select projection A collection navigation read inside a Select projection is loaded by a subquery of its own. Nothing ordered it, so EF Core returned its rows in key order and the configured default ordering was ignored. Only Include() collections got it. ProjectionOrderingApplicator orders each unordered collection navigation in a Select lambda by its entity's default, after any Where and before the operators that project the entity away or choose which rows come back. A navigation bound into the result as it is is ordered too. Collections only aggregated, or composed with an operator EF Core drops ordering beneath such as Distinct, are left alone. Explicit ordering in a projection is kept and checked by redundant ordering detection. Version bumped to 1.4.0. --- readme.md | 35 +- src/Directory.Build.props | 2 +- src/EfOrderBy/GlobalUsings.cs | 1 + src/EfOrderBy/Interceptor.cs | 16 +- src/EfOrderBy/ProjectionOrderingApplicator.cs | 353 ++++++++++++++++++ src/EfOrderBy/QueryAnalyzer.cs | 13 +- ...ludeAndSelect_WithNavInSelect.verified.txt | 2 +- ...gateInProjection_IsNotOrdered.verified.txt | 15 + ...avigation_AppliesDefaultOrder.verified.txt | 79 ++++ ...tIsNotANavigation_IsLeftAlone.verified.txt | 11 + ...inctInProjection_IsNotOrdered.verified.txt | 8 + ...itOrderingInProjection_IsKept.verified.txt | 40 ++ ...dCollection_OrdersAfterFilter.verified.txt | 44 +++ ...rojection_AppliesDefaultOrder.verified.txt | 27 ++ ...ions_EachGetTheirDefaultOrder.verified.txt | 75 ++++ ...ollection_AppliesDefaultOrder.verified.txt | 68 ++++ ...InProjection_OrdersBeforeTake.verified.txt | 50 +++ src/Tests/ProjectionOrderingTests.cs | 248 ++++++++++++ src/Tests/RedundantOrderByTests.cs | 84 +++++ src/Tests/Snippets.cs | 22 ++ 20 files changed, 1180 insertions(+), 13 deletions(-) create mode 100644 src/EfOrderBy/ProjectionOrderingApplicator.cs create mode 100644 src/Tests/ProjectionOrderingTests.AggregateInProjection_IsNotOrdered.verified.txt create mode 100644 src/Tests/ProjectionOrderingTests.BoundNavigation_AppliesDefaultOrder.verified.txt create mode 100644 src/Tests/ProjectionOrderingTests.CollectionThatIsNotANavigation_IsLeftAlone.verified.txt create mode 100644 src/Tests/ProjectionOrderingTests.DistinctInProjection_IsNotOrdered.verified.txt create mode 100644 src/Tests/ProjectionOrderingTests.ExplicitOrderingInProjection_IsKept.verified.txt create mode 100644 src/Tests/ProjectionOrderingTests.FilteredCollection_OrdersAfterFilter.verified.txt create mode 100644 src/Tests/ProjectionOrderingTests.FirstOrDefaultInProjection_AppliesDefaultOrder.verified.txt create mode 100644 src/Tests/ProjectionOrderingTests.NestedCollections_EachGetTheirDefaultOrder.verified.txt create mode 100644 src/Tests/ProjectionOrderingTests.ProjectedCollection_AppliesDefaultOrder.verified.txt create mode 100644 src/Tests/ProjectionOrderingTests.TakeInProjection_OrdersBeforeTake.verified.txt create mode 100644 src/Tests/ProjectionOrderingTests.cs diff --git a/readme.md b/readme.md index 131fca4..b6542a4 100644 --- a/readme.md +++ b/readme.md @@ -16,6 +16,7 @@ https://nuget.org/packages/EfOrderBy/ - **Automatic ordering**: Queries without explicit `OrderBy` automatically use configured default ordering - **Include() support**: Nested collections in `.Include()` expressions are automatically ordered +- **Projection support**: Collection navigations loaded by a `.Select()` projection are automatically ordered - **Deterministic paging**: Ordering is applied before `Skip`/`Take`/`First`, so pages and single results are stable - **Inheritance support**: Ordering configured on a base entity type is automatically inherited by derived types (TPH) - **Fluent configuration**: Configure default ordering using the familiar EF Core fluent API @@ -132,6 +133,34 @@ var departments = await context.Departments +## Projection Support + +A collection navigation read inside a `Select` projection is loaded by a subquery of its own, and is ordered the same way as one in an `Include`: + + + +```cs +// Departments ordered by DisplayOrder +// Employees ordered by HireDate, then Salary descending +var departments = await context.Departments + .Select(_ => new + { + _.Name, + Employees = _.Employees + .Where(employee => employee.Salary > 0) + .Select(employee => employee.Name) + .ToList() + }) + .ToListAsync(); +``` +snippet source | anchor + + +The ordering goes after any `Where` filters and before the operators that project the entity away or choose which rows come back: `Select`, `Skip`, `Take`, `First`, `FirstOrDefault`, `Last`, `LastOrDefault`, `ElementAt`, `ElementAtOrDefault`, `ToList`, `ToArray` and `AsEnumerable`. A navigation bound into the result as it is, such as `new { _.Name, _.Employees }`, is ordered too. + +A collection whose order cannot reach the result is left alone: one that is only aggregated, with `Count`, `Any`, `Sum` and the like, or one composed with an operator EF Core drops ordering beneath, such as `Distinct`. Explicit ordering inside the projection is kept, and is checked by [redundant ordering detection](#detect-redundant-ordering) the same way as ordering inside an `Include`. + + ## Inheritance Support When using TPH (Table Per Hierarchy) inheritance, ordering configured on a base entity type is automatically inherited by derived types. This eliminates the need to duplicate `.OrderBy()` on every derived type. @@ -182,7 +211,7 @@ public class InheritanceDbContext : DbContext public DbSet DerivedEntitiesB => Set(); } ``` -snippet source | anchor +snippet source | anchor Behavior: @@ -205,7 +234,7 @@ builder.Entity() .ThenBy(_ => _.Name) .ThenByDescending(_ => _.Price); ``` -snippet source | anchor +snippet source | anchor @@ -548,7 +577,7 @@ public class AppDbContext : DbContext public DbSet Employees => Set(); } ``` -snippet source | anchor +snippet source | anchor diff --git a/src/Directory.Build.props b/src/Directory.Build.props index d76e846..7463b7d 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -5,7 +5,7 @@ enable latest true - 1.3.0 + 1.4.0 1.0.0 true true diff --git a/src/EfOrderBy/GlobalUsings.cs b/src/EfOrderBy/GlobalUsings.cs index e79f46f..6f403cc 100644 --- a/src/EfOrderBy/GlobalUsings.cs +++ b/src/EfOrderBy/GlobalUsings.cs @@ -1,4 +1,5 @@ global using System.Collections.Concurrent; +global using System.Diagnostics.CodeAnalysis; global using System.Linq.Expressions; global using EfOrderBy; global using Microsoft.EntityFrameworkCore; diff --git a/src/EfOrderBy/Interceptor.cs b/src/EfOrderBy/Interceptor.cs index fee34d9..fc73eda 100644 --- a/src/EfOrderBy/Interceptor.cs +++ b/src/EfOrderBy/Interceptor.cs @@ -24,18 +24,24 @@ public Expression QueryCompilationStarting(Expression query, QueryExpressionEven var detectRedundantOrdering = extension?.ThrowOnRedundantOrderBy ?? OrderBySettings.ThrowOnRedundantOrderBy; - // Analyze the query for ordering and includes in a single pass - var (hasOrdering, hasInclude) = QueryAnalyzer.Analyze(query); + // Analyze the query for ordering, includes and projections in a single pass + var (hasOrdering, hasInclude, hasSelect) = QueryAnalyzer.Analyze(query); var queryWithOrderedIncludes = query; // Ordering nested collections rewrites the whole tree, so only do it when there is an - // Include to rewrite. Neither flag above can change as a result, since the rewrite only - // touches the inside of Include lambdas. + // Include or a projection to rewrite. None of the flags above can change as a result, since + // the rewrites only touch the inside of Include and Select lambdas. if (hasInclude) { var visitor = new IncludeOrderingApplicator(model, detectRedundantOrdering); - queryWithOrderedIncludes = visitor.Visit(query); + queryWithOrderedIncludes = visitor.Visit(queryWithOrderedIncludes); + } + + if (hasSelect) + { + var visitor = new ProjectionOrderingApplicator(model, detectRedundantOrdering); + queryWithOrderedIncludes = visitor.Visit(queryWithOrderedIncludes); } if (hasOrdering) diff --git a/src/EfOrderBy/ProjectionOrderingApplicator.cs b/src/EfOrderBy/ProjectionOrderingApplicator.cs new file mode 100644 index 0000000..c4f8695 --- /dev/null +++ b/src/EfOrderBy/ProjectionOrderingApplicator.cs @@ -0,0 +1,353 @@ +/// +/// Visitor that applies default ordering to the collection navigations a Select projection loads. +/// +/// +/// A collection navigation read inside a projection, for example +/// Select(_ => new { Employees = _.Employees.Where(...).Select(...).ToList() }), is loaded by a +/// subquery of its own. Without ordering, EF Core returns that subquery's rows in key order, so +/// the default ordering configured for the collection's entity was ignored, the same as it was for +/// an Include before IncludeOrderingApplicator. +/// The ordering is placed after any Where filters and before the operators that project the entity +/// away or choose which rows come back, mirroring how the outer query is ordered. A collection +/// whose order cannot affect the result, such as one only counted or tested with Any, is left as +/// it is, as is one composed with an operator the ordering could not be placed beneath. +/// +sealed class ProjectionOrderingApplicator(IModel model, bool detectRedundantOrdering) : + ExpressionVisitor +{ + static readonly ConcurrentDictionary toListMethodCache = new(); + + static readonly MethodInfo toListMethod = typeof(Enumerable) + .GetMethod(nameof(Enumerable.ToList))!; + + // Lambdas of the Select projections currently being visited. Ordering is only applied inside one, + // since that is where a collection is loaded into the result. A collection read in a Where or + // OrderBy lambda of the outer query only filters or sorts it. + int projectionDepth; + + protected override Expression VisitMethodCall(MethodCallExpression node) + { + if (IsQueryableSelect(node)) + { + return VisitSelect(node); + } + + if (projectionDepth > 0 && + TryFindNavigationChain(node, out var navigation)) + { + return ProcessChain(node, navigation); + } + + return base.VisitMethodCall(node); + } + + static bool IsQueryableSelect(MethodCallExpression node) => + node.Method.DeclaringType == typeof(Queryable) && + node.Method.Name == nameof(Queryable.Select) && + node.Arguments.Count == 2 && + node.Arguments[1] is UnaryExpression { Operand: LambdaExpression }; + + Expression VisitSelect(MethodCallExpression node) + { + var source = Visit(node.Arguments[0]); + + var quote = (UnaryExpression) node.Arguments[1]; + projectionDepth++; + Expression selector; + try + { + selector = Visit(quote); + } + finally + { + projectionDepth--; + } + + return node.Update(node.Object, [source, selector]); + } + + // A navigation bound into the result as it is, for example new { _.Employees }, is loaded the + // same way as one read through a chain of operators, so it is ordered as well. It is replaced + // by the ordered navigation copied into a list, which only fits where a list can stand in for + // the navigation's declared type. + protected override MemberAssignment VisitMemberAssignment(MemberAssignment node) + { + var visited = base.VisitMemberAssignment(node); + if (projectionDepth == 0) + { + return visited; + } + + return visited.Update(OrderBoundNavigation(visited.Expression)); + } + + protected override Expression VisitNew(NewExpression node) + { + var visited = (NewExpression) base.VisitNew(node); + if (projectionDepth == 0) + { + return visited; + } + + Expression[]? arguments = null; + for (var index = 0; index < visited.Arguments.Count; index++) + { + var argument = visited.Arguments[index]; + var ordered = OrderBoundNavigation(argument); + if (ReferenceEquals(ordered, argument)) + { + continue; + } + + arguments ??= visited.Arguments.ToArray(); + arguments[index] = ordered; + } + + if (arguments == null) + { + return visited; + } + + return visited.Update(arguments); + } + + Expression OrderBoundNavigation(Expression expression) + { + if (expression is not MemberExpression member || + FindCollectionNavigation(member) is not { } navigation || + GetConfiguration(navigation) is not { } configuration) + { + return expression; + } + + var elementType = navigation.TargetEntityType.ClrType; + var listType = typeof(List<>).MakeGenericType(elementType); + if (!expression.Type.IsAssignableFrom(listType)) + { + return expression; + } + + var toList = toListMethodCache.GetOrAdd( + elementType, + static _ => toListMethod.MakeGenericMethod(_)); + return Expression.Call(toList, AppendOrdering(expression, configuration)); + } + + /// + /// Whether is the outermost operator of a chain over a collection + /// navigation, such as _.Employees.Where(...).Select(...).ToList(). + /// + bool TryFindNavigationChain(MethodCallExpression node, [NotNullWhen(true)] out INavigationBase? navigation) + { + navigation = null; + Expression current = node; + while (current is MethodCallExpression call) + { + if (!IsChainOperator(call)) + { + return false; + } + + current = call.Arguments[0]; + } + + if (current is not MemberExpression member) + { + return false; + } + + navigation = FindCollectionNavigation(member); + return navigation != null; + } + + static bool IsChainOperator(MethodCallExpression call) => + call.Method.IsStatic && + call.Method.DeclaringType == typeof(Enumerable) && + call.Arguments.Count > 0; + + INavigationBase? FindCollectionNavigation(MemberExpression member) + { + if (member.Expression == null) + { + return null; + } + + var entity = model.FindEntityType(member.Expression.Type); + if (entity == null) + { + return null; + } + + var name = member.Member.Name; + INavigationBase? navigation = entity.FindNavigation(name); + navigation ??= entity.FindSkipNavigation(name); + if (navigation is not { IsCollection: true }) + { + return null; + } + + return navigation; + } + + Expression ProcessChain(MethodCallExpression node, INavigationBase navigation) + { + // Lambdas passed along the chain can load collections of their own, for example + // _.Employees.Select(employee => new { Tasks = employee.Tasks.ToList() }) + var chain = VisitChainArguments(node); + + if (QueryAnalyzer.HasOrdering(chain)) + { + if (detectRedundantOrdering) + { + RedundantOrder.Validate(chain); + } + + return chain; + } + + if (GetConfiguration(navigation) is not { } configuration || + !CanOrder(chain)) + { + return chain; + } + + return ApplyOrdering(chain, configuration); + } + + // Visits every argument of each operator in the chain except the source it composes onto, which + // is the next operator down or the navigation itself. Visiting the navigation as a member would + // order it a second time as if it were bound into the result. + Expression VisitChainArguments(Expression expression) + { + if (expression is not MethodCallExpression call) + { + return expression; + } + + var arguments = new Expression[call.Arguments.Count]; + arguments[0] = VisitChainArguments(call.Arguments[0]); + for (var index = 1; index < arguments.Length; index++) + { + arguments[index] = VisitChainArgument(call.Arguments[index]); + } + + return call.Update(call.Object, arguments); + } + + // A lambda passed to the chain, such as a Select selector, can load collections into the result. + // Any other argument is not part of the result as it is: the second sequence of a Concat or a + // Union is one operand of a set operation, and ordering an operand orders nothing. + Expression VisitChainArgument(Expression argument) + { + if (argument is LambdaExpression) + { + return Visit(argument); + } + + var depth = projectionDepth; + projectionDepth = 0; + try + { + return Visit(argument); + } + finally + { + projectionDepth = depth; + } + } + + /// + /// Operators the default ordering has to be applied beneath rather than after. + /// + /// + /// Select projects the entity away, Skip, Take and the single result operators choose which + /// rows come back, and the rest copy the sequence as it is. + /// + static bool ShouldOrderBefore(MethodCallExpression call) => + call.Method.Name is + nameof(Enumerable.Select) or + nameof(Enumerable.Skip) or + nameof(Enumerable.Take) or + nameof(Enumerable.First) or + nameof(Enumerable.FirstOrDefault) or + nameof(Enumerable.Last) or + nameof(Enumerable.LastOrDefault) or + nameof(Enumerable.ElementAt) or + nameof(Enumerable.ElementAtOrDefault) or + nameof(Enumerable.ToList) or + nameof(Enumerable.ToArray) or + nameof(Enumerable.AsEnumerable); + + /// + /// Whether the ordering can go in beneath the operators that come after it, with only Where + /// filters between it and the navigation. Anything else, an aggregate such as Count or Any, or + /// an operator such as Distinct that EF Core drops ordering beneath, means the order of the + /// collection does not reach the result. + /// + static bool CanOrder(Expression chain) + { + var current = chain; + while (current is MethodCallExpression call && + ShouldOrderBefore(call)) + { + current = call.Arguments[0]; + } + + while (current is MethodCallExpression call) + { + if (call.Method.Name != nameof(Enumerable.Where)) + { + return false; + } + + current = call.Arguments[0]; + } + + return true; + } + + static Expression ApplyOrdering(Expression chain, Configuration configuration) + { + // Push past everything the ordering has to precede, then rebuild the chain around it + if (chain is MethodCallExpression call && + ShouldOrderBefore(call)) + { + var arguments = call.Arguments.ToArray(); + arguments[0] = ApplyOrdering(arguments[0], configuration); + return call.Update(call.Object, arguments); + } + + return AppendOrdering(chain, configuration); + } + + static Expression AppendOrdering(Expression source, Configuration configuration) + { + var result = source; + + foreach (var clause in configuration.Clauses) + { + result = clause.AppendEnumerableOrder(result); + } + + return result; + } + + static Configuration? GetConfiguration(INavigationBase navigation) + { + var target = navigation.TargetEntityType; + + // A JSON mapped collection is read out of its parent's JSON document rather than joined, + // so its document order is left as it is, the same as for an Include + if (target.IsMappedToJson()) + { + return null; + } + + if (Configuration.TryGet(target.ClrType) is not { Clauses.Count: > 0 } configuration) + { + return null; + } + + return configuration; + } +} diff --git a/src/EfOrderBy/QueryAnalyzer.cs b/src/EfOrderBy/QueryAnalyzer.cs index c1c61dc..9d4867d 100644 --- a/src/EfOrderBy/QueryAnalyzer.cs +++ b/src/EfOrderBy/QueryAnalyzer.cs @@ -1,5 +1,6 @@ /// -/// Scans the outermost chain of a query for existing ordering and for Include calls. +/// Scans the outermost chain of a query for existing ordering, for Include calls, and for Select +/// projections. /// /// /// Only the chain is walked, never the lambdas passed to it. Ordering nested inside a lambda, @@ -8,10 +9,11 @@ /// static class QueryAnalyzer { - public static (bool HasOrdering, bool HasInclude) Analyze(Expression expression) + public static (bool HasOrdering, bool HasInclude, bool HasSelect) Analyze(Expression expression) { var hasOrdering = false; var hasInclude = false; + var hasSelect = false; while (expression is MethodCallExpression call) { @@ -24,6 +26,11 @@ public static (bool HasOrdering, bool HasInclude) Analyze(Expression expression) { hasInclude = true; } + else if (declaringType == typeof(Queryable) && + name == "Select") + { + hasSelect = true; + } else if ((declaringType == typeof(Queryable) || declaringType == typeof(Enumerable)) && name is @@ -45,7 +52,7 @@ name is expression = call.Arguments[0]; } - return (hasOrdering, hasInclude); + return (hasOrdering, hasInclude, hasSelect); } public static bool HasOrdering(Expression expression) => diff --git a/src/Tests/DefaultOrderByTests.DefaultOrdering_WithIncludeAndSelect_WithNavInSelect.verified.txt b/src/Tests/DefaultOrderByTests.DefaultOrdering_WithIncludeAndSelect_WithNavInSelect.verified.txt index 0822389..c2f4324 100644 --- a/src/Tests/DefaultOrderByTests.DefaultOrdering_WithIncludeAndSelect_WithNavInSelect.verified.txt +++ b/src/Tests/DefaultOrderByTests.DefaultOrdering_WithIncludeAndSelect_WithNavInSelect.verified.txt @@ -1,4 +1,4 @@ SELECT [d].[Id], [d].[Name], [d].[DisplayOrder], [e].[Id], [e].[DepartmentId], [e].[HireDate], [e].[Name], [e].[Salary] FROM [Departments] AS [d] LEFT JOIN [Employees] AS [e] ON [d].[Id] = [e].[DepartmentId] -ORDER BY [d].[DisplayOrder], [d].[Id] \ No newline at end of file +ORDER BY [d].[DisplayOrder], [d].[Id], [e].[HireDate] DESC \ No newline at end of file diff --git a/src/Tests/ProjectionOrderingTests.AggregateInProjection_IsNotOrdered.verified.txt b/src/Tests/ProjectionOrderingTests.AggregateInProjection_IsNotOrdered.verified.txt new file mode 100644 index 0000000..177393b --- /dev/null +++ b/src/Tests/ProjectionOrderingTests.AggregateInProjection_IsNotOrdered.verified.txt @@ -0,0 +1,15 @@ +SELECT [d].[Name], ( + SELECT COUNT(*) + FROM [Employees] AS [e] + WHERE [d].[Id] = [e].[DepartmentId]) AS [Count], CASE + WHEN EXISTS ( + SELECT 1 + FROM [Employees] AS [e0] + WHERE [d].[Id] = [e0].[DepartmentId] AND [e0].[HireDate] > '2024-01-01T00:00:00.0000000') THEN CAST(1 AS bit) + ELSE CAST(0 AS bit) +END AS [AnyRecent], ( + SELECT COALESCE(SUM([e1].[Salary]), 0) + FROM [Employees] AS [e1] + WHERE [d].[Id] = [e1].[DepartmentId]) AS [Payroll] +FROM [Departments] AS [d] +ORDER BY [d].[DisplayOrder] \ No newline at end of file diff --git a/src/Tests/ProjectionOrderingTests.BoundNavigation_AppliesDefaultOrder.verified.txt b/src/Tests/ProjectionOrderingTests.BoundNavigation_AppliesDefaultOrder.verified.txt new file mode 100644 index 0000000..90898fc --- /dev/null +++ b/src/Tests/ProjectionOrderingTests.BoundNavigation_AppliesDefaultOrder.verified.txt @@ -0,0 +1,79 @@ +{ + target: [ + { + Name: Engineering, + Employees: [ + { + Id: 2, + DepartmentId: 1, + Name: Bob, + HireDate: 2024-03-20, + Salary: 85000 + }, + { + Id: 1, + DepartmentId: 1, + Name: Alice, + HireDate: 2024-01-15, + Salary: 90000 + }, + { + Id: 3, + DepartmentId: 1, + Name: Charlie, + HireDate: 2023-06-10, + Salary: 95000 + } + ] + }, + { + Name: Sales, + Employees: [ + { + Id: 4, + DepartmentId: 2, + Name: Diana, + HireDate: 2024-02-05, + Salary: 70000 + }, + { + Id: 5, + DepartmentId: 2, + Name: Eve, + HireDate: 2023-11-01, + Salary: 72000 + } + ] + }, + { + Name: HR, + Employees: [ + { + Id: 6, + DepartmentId: 3, + Name: Frank, + HireDate: 2024-04-10, + Salary: 65000 + } + ] + } + ], + sql: { + Text: +select d.Name, + d.Id, + e.Id, + e.DepartmentId, + e.HireDate, + e.Name, + e.Salary +from Departments as d + left outer join + Employees as e + on d.Id = e.DepartmentId +order by d.DisplayOrder, + d.Id, + e.HireDate desc, + HasTransaction: false + } +} \ No newline at end of file diff --git a/src/Tests/ProjectionOrderingTests.CollectionThatIsNotANavigation_IsLeftAlone.verified.txt b/src/Tests/ProjectionOrderingTests.CollectionThatIsNotANavigation_IsLeftAlone.verified.txt new file mode 100644 index 0000000..8af7cf8 --- /dev/null +++ b/src/Tests/ProjectionOrderingTests.CollectionThatIsNotANavigation_IsLeftAlone.verified.txt @@ -0,0 +1,11 @@ +DECLARE @names1 nvarchar(4000) = N'Bob'; +DECLARE @names2 nvarchar(4000) = N'Alice'; + +SELECT [d].[Name], [d].[Id], [e0].[Name], [e0].[Id] +FROM [Departments] AS [d] +LEFT JOIN ( + SELECT [e].[Name], [e].[Id], [e].[HireDate], [e].[DepartmentId] + FROM [Employees] AS [e] + WHERE [e].[Name] IN (@names1, @names2) +) AS [e0] ON [d].[Id] = [e0].[DepartmentId] +ORDER BY [d].[DisplayOrder], [d].[Id], [e0].[HireDate] DESC \ No newline at end of file diff --git a/src/Tests/ProjectionOrderingTests.DistinctInProjection_IsNotOrdered.verified.txt b/src/Tests/ProjectionOrderingTests.DistinctInProjection_IsNotOrdered.verified.txt new file mode 100644 index 0000000..3bcd31a --- /dev/null +++ b/src/Tests/ProjectionOrderingTests.DistinctInProjection_IsNotOrdered.verified.txt @@ -0,0 +1,8 @@ +SELECT [d].[Name], [d].[Id], [e0].[Name] +FROM [Departments] AS [d] +OUTER APPLY ( + SELECT DISTINCT [e].[Name] + FROM [Employees] AS [e] + WHERE [d].[Id] = [e].[DepartmentId] +) AS [e0] +ORDER BY [d].[DisplayOrder], [d].[Id] \ No newline at end of file diff --git a/src/Tests/ProjectionOrderingTests.ExplicitOrderingInProjection_IsKept.verified.txt b/src/Tests/ProjectionOrderingTests.ExplicitOrderingInProjection_IsKept.verified.txt new file mode 100644 index 0000000..0ec4164 --- /dev/null +++ b/src/Tests/ProjectionOrderingTests.ExplicitOrderingInProjection_IsKept.verified.txt @@ -0,0 +1,40 @@ +{ + target: [ + { + Name: Engineering, + Employees: [ + Alice, + Bob, + Charlie + ] + }, + { + Name: Sales, + Employees: [ + Diana, + Eve + ] + }, + { + Name: HR, + Employees: [ + Frank + ] + } + ], + sql: { + Text: +select d.Name, + d.Id, + e.Name, + e.Id +from Departments as d + left outer join + Employees as e + on d.Id = e.DepartmentId +order by d.DisplayOrder, + d.Id, + e.Name, + HasTransaction: false + } +} \ No newline at end of file diff --git a/src/Tests/ProjectionOrderingTests.FilteredCollection_OrdersAfterFilter.verified.txt b/src/Tests/ProjectionOrderingTests.FilteredCollection_OrdersAfterFilter.verified.txt new file mode 100644 index 0000000..2c08815 --- /dev/null +++ b/src/Tests/ProjectionOrderingTests.FilteredCollection_OrdersAfterFilter.verified.txt @@ -0,0 +1,44 @@ +{ + target: [ + { + Name: Engineering, + Employees: [ + Bob, + Charlie + ] + }, + { + Name: Sales, + Employees: [ + Diana, + Eve + ] + }, + { + Name: HR, + Employees: [ + Frank + ] + } + ], + sql: { + Text: +select d.Name, + d.Id, + e0.Name, + e0.Id +from Departments as d + left outer join + (select e.Name, + e.Id, + e.HireDate, + e.DepartmentId + from Employees as e + where e.Name <> N'Alice') as e0 + on d.Id = e0.DepartmentId +order by d.DisplayOrder, + d.Id, + e0.HireDate desc, + HasTransaction: false + } +} \ No newline at end of file diff --git a/src/Tests/ProjectionOrderingTests.FirstOrDefaultInProjection_AppliesDefaultOrder.verified.txt b/src/Tests/ProjectionOrderingTests.FirstOrDefaultInProjection_AppliesDefaultOrder.verified.txt new file mode 100644 index 0000000..25af99e --- /dev/null +++ b/src/Tests/ProjectionOrderingTests.FirstOrDefaultInProjection_AppliesDefaultOrder.verified.txt @@ -0,0 +1,27 @@ +{ + target: [ + { + Name: Engineering, + NewestHire: Bob + }, + { + Name: Sales, + NewestHire: Diana + }, + { + Name: HR, + NewestHire: Frank + } + ], + sql: { + Text: +select d.Name, + (select top (1) e.Name + from Employees as e + where d.Id = e.DepartmentId + order by e.HireDate desc) as NewestHire +from Departments as d +order by d.DisplayOrder, + HasTransaction: false + } +} \ No newline at end of file diff --git a/src/Tests/ProjectionOrderingTests.NestedCollections_EachGetTheirDefaultOrder.verified.txt b/src/Tests/ProjectionOrderingTests.NestedCollections_EachGetTheirDefaultOrder.verified.txt new file mode 100644 index 0000000..e7cf7e9 --- /dev/null +++ b/src/Tests/ProjectionOrderingTests.NestedCollections_EachGetTheirDefaultOrder.verified.txt @@ -0,0 +1,75 @@ +{ + target: [ + { + Name: Engineering, + Employees: [ + { + Name: Bob, + Tasks: [ + Monitor, + Deploy + ] + }, + { + Name: Alice, + Tasks: [ + Code review, + Testing, + Design + ] + }, + { + Name: Charlie + } + ] + }, + { + Name: Sales, + Employees: [ + { + Name: Diana + }, + { + Name: Eve + } + ] + }, + { + Name: HR, + Employees: [ + { + Name: Frank + } + ] + } + ], + sql: { + Text: +select d.Name, + d.Id, + s.Name, + s.Id, + s.Title, + s.Id0 +from Departments as d + left outer join + (select e.Name, + e.Id, + e0.Title, + e0.Id as Id0, + e.HireDate, + e0.Priority, + e.DepartmentId + from Employees as e + left outer join + EmployeeTasks as e0 + on e.Id = e0.EmployeeId) as s + on d.Id = s.DepartmentId +order by d.DisplayOrder, + d.Id, + s.HireDate desc, + s.Id, + s.Priority, + HasTransaction: false + } +} \ No newline at end of file diff --git a/src/Tests/ProjectionOrderingTests.ProjectedCollection_AppliesDefaultOrder.verified.txt b/src/Tests/ProjectionOrderingTests.ProjectedCollection_AppliesDefaultOrder.verified.txt new file mode 100644 index 0000000..74f28b7 --- /dev/null +++ b/src/Tests/ProjectionOrderingTests.ProjectedCollection_AppliesDefaultOrder.verified.txt @@ -0,0 +1,68 @@ +{ + target: [ + { + Id: 1, + Name: Engineering, + Employees: [ + { + Id: 2, + Name: Bob, + HireDate: 2024-03-20 + }, + { + Id: 1, + Name: Alice, + HireDate: 2024-01-15 + }, + { + Id: 3, + Name: Charlie, + HireDate: 2023-06-10 + } + ] + }, + { + Id: 2, + Name: Sales, + Employees: [ + { + Id: 4, + Name: Diana, + HireDate: 2024-02-05 + }, + { + Id: 5, + Name: Eve, + HireDate: 2023-11-01 + } + ] + }, + { + Id: 3, + Name: HR, + Employees: [ + { + Id: 6, + Name: Frank, + HireDate: 2024-04-10 + } + ] + } + ], + sql: { + Text: +select d.Id, + d.Name, + e.Id, + e.Name, + e.HireDate +from Departments as d + left outer join + Employees as e + on d.Id = e.DepartmentId +order by d.DisplayOrder, + d.Id, + e.HireDate desc, + HasTransaction: false + } +} \ No newline at end of file diff --git a/src/Tests/ProjectionOrderingTests.TakeInProjection_OrdersBeforeTake.verified.txt b/src/Tests/ProjectionOrderingTests.TakeInProjection_OrdersBeforeTake.verified.txt new file mode 100644 index 0000000..77d73d1 --- /dev/null +++ b/src/Tests/ProjectionOrderingTests.TakeInProjection_OrdersBeforeTake.verified.txt @@ -0,0 +1,50 @@ +{ + target: [ + { + Name: Engineering, + Employees: [ + Bob, + Alice + ] + }, + { + Name: Sales, + Employees: [ + Diana, + Eve + ] + }, + { + Name: HR, + Employees: [ + Frank + ] + } + ], + sql: { + Text: +select d.Name, + d.Id, + e1.Name, + e1.Id +from Departments as d + left outer join + (select e0.Name, + e0.Id, + e0.DepartmentId, + e0.HireDate + from (select e.Name, + e.Id, + e.DepartmentId, + ROW_NUMBER() over (partition by e.DepartmentId order by e.HireDate desc) as row, + e.HireDate + from Employees as e) as e0 + where e0.row <= 2) as e1 + on d.Id = e1.DepartmentId +order by d.DisplayOrder, + d.Id, + e1.DepartmentId, + e1.HireDate desc, + HasTransaction: false + } +} \ No newline at end of file diff --git a/src/Tests/ProjectionOrderingTests.cs b/src/Tests/ProjectionOrderingTests.cs new file mode 100644 index 0000000..a388a59 --- /dev/null +++ b/src/Tests/ProjectionOrderingTests.cs @@ -0,0 +1,248 @@ +// A collection navigation read inside a Select projection is loaded by a subquery of its own, and +// gets the default ordering of its entity the same way a collection in an Include does. +// Department: DisplayOrder. Employee: HireDate descending. EmployeeTask: Priority. +public class ProjectionOrderingTests +{ + [Test] + public async Task ProjectedCollection_AppliesDefaultOrder() + { + await using var database = await ModuleInitializer.SqlInstance.Build(); + await using var context = database.NewDbContext(); + + Recording.Start(); + + // The shape GraphQL.EntityFramework builds for a navigation list field + var results = await context.Departments + .Select(_ => new Department + { + Id = _.Id, + Name = _.Name, + Employees = _.Employees + .Select(employee => new Employee + { + Id = employee.Id, + Name = employee.Name, + HireDate = employee.HireDate + }) + .ToList() + }) + .ToListAsync(); + + await Assert.That(results[0].Employees.Select(_ => _.Name)).IsEquivalentTo([ + "Bob", // 2024-03-20 + "Alice", // 2024-01-15 + "Charlie" // 2023-06-10 + ], CollectionOrdering.Matching); + await Verify(results); + } + + [Test] + public async Task FilteredCollection_OrdersAfterFilter() + { + await using var database = await ModuleInitializer.SqlInstance.Build(); + await using var context = database.NewDbContext(); + + Recording.Start(); + var results = await context.Departments + .Select(_ => new + { + _.Name, + Employees = _.Employees + .Where(employee => employee.Name != "Alice") + .Select(employee => employee.Name) + .ToList() + }) + .ToListAsync(); + + await Assert.That(results[0].Employees).IsEquivalentTo(["Bob", "Charlie"], CollectionOrdering.Matching); + await Verify(results); + } + + [Test] + public async Task BoundNavigation_AppliesDefaultOrder() + { + await using var database = await ModuleInitializer.SqlInstance.Build(); + await using var context = database.NewDbContext(); + + Recording.Start(); + var results = await context.Departments + .Select(_ => new + { + _.Name, + _.Employees + }) + .ToListAsync(); + + await Assert.That(results[0].Employees.Select(_ => _.Name)).IsEquivalentTo([ + "Bob", + "Alice", + "Charlie" + ], CollectionOrdering.Matching); + await Verify(results); + } + + [Test] + public async Task NestedCollections_EachGetTheirDefaultOrder() + { + await using var database = await ModuleInitializer.SqlInstance.Build(); + await using var context = database.NewDbContext(); + + Recording.Start(); + var results = await context.Departments + .AsSingleQuery() + .Select(_ => new + { + _.Name, + Employees = _.Employees + .Select(employee => new + { + employee.Name, + Tasks = employee.Tasks + .Select(task => task.Title) + .ToList() + }) + .ToList() + }) + .ToListAsync(); + + var engineering = results[0].Employees; + await Assert.That(engineering.Select(_ => _.Name)).IsEquivalentTo(["Bob", "Alice", "Charlie"], CollectionOrdering.Matching); + var alice = engineering[1]; + await Assert.That(alice.Tasks).IsEquivalentTo([ + "Code review", // Priority 1 + "Testing", // Priority 2 + "Design" // Priority 3 + ], CollectionOrdering.Matching); + await Verify(results); + } + + [Test] + public async Task FirstOrDefaultInProjection_AppliesDefaultOrder() + { + await using var database = await ModuleInitializer.SqlInstance.Build(); + await using var context = database.NewDbContext(); + + Recording.Start(); + var results = await context.Departments + .Select(_ => new + { + _.Name, + NewestHire = _.Employees + .Select(employee => employee.Name) + .FirstOrDefault() + }) + .ToListAsync(); + + await Assert.That(results.Select(_ => _.NewestHire!)).IsEquivalentTo(["Bob", "Diana", "Frank"], CollectionOrdering.Matching); + await Verify(results); + } + + [Test] + public async Task TakeInProjection_OrdersBeforeTake() + { + await using var database = await ModuleInitializer.SqlInstance.Build(); + await using var context = database.NewDbContext(); + + Recording.Start(); + var results = await context.Departments + .Select(_ => new + { + _.Name, + Employees = _.Employees + .Take(2) + .Select(employee => employee.Name) + .ToList() + }) + .ToListAsync(); + + await Assert.That(results[0].Employees).IsEquivalentTo(["Bob", "Alice"], CollectionOrdering.Matching); + await Verify(results); + } + + [Test] + public async Task ExplicitOrderingInProjection_IsKept() + { + await using var database = await ModuleInitializer.SqlInstance.Build(); + await using var context = database.NewDbContext(); + + Recording.Start(); + var results = await context.Departments + .Select(_ => new + { + _.Name, + Employees = _.Employees + .OrderBy(employee => employee.Name) + .Select(employee => employee.Name) + .ToList() + }) + .ToListAsync(); + + await Assert.That(results[0].Employees).IsEquivalentTo(["Alice", "Bob", "Charlie"], CollectionOrdering.Matching); + await Verify(results); + } + + [Test] + public async Task AggregateInProjection_IsNotOrdered() + { + await using var database = await ModuleInitializer.SqlInstance.Build(); + await using var context = database.NewDbContext(); + + var sql = context.Departments + .Select(_ => new + { + _.Name, + Count = _.Employees.Count(), + AnyRecent = _.Employees.Any(employee => employee.HireDate > new DateTime(2024, 1, 1)), + Payroll = _.Employees.Select(employee => employee.Salary).Sum() + }) + .ToQueryString(); + + await Assert.That(sql).DoesNotContain("ORDER BY [e"); + await Verify(sql); + } + + [Test] + public async Task DistinctInProjection_IsNotOrdered() + { + await using var database = await ModuleInitializer.SqlInstance.Build(); + await using var context = database.NewDbContext(); + + // EF Core drops ordering beneath a Distinct, so there is nothing to place it beneath + var sql = context.Departments + .Select(_ => new + { + _.Name, + Names = _.Employees + .Select(employee => employee.Name) + .Distinct() + .ToList() + }) + .ToQueryString(); + + await Assert.That(sql).DoesNotContain("HireDate"); + await Verify(sql); + } + + [Test] + public async Task CollectionThatIsNotANavigation_IsLeftAlone() + { + await using var database = await ModuleInitializer.SqlInstance.Build(); + await using var context = database.NewDbContext(); + + // Only the navigation is ordered. The list of names is a parameter, not a navigation. + var names = new List { "Bob", "Alice" }; + var sql = context.Departments + .Select(_ => new + { + _.Name, + Matching = _.Employees + .Where(employee => names.Contains(employee.Name)) + .Select(employee => employee.Name) + .ToList() + }) + .ToQueryString(); + + await Assert.That(sql).Contains("[HireDate] DESC"); + await Verify(sql); + } +} diff --git a/src/Tests/RedundantOrderByTests.cs b/src/Tests/RedundantOrderByTests.cs index 37fdad7..13bd95a 100644 --- a/src/Tests/RedundantOrderByTests.cs +++ b/src/Tests/RedundantOrderByTests.cs @@ -206,6 +206,90 @@ await Assert.That( .ToQueryString()).ThrowsNothing(); } + [Test] + public async Task Projection_ExactMatch_Throws() + { + using var context = new RedundantEnabledContext(enabled); + + var exception = Assert.ThrowsExactly( + () => context.Entities + .Select(_ => new + { + _.Name, + Children = _.Children + .OrderBy(child => child.SortOrder) + .Select(child => child.Title) + .ToList() + }) + .ToQueryString()); + + await Assert.That(exception!.Message).Contains("RedundantChild"); + await Assert.That(exception.Message).Contains("OrderBy(SortOrder)"); + } + + [Test] + public void Projection_ExactMatchWithFilter_Throws() + { + using var context = new RedundantEnabledContext(enabled); + + Assert.ThrowsExactly( + () => context.Entities + .Select(_ => new + { + Children = _.Children + .Where(child => child.SortOrder > 0) + .OrderBy(child => child.SortOrder) + .ToList() + }) + .ToQueryString()); + } + + [Test] + public async Task Projection_DifferentOrdering_DoesNotThrow() + { + using var context = new RedundantEnabledContext(enabled); + + await Assert.That( + () => context.Entities + .Select(_ => new + { + Children = _.Children + .OrderBy(child => child.Title) + .ToList() + }) + .ToQueryString()).ThrowsNothing(); + } + + [Test] + public async Task Projection_WithoutOrdering_DoesNotThrow() + { + using var context = new RedundantEnabledContext(enabled); + + await Assert.That( + () => context.Entities + .Select(_ => new + { + Children = _.Children.ToList() + }) + .ToQueryString()).ThrowsNothing(); + } + + [Test] + public async Task Projection_ExactMatch_DetectionDisabled_DoesNotThrow() + { + using var context = new RedundantDisabledContext(disabled); + + await Assert.That( + () => context.Entities + .Select(_ => new + { + Children = _.Children + .OrderBy(child => child.SortOrder) + .ToList() + }) + .ToQueryString()).ThrowsNothing(); + } + [Test] public async Task ExactMatchInsideConcat_DoesNotThrow() { diff --git a/src/Tests/Snippets.cs b/src/Tests/Snippets.cs index 18aee2d..6f60d42 100644 --- a/src/Tests/Snippets.cs +++ b/src/Tests/Snippets.cs @@ -148,6 +148,28 @@ static async Task IncludeSupport() #endregion } + static async Task ProjectionSupport() + { + AppDbContext context = null!; + + #region ProjectionSupport + + // Departments ordered by DisplayOrder + // Employees ordered by HireDate, then Salary descending + var departments = await context.Departments + .Select(_ => new + { + _.Name, + Employees = _.Employees + .Where(employee => employee.Salary > 0) + .Select(employee => employee.Name) + .ToList() + }) + .ToListAsync(); + + #endregion + } + static void MultiColumnOrdering(ModelBuilder builder) { #region MultiColumnOrdering