diff --git a/readme.md b/readme.md
index 7401fa9..a06a96b 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
+- **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
- **Multi-column ordering**: Chain multiple ordering clauses with `ThenBy` and `ThenByDescending`
@@ -82,6 +83,37 @@ var employeesByName = await context.Employees
+## Paging and Single Results
+
+The default ordering is applied *before* any operator that chooses which rows come back, so
+`Skip`, `Take`, `First`, `FirstOrDefault`, `Last`, `LastOrDefault` and `ElementAt` all select
+from an ordered sequence:
+
+
+
+```cs
+// The ordering is applied before the page is taken, so the page is
+// taken from an ordered sequence rather than sorted after the fact
+var secondPage = await context.Employees
+ .Skip(20)
+ .Take(20)
+ .ToListAsync();
+
+// Ordered by HireDate, then Salary descending, so this is the
+// earliest hire rather than an arbitrary row
+var first = await context.Employees
+ .FirstAsync();
+```
+snippet source | anchor
+
+
+Without this, a page would be an arbitrary set of rows that happens to be sorted, and pages
+could both skip and repeat rows.
+
+Aggregates such as `Count` and `Any`, and `Single`, are left alone, since ordering them is
+pointless work.
+
+
## Include() Support
Nested collections in `.Include()` expressions are automatically ordered:
@@ -95,7 +127,7 @@ var departments = await context.Departments
.Include(_ => _.Employees)
.ToListAsync();
```
-snippet source | anchor
+snippet source | anchor
@@ -149,7 +181,7 @@ public class InheritanceDbContext : DbContext
public DbSet DerivedEntitiesB => Set();
}
```
-snippet source | anchor
+snippet source | anchor
Behavior:
@@ -172,7 +204,7 @@ builder.Entity()
.ThenBy(_ => _.Name)
.ThenByDescending(_ => _.Price);
```
-snippet source | anchor
+snippet source | anchor
@@ -422,7 +454,7 @@ public class AppDbContext : DbContext
public DbSet Employees => Set();
}
```
-snippet source | anchor
+snippet source | anchor
diff --git a/src/EfOrderBy/Configuration.cs b/src/EfOrderBy/Configuration.cs
index 8efa092..b15c299 100644
--- a/src/EfOrderBy/Configuration.cs
+++ b/src/EfOrderBy/Configuration.cs
@@ -59,15 +59,22 @@ internal void AddClause(PropertyInfo propertyInfo, bool descending, bool isThenB
/// Creates a new Configuration for a derived type by replaying the clause metadata.
/// The derived type must have the same properties (inherited from the base).
///
+ // The default lookup omits non public properties, which an entity can map,
+ // so without these the ordering fails to inherit for those properties.
+ const BindingFlags propertyFlags =
+ BindingFlags.Public |
+ BindingFlags.NonPublic |
+ BindingFlags.Instance;
+
internal Configuration CreateForDerivedType(Type derivedType)
{
var derived = new Configuration(derivedType) { IsInherited = true };
foreach (var meta in ClauseMetadataList)
{
- var prop = derivedType.GetProperty(meta.PropertyName);
- if (prop != null)
+ var property = derivedType.GetProperty(meta.PropertyName, propertyFlags);
+ if (property != null)
{
- derived.AddClause(prop, meta.Descending, meta.IsThenBy);
+ derived.AddClause(property, meta.Descending, meta.IsThenBy);
continue;
}
diff --git a/src/EfOrderBy/IncludeOrderingApplicator.cs b/src/EfOrderBy/IncludeOrderingApplicator.cs
index b1f270c..26819e0 100644
--- a/src/EfOrderBy/IncludeOrderingApplicator.cs
+++ b/src/EfOrderBy/IncludeOrderingApplicator.cs
@@ -4,7 +4,7 @@
sealed class IncludeOrderingApplicator(IModel model, bool detectRedundantOrdering) :
ExpressionVisitor
{
- static readonly ConcurrentDictionary collectionElementTypeCache = new();
+ // MakeGenericMethod is expensive enough to be worth caching, unlike the element type lookup
static readonly ConcurrentDictionary<(MethodInfo, Type), MethodInfo> includeMethodCache = new();
protected override Expression VisitMethodCall(MethodCallExpression node)
@@ -45,7 +45,7 @@ MethodCallExpression ProcessInclude(MethodCallExpression includeCall)
}
// Check if the navigation already has ordering
- if (Interceptor.HasOrdering(lambda.Body))
+ if (QueryAnalyzer.HasOrdering(lambda.Body))
{
// Already has explicit ordering, don't apply default
if (detectRedundantOrdering)
@@ -112,35 +112,36 @@ static Expression ApplyOrdering(Expression source, Configuration configuration)
return result;
}
- static Type? GetCollectionElementType(Type type) =>
- collectionElementTypeCache.GetOrAdd(type,
- static type =>
+ // Not cached. A navigation is declared as one of the types below, so the fast path is a
+ // handful of type comparisons, which is cheaper than a dictionary lookup and does not pin
+ // every type it is handed for the life of the process.
+ static Type? GetCollectionElementType(Type type)
+ {
+ // Check for IEnumerable
+ if (type.IsGenericType)
+ {
+ var genericDef = type.GetGenericTypeDefinition();
+ if (genericDef == typeof(IEnumerable<>) ||
+ genericDef == typeof(ICollection<>) ||
+ genericDef == typeof(IList<>) ||
+ genericDef == typeof(List<>))
{
- // Check for IEnumerable
- if (type.IsGenericType)
- {
- var genericDef = type.GetGenericTypeDefinition();
- if (genericDef == typeof(IEnumerable<>) ||
- genericDef == typeof(ICollection<>) ||
- genericDef == typeof(IList<>) ||
- genericDef == typeof(List<>))
- {
- return type.GetGenericArguments()[0];
- }
- }
-
- // Check interfaces
- foreach (var iface in type.GetInterfaces())
- {
- if (iface.IsGenericType &&
- iface.GetGenericTypeDefinition() == typeof(IEnumerable<>))
- {
- return iface.GetGenericArguments()[0];
- }
- }
-
- return null;
- });
+ return type.GetGenericArguments()[0];
+ }
+ }
+
+ // Check interfaces
+ foreach (var iface in type.GetInterfaces())
+ {
+ if (iface.IsGenericType &&
+ iface.GetGenericTypeDefinition() == typeof(IEnumerable<>))
+ {
+ return iface.GetGenericArguments()[0];
+ }
+ }
+
+ return null;
+ }
Configuration? GetConfiguration(Type element)
{
diff --git a/src/EfOrderBy/Interceptor.cs b/src/EfOrderBy/Interceptor.cs
index 58c66dc..63ab96a 100644
--- a/src/EfOrderBy/Interceptor.cs
+++ b/src/EfOrderBy/Interceptor.cs
@@ -3,8 +3,6 @@
///
sealed class Interceptor : IQueryExpressionInterceptor
{
- static readonly ConcurrentDictionary queryElementTypeCache = new();
-
public Expression QueryCompilationStarting(Expression query, QueryExpressionEventData eventData)
{
var context = eventData.Context;
@@ -14,19 +12,30 @@ public Expression QueryCompilationStarting(Expression query, QueryExpressionEven
}
var model = context.Model;
- RequiredOrder.Validate(context);
- var detectRedundantOrdering = RedundantOrder.IsEnabled(context);
+ // Both of the options below live on the same extension, so resolve it once
+ var extension = context.GetService()
+ .FindExtension();
+
+ RequiredOrder.Validate(context, extension);
- // First, process Include nodes to add ordering to nested collections
- var visitor = new IncludeOrderingApplicator(model, detectRedundantOrdering);
- var queryWithOrderedIncludes = visitor.Visit(query);
+ var detectRedundantOrdering = extension?.ThrowOnRedundantOrderBy ?? false;
// Analyze the query for ordering and includes in a single pass
- var analyzer = new QueryAnalyzer();
- analyzer.Visit(queryWithOrderedIncludes);
+ var (hasOrdering, hasInclude) = 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.
+ if (hasInclude)
+ {
+ var visitor = new IncludeOrderingApplicator(model, detectRedundantOrdering);
+ queryWithOrderedIncludes = visitor.Visit(query);
+ }
- if (analyzer.HasOrdering)
+ if (hasOrdering)
{
if (detectRedundantOrdering)
{
@@ -36,12 +45,11 @@ public Expression QueryCompilationStarting(Expression query, QueryExpressionEven
return queryWithOrderedIncludes;
}
- // For queries with Select projection, we need to get the entity type from the source
- // BEFORE the Select, not from the result type (which would be the projected type)
- // Walk through Select/Include to find the actual source entity type
- var sourceExpression = GetSourceBeforeProjection(queryWithOrderedIncludes);
+ // The entity type has to come from the source the ordering will be applied to, not from
+ // the result type, which may be a projection, a single entity, or a limited page
+ var source = GetOrderingSource(queryWithOrderedIncludes);
- var elementType = GetQueryElementType(sourceExpression.Type);
+ var elementType = GetQueryElementType(source.Type);
if (elementType == null)
{
return queryWithOrderedIncludes;
@@ -58,134 +66,109 @@ public Expression QueryCompilationStarting(Expression query, QueryExpressionEven
return queryWithOrderedIncludes;
}
- // Apply default ordering to the top-level query
- // If there's a Select projection at the end, we need to insert OrderBy before it
- return ApplyOrderingBeforeSelect(queryWithOrderedIncludes, configuration, analyzer.HasInclude);
+ return ApplyOrdering(queryWithOrderedIncludes, configuration);
}
- public static bool HasOrdering(Expression expression)
+ ///
+ /// Operators the default ordering has to be applied beneath rather than after.
+ ///
+ ///
+ /// Skip, Take and the single result operators choose which rows come back, so ordering
+ /// them afterwards sorts an arbitrary subset instead of choosing from an ordered one.
+ /// Select projects the entity away. EF Core replaces ordering that sits outside an Include
+ /// with key ordering. The rest only tag the query and pass their source through unchanged.
+ ///
+ static bool ShouldOrderBefore(MethodCallExpression call)
{
- var analyzer = new QueryAnalyzer();
- analyzer.Visit(expression);
- return analyzer.HasOrdering;
- }
+ var declaringType = call.Method.DeclaringType;
+ var name = call.Method.Name;
- static Expression GetSourceBeforeProjection(Expression expression)
- {
- // Walk through Select and Include to find the source entity expression
- while (expression is MethodCallExpression call)
+ if (declaringType == typeof(Queryable))
{
- var method = call.Method.Name;
- var declaringType = call.Method.DeclaringType;
+ return name is
+ "Select" or
+ "Skip" or
+ "Take" or
+ "First" or
+ "FirstOrDefault" or
+ "Last" or
+ "LastOrDefault" or
+ "ElementAt" or
+ "ElementAtOrDefault";
+ }
- // Skip over Select projections
- if (declaringType == typeof(Queryable) &&
- method == "Select")
- {
- expression = call.Arguments[0];
- continue;
- }
+ if (declaringType == typeof(EntityFrameworkQueryableExtensions))
+ {
+ return name is
+ "Include" or
+ "ThenInclude" or
+ "AsTracking" or
+ "AsNoTracking" or
+ "AsNoTrackingWithIdentityResolution" or
+ "AsSingleQuery" or
+ "AsSplitQuery" or
+ "IgnoreAutoIncludes" or
+ "IgnoreQueryFilters" or
+ "TagWith" or
+ "TagWithCallSite";
+ }
- // Skip over Include operations
- if (declaringType == typeof(EntityFrameworkQueryableExtensions) &&
- method is "Include" or "ThenInclude")
- {
- expression = call.Arguments[0];
- continue;
- }
+ return false;
+ }
- // Stop at any other operation
- break;
+ static Expression GetOrderingSource(Expression expression)
+ {
+ while (expression is MethodCallExpression call &&
+ ShouldOrderBefore(call))
+ {
+ expression = call.Arguments[0];
}
return expression;
}
- static Type? GetQueryElementType(Type type) =>
- queryElementTypeCache.GetOrAdd(type, static type =>
+ // Not cached. A query root is already IQueryable, so the fast path below is two type
+ // comparisons, which is cheaper than a dictionary lookup and does not pin every type it
+ // is handed for the life of the process.
+ static Type? GetQueryElementType(Type type)
+ {
+ if (type.IsGenericType)
{
- if (type.IsGenericType)
+ var genericDef = type.GetGenericTypeDefinition();
+ if (genericDef == typeof(IQueryable<>) ||
+ genericDef == typeof(IOrderedQueryable<>))
{
- var genericDef = type.GetGenericTypeDefinition();
- if (genericDef == typeof(IQueryable<>) ||
- genericDef == typeof(IOrderedQueryable<>))
- {
- return type.GetGenericArguments()[0];
- }
+ return type.GetGenericArguments()[0];
}
-
- foreach (var iface in type.GetInterfaces())
- {
- if (iface.IsGenericType &&
- iface.GetGenericTypeDefinition() == typeof(IQueryable<>))
- {
- return iface.GetGenericArguments()[0];
- }
- }
-
- return null;
- });
-
- static Expression ApplyOrderingBeforeSelect(Expression query, Configuration configuration, bool hasInclude)
- {
- // Check if the query contains Include first - need to apply ordering before Include
- // to prevent EF Core from replacing it with just Id ordering
- // This must be checked before Select, because queries can be: OrderBy().Include().Select()
- if (hasInclude)
- {
- return ApplyOrderingBeforeInclude(query, configuration);
}
- // Check if the query ends with a Select call
- if (query is MethodCallExpression call &&
- call.Method.DeclaringType == typeof(Queryable) &&
- call.Method.Name == "Select")
+ foreach (var iface in type.GetInterfaces())
{
- // Apply ordering to the source of the Select, then recreate the Select
- var orderedSource = ApplyOrdering(call.Arguments[0], configuration);
- return Expression.Call(call.Method, orderedSource, call.Arguments[1]);
+ if (iface.IsGenericType &&
+ iface.GetGenericTypeDefinition() == typeof(IQueryable<>))
+ {
+ return iface.GetGenericArguments()[0];
+ }
}
- // No Select or Include, apply ordering normally
- return ApplyOrdering(query, configuration);
+ return null;
}
- static Expression ApplyOrderingBeforeInclude(Expression query, Configuration configuration)
+ static Expression ApplyOrdering(Expression query, Configuration configuration)
{
- // Check if the query ends with a Select call (after Include)
- if (query is MethodCallExpression selectCall &&
- selectCall.Method.DeclaringType == typeof(Queryable) &&
- selectCall.Method.Name == "Select")
- {
- // Recursively apply ordering before Include in the source, then recreate Select
- var orderedSource = ApplyOrderingBeforeInclude(selectCall.Arguments[0], configuration);
- return Expression.Call(selectCall.Method, orderedSource, selectCall.Arguments[1]);
- }
-
- // Find the last method call before Include
- if (query is MethodCallExpression methodCall &&
- methodCall.Method.DeclaringType == typeof(EntityFrameworkQueryableExtensions) &&
- methodCall.Method.Name is "Include" or "ThenInclude")
+ // Push past everything the ordering has to precede, then rebuild the chain around it
+ if (query is MethodCallExpression call &&
+ ShouldOrderBefore(call))
{
- // Apply ordering to the source of the Include, then recreate the Include
- var orderedSource = ApplyOrderingBeforeInclude(methodCall.Arguments[0], configuration);
-
- // Recreate the Include call with the ordered source
- var args = new Expression[methodCall.Arguments.Count];
- args[0] = orderedSource;
- for (var i = 1; i < methodCall.Arguments.Count; i++)
- {
- args[i] = methodCall.Arguments[i];
- }
-
- return Expression.Call(methodCall.Method, args);
+ var arguments = call.Arguments.ToArray();
+ arguments[0] = ApplyOrdering(arguments[0], configuration);
+ return call.Update(call.Object, arguments);
}
- // No Include at this level, apply ordering here
- return ApplyOrdering(query, configuration);
+ return AppendOrdering(query, configuration);
}
- static Expression ApplyOrdering(Expression source, Configuration configuration)
+ static Expression AppendOrdering(Expression source, Configuration configuration)
{
var result = source;
diff --git a/src/EfOrderBy/OrderByClause.cs b/src/EfOrderBy/OrderByClause.cs
index 645c031..52c47be 100644
--- a/src/EfOrderBy/OrderByClause.cs
+++ b/src/EfOrderBy/OrderByClause.cs
@@ -26,38 +26,40 @@ internal OrderByClause(Type elementType, ParameterExpression parameter, Property
quotedLambda = Expression.Quote(lambda);
}
- static MethodInfo FindMethod(MethodInfo[] methods, string name) =>
- methods.First(_ => _.Name == name &&
- _.GetParameters().Length == 2);
+ // The overload taking a comparer cannot be expressed as configuration, so match on the
+ // two parameter one. The method table is not held in a field, since keeping it alive for
+ // the life of the process buys nothing once these eight have been resolved.
+ static MethodInfo FindMethod(Type type, string name) =>
+ type.GetMethods()
+ .First(_ => _.Name == name &&
+ _.GetParameters().Length == 2);
- static MethodInfo[] queryableMethods = typeof(Queryable).GetMethods();
- static MethodInfo[] enumerableMethods = typeof(Enumerable).GetMethods();
+ static readonly MethodInfo queryableOrderBy = FindMethod(typeof(Queryable), nameof(Queryable.OrderBy));
+ static readonly MethodInfo queryableOrderByDescending = FindMethod(typeof(Queryable), nameof(Queryable.OrderByDescending));
+ static readonly MethodInfo queryableThenBy = FindMethod(typeof(Queryable), nameof(Queryable.ThenBy));
+ static readonly MethodInfo queryableThenByDescending = FindMethod(typeof(Queryable), nameof(Queryable.ThenByDescending));
- static MethodInfo queryableOrderBy = FindMethod(queryableMethods, nameof(Queryable.OrderBy));
- static MethodInfo queryableOrderByDescending = FindMethod(queryableMethods, nameof(Queryable.OrderByDescending));
- static MethodInfo queryableThenBy = FindMethod(queryableMethods, nameof(Queryable.ThenBy));
- static MethodInfo queryableThenByDescending = FindMethod(queryableMethods, nameof(Queryable.ThenByDescending));
+ static readonly MethodInfo enumerableOrderBy = FindMethod(typeof(Enumerable), nameof(Enumerable.OrderBy));
+ static readonly MethodInfo enumerableOrderByDescending = FindMethod(typeof(Enumerable), nameof(Enumerable.OrderByDescending));
+ static readonly MethodInfo enumerableThenBy = FindMethod(typeof(Enumerable), nameof(Enumerable.ThenBy));
+ static readonly MethodInfo enumerableThenByDescending = FindMethod(typeof(Enumerable), nameof(Enumerable.ThenByDescending));
- static MethodInfo enumerableOrderBy = FindMethod(enumerableMethods, nameof(Enumerable.OrderBy));
- static MethodInfo enumerableOrderByDescending = FindMethod(enumerableMethods, nameof(Enumerable.OrderByDescending));
- static MethodInfo enumerableThenBy = FindMethod(enumerableMethods, nameof(Enumerable.ThenBy));
- static MethodInfo enumerableThenByDescending = FindMethod(enumerableMethods, nameof(Enumerable.ThenByDescending));
- LambdaExpression lambda;
+ readonly LambdaExpression lambda;
// The fully generic Enumerable method (e.g., OrderBy)
// ready to be invoked without further generic type arguments.
- MethodInfo enumerableMethod;
+ readonly MethodInfo enumerableMethod;
// The fully generic Queryable method (e.g., OrderBy)
// ready to be invoked without further generic type arguments.
- MethodInfo queryableMethod;
+ readonly MethodInfo queryableMethod;
public Expression AppendEnumerableOrder(Expression result) =>
// Enumerable methods expect Func, so we pass the lambda directly (no Quote)
Expression.Call(enumerableMethod, result, lambda);
// Queryable methods expect Expression>, so we use Quote()
- UnaryExpression quotedLambda;
+ readonly UnaryExpression quotedLambda;
public Expression AppendQueryableOrder(Expression result) =>
Expression.Call(queryableMethod, result, quotedLambda);
diff --git a/src/EfOrderBy/OrderByExtensions.cs b/src/EfOrderBy/OrderByExtensions.cs
index c4e77e5..06c43e2 100644
--- a/src/EfOrderBy/OrderByExtensions.cs
+++ b/src/EfOrderBy/OrderByExtensions.cs
@@ -5,7 +5,7 @@ namespace EfOrderBy;
///
public static class OrderByExtensions
{
- static Interceptor interceptor = new();
+ static readonly Interceptor interceptor = new();
///
/// Adds the default ordering interceptor to automatically apply ordering to queries
diff --git a/src/EfOrderBy/QueryAnalyzer.cs b/src/EfOrderBy/QueryAnalyzer.cs
index dbca5d3..c1c61dc 100644
--- a/src/EfOrderBy/QueryAnalyzer.cs
+++ b/src/EfOrderBy/QueryAnalyzer.cs
@@ -1,46 +1,53 @@
-sealed class QueryAnalyzer :
- ExpressionVisitor
+///
+/// Scans the outermost chain of a query for existing ordering and for Include calls.
+///
+///
+/// Only the chain is walked, never the lambdas passed to it. Ordering nested inside a lambda,
+/// for example Where(_ => _.Children.OrderBy(child => child.Name).Any()), orders a subquery and
+/// must not suppress the default ordering of the query it is nested in.
+///
+static class QueryAnalyzer
{
- public bool HasOrdering { get; private set; }
- public bool HasInclude { get; private set; }
-
- protected override Expression VisitMethodCall(MethodCallExpression node)
+ public static (bool HasOrdering, bool HasInclude) Analyze(Expression expression)
{
- var name = node.Method.Name;
- var type = node.Method.DeclaringType;
+ var hasOrdering = false;
+ var hasInclude = false;
- // Check if this is an Include/ThenInclude - don't look for ordering inside its lambda
- if (type == typeof(EntityFrameworkQueryableExtensions) &&
- name is "Include" or "ThenInclude")
+ while (expression is MethodCallExpression call)
{
- HasInclude = true;
- // Visit the source (first argument) but skip the lambda (second argument)
- // to avoid detecting ordering within Include(_ => _.Collection.OrderBy(...))
- Visit(node.Arguments[0]);
- return node;
- }
+ var method = call.Method;
+ var declaringType = method.DeclaringType;
+ var name = method.Name;
- // Check if this is a Select - don't look for ordering inside its lambda
- if (type == typeof(Queryable) &&
- name == "Select")
- {
- // Visit the source (first argument) but skip the lambda (second argument)
- // to avoid detecting ordering within Select(_ => new { ... _.Children.OrderBy(...) })
- Visit(node.Arguments[0]);
- return node;
- }
+ if (declaringType == typeof(EntityFrameworkQueryableExtensions) &&
+ name is "Include" or "ThenInclude")
+ {
+ hasInclude = true;
+ }
+ else if ((declaringType == typeof(Queryable) ||
+ declaringType == typeof(Enumerable)) &&
+ name is
+ "OrderBy" or
+ "OrderByDescending" or
+ "ThenBy" or
+ "ThenByDescending")
+ {
+ hasOrdering = true;
+ }
- if ((type == typeof(Queryable) ||
- type == typeof(Enumerable)) &&
- name is
- "OrderBy" or
- "OrderByDescending" or
- "ThenBy" or
- "ThenByDescending")
- {
- HasOrdering = true;
+ // Only static operators compose a chain, and the source is always their first argument
+ if (!method.IsStatic ||
+ call.Arguments.Count == 0)
+ {
+ break;
+ }
+
+ expression = call.Arguments[0];
}
- return base.VisitMethodCall(node);
+ return (hasOrdering, hasInclude);
}
+
+ public static bool HasOrdering(Expression expression) =>
+ Analyze(expression).HasOrdering;
}
diff --git a/src/EfOrderBy/RedundantOrder.cs b/src/EfOrderBy/RedundantOrder.cs
index 5e81372..b04ad4a 100644
--- a/src/EfOrderBy/RedundantOrder.cs
+++ b/src/EfOrderBy/RedundantOrder.cs
@@ -3,12 +3,6 @@
///
static class RedundantOrder
{
- public static bool IsEnabled(DbContext context) =>
- context.GetService()
- .FindExtension()
- ?.ThrowOnRedundantOrderBy ??
- false;
-
public static void Validate(Expression expression)
{
if (FindOrdering(expression) is not { } ordering)
@@ -61,8 +55,7 @@ public static void Validate(Expression expression)
return null;
}
- // The chain is walked outermost first, so each clause found is applied before the previous
- clauses.Insert(0, new(propertyName, descending, isThenBy));
+ clauses.Add(new(propertyName, descending, isThenBy));
elementType = method.GetGenericArguments()[0];
expression = call.Arguments[0];
continue;
@@ -87,15 +80,39 @@ public static void Validate(Expression expression)
return null;
}
+ // The chain is walked outermost first, so the clauses come out in reverse of how they apply
+ clauses.Reverse();
return (elementType, clauses);
}
+ // Operators that combine two sequences. Their first argument is only one side of the
+ // query, and the default ordering is never applied to the combined result, so ordering
+ // found down that side is not made redundant by it.
+ static readonly HashSet combining =
+ [
+ "Concat",
+ "Except",
+ "ExceptBy",
+ "GroupJoin",
+ "Intersect",
+ "IntersectBy",
+ "Join",
+ "LeftJoin",
+ "RightJoin",
+ "SequenceEqual",
+ "Union",
+ "UnionBy",
+ "Zip"
+ ];
+
// Ordering can sit behind calls like Where, AsNoTracking, or TagWith,
// all of which take the query being composed as their first argument.
static Expression? FindSource(MethodCallExpression call)
{
- if (call.Method.IsStatic &&
+ var method = call.Method;
+ if (method.IsStatic &&
call.Arguments.Count > 0 &&
+ !combining.Contains(method.Name) &&
typeof(IEnumerable).IsAssignableFrom(call.Arguments[0].Type))
{
return call.Arguments[0];
diff --git a/src/EfOrderBy/RequiredOrder.cs b/src/EfOrderBy/RequiredOrder.cs
index 8b310bd..3013e14 100644
--- a/src/EfOrderBy/RequiredOrder.cs
+++ b/src/EfOrderBy/RequiredOrder.cs
@@ -1,8 +1,8 @@
static class RequiredOrder
{
- static ConcurrentDictionary validated = [];
+ static readonly ConcurrentDictionary validated = [];
- public static void Validate(DbContext context)
+ public static void Validate(DbContext context, OrderRequiredExtension? extension)
{
var contextType = context.GetType();
@@ -12,12 +12,8 @@ public static void Validate(DbContext context)
return;
}
- // Check if this DbContext requires ordering for all entities (opt-in feature)
- var requireOrdering = context.GetService()
- .FindExtension()
- ?.RequireOrderingForAllEntities ?? false;
-
- if (requireOrdering)
+ // Requiring ordering for all entities is opt-in
+ if (extension is { RequireOrderingForAllEntities: true })
{
ValidateAllEntitiesHaveOrdering(context.Model);
}
diff --git a/src/Tests/InheritedOrderingTests.cs b/src/Tests/InheritedOrderingTests.cs
new file mode 100644
index 0000000..f545464
--- /dev/null
+++ b/src/Tests/InheritedOrderingTests.cs
@@ -0,0 +1,49 @@
+[TestFixture]
+public class InheritedOrderingTests
+{
+ static DbContextOptions options =
+ new DbContextOptionsBuilder()
+ .UseSqlServer("Server=.;Database=Test;")
+ .UseDefaultOrderBy()
+ .Options;
+
+ [Test]
+ public void NonPublicProperty_InheritsOrderingToDerivedType()
+ {
+ using var context = new InternalOrderContext(options);
+
+ var sql = context.Derived.ToQueryString();
+
+ Assert.That(sql, Does.Contain("ORDER BY"));
+ Assert.That(sql, Does.Contain("SortOrder"));
+ }
+}
+
+public class InternalOrderBase
+{
+ public int Id { get; set; }
+
+ // Mapped explicitly below, since conventions only pick up public properties
+ internal int SortOrder { get; set; }
+}
+
+public class InternalOrderDerived : InternalOrderBase
+{
+ public string Extra { get; set; } = "";
+}
+
+public class InternalOrderContext(DbContextOptions options) :
+ DbContext(options)
+{
+ public DbSet Bases => Set();
+ public DbSet Derived => Set();
+
+ protected override void OnModelCreating(ModelBuilder builder)
+ {
+ builder.Entity()
+ .Property(_ => _.SortOrder);
+
+ builder.Entity()
+ .OrderBy(_ => _.SortOrder);
+ }
+}
diff --git a/src/Tests/NestedOrderingTests.OrderingInsideAnyLambda_StillAppliesDefaultOrder.verified.txt b/src/Tests/NestedOrderingTests.OrderingInsideAnyLambda_StillAppliesDefaultOrder.verified.txt
new file mode 100644
index 0000000..5f5d671
--- /dev/null
+++ b/src/Tests/NestedOrderingTests.OrderingInsideAnyLambda_StillAppliesDefaultOrder.verified.txt
@@ -0,0 +1,61 @@
+{
+ target: [
+ {
+ Id: 6,
+ DepartmentId: 3,
+ Name: Frank,
+ HireDate: 2024-04-10,
+ Salary: 65000
+ },
+ {
+ Id: 2,
+ DepartmentId: 1,
+ Name: Bob,
+ HireDate: 2024-03-20,
+ Salary: 85000
+ },
+ {
+ Id: 4,
+ DepartmentId: 2,
+ Name: Diana,
+ HireDate: 2024-02-05,
+ Salary: 70000
+ },
+ {
+ Id: 1,
+ DepartmentId: 1,
+ Name: Alice,
+ HireDate: 2024-01-15,
+ Salary: 90000
+ },
+ {
+ Id: 5,
+ DepartmentId: 2,
+ Name: Eve,
+ HireDate: 2023-11-01,
+ Salary: 72000
+ },
+ {
+ Id: 3,
+ DepartmentId: 1,
+ Name: Charlie,
+ HireDate: 2023-06-10,
+ Salary: 95000
+ }
+ ],
+ sql: {
+ Text:
+select e.Id,
+ e.DepartmentId,
+ e.HireDate,
+ e.Name,
+ e.Salary
+from Employees as e
+where not exists (select 1
+ from EmployeeTasks as e0
+ where e.Id = e0.EmployeeId
+ and e0.Priority < 0)
+order by e.HireDate desc,
+ HasTransaction: false
+ }
+}
\ No newline at end of file
diff --git a/src/Tests/NestedOrderingTests.OrderingInsideIncludeFilter_StillOrdersNestedCollection.verified.txt b/src/Tests/NestedOrderingTests.OrderingInsideIncludeFilter_StillOrdersNestedCollection.verified.txt
new file mode 100644
index 0000000..d69afed
--- /dev/null
+++ b/src/Tests/NestedOrderingTests.OrderingInsideIncludeFilter_StillOrdersNestedCollection.verified.txt
@@ -0,0 +1,94 @@
+{
+ target: [
+ {
+ Id: 1,
+ Name: Engineering,
+ DisplayOrder: 1,
+ 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
+ }
+ ]
+ },
+ {
+ Id: 2,
+ Name: Sales,
+ DisplayOrder: 2,
+ 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
+ }
+ ]
+ },
+ {
+ Id: 3,
+ Name: HR,
+ DisplayOrder: 3,
+ Employees: [
+ {
+ Id: 6,
+ DepartmentId: 3,
+ Name: Frank,
+ HireDate: 2024-04-10,
+ Salary: 65000
+ }
+ ]
+ }
+ ],
+ sql: {
+ Text:
+select d.Id,
+ d.DisplayOrder,
+ d.Name,
+ e1.Id,
+ e1.DepartmentId,
+ e1.HireDate,
+ e1.Name,
+ e1.Salary
+from Departments as d
+ left outer join
+ (select e.Id,
+ e.DepartmentId,
+ e.HireDate,
+ e.Name,
+ e.Salary
+ from Employees as e
+ where (select COUNT(*)
+ from EmployeeTasks as e0
+ where e.Id = e0.EmployeeId) >= 0) as e1
+ on d.Id = e1.DepartmentId
+order by d.DisplayOrder,
+ d.Id,
+ e1.HireDate desc,
+ HasTransaction: false
+ }
+}
\ No newline at end of file
diff --git a/src/Tests/NestedOrderingTests.OrderingInsideSelectManyLambda_StillAppliesDefaultOrder.verified.txt b/src/Tests/NestedOrderingTests.OrderingInsideSelectManyLambda_StillAppliesDefaultOrder.verified.txt
new file mode 100644
index 0000000..8cc878a
--- /dev/null
+++ b/src/Tests/NestedOrderingTests.OrderingInsideSelectManyLambda_StillAppliesDefaultOrder.verified.txt
@@ -0,0 +1,60 @@
+{
+ target: [
+ {
+ Id: 6,
+ DepartmentId: 3,
+ Name: Frank,
+ HireDate: 2024-04-10,
+ Salary: 65000
+ },
+ {
+ Id: 2,
+ DepartmentId: 1,
+ Name: Bob,
+ HireDate: 2024-03-20,
+ Salary: 85000
+ },
+ {
+ Id: 4,
+ DepartmentId: 2,
+ Name: Diana,
+ HireDate: 2024-02-05,
+ Salary: 70000
+ },
+ {
+ Id: 1,
+ DepartmentId: 1,
+ Name: Alice,
+ HireDate: 2024-01-15,
+ Salary: 90000
+ },
+ {
+ Id: 5,
+ DepartmentId: 2,
+ Name: Eve,
+ HireDate: 2023-11-01,
+ Salary: 72000
+ },
+ {
+ Id: 3,
+ DepartmentId: 1,
+ Name: Charlie,
+ HireDate: 2023-06-10,
+ Salary: 95000
+ }
+ ],
+ sql: {
+ Text:
+select e.Id,
+ e.DepartmentId,
+ e.HireDate,
+ e.Name,
+ e.Salary
+from Departments as d
+ inner join
+ Employees as e
+ on d.Id = e.DepartmentId
+order by e.HireDate desc,
+ HasTransaction: false
+ }
+}
\ No newline at end of file
diff --git a/src/Tests/NestedOrderingTests.OrderingInsideWhereLambda_StillAppliesDefaultOrder.verified.txt b/src/Tests/NestedOrderingTests.OrderingInsideWhereLambda_StillAppliesDefaultOrder.verified.txt
new file mode 100644
index 0000000..693fed2
--- /dev/null
+++ b/src/Tests/NestedOrderingTests.OrderingInsideWhereLambda_StillAppliesDefaultOrder.verified.txt
@@ -0,0 +1,60 @@
+{
+ target: [
+ {
+ Id: 6,
+ DepartmentId: 3,
+ Name: Frank,
+ HireDate: 2024-04-10,
+ Salary: 65000
+ },
+ {
+ Id: 2,
+ DepartmentId: 1,
+ Name: Bob,
+ HireDate: 2024-03-20,
+ Salary: 85000
+ },
+ {
+ Id: 4,
+ DepartmentId: 2,
+ Name: Diana,
+ HireDate: 2024-02-05,
+ Salary: 70000
+ },
+ {
+ Id: 1,
+ DepartmentId: 1,
+ Name: Alice,
+ HireDate: 2024-01-15,
+ Salary: 90000
+ },
+ {
+ Id: 5,
+ DepartmentId: 2,
+ Name: Eve,
+ HireDate: 2023-11-01,
+ Salary: 72000
+ },
+ {
+ Id: 3,
+ DepartmentId: 1,
+ Name: Charlie,
+ HireDate: 2023-06-10,
+ Salary: 95000
+ }
+ ],
+ sql: {
+ Text:
+select e.Id,
+ e.DepartmentId,
+ e.HireDate,
+ e.Name,
+ e.Salary
+from Employees as e
+where (select COUNT(*)
+ from EmployeeTasks as e0
+ where e.Id = e0.EmployeeId) >= 0
+order by e.HireDate desc,
+ HasTransaction: false
+ }
+}
\ No newline at end of file
diff --git a/src/Tests/NestedOrderingTests.cs b/src/Tests/NestedOrderingTests.cs
new file mode 100644
index 0000000..980f8d4
--- /dev/null
+++ b/src/Tests/NestedOrderingTests.cs
@@ -0,0 +1,84 @@
+// Ordering that appears inside a lambda belongs to a subquery. It must not be mistaken
+// for explicit ordering of the query the lambda is nested in.
+[TestFixture]
+public class NestedOrderingTests
+{
+ [Test]
+ public async Task OrderingInsideWhereLambda_StillAppliesDefaultOrder()
+ {
+ await using var database = await ModuleInitializer.SqlInstance.Build();
+ await using var context = database.NewDbContext();
+
+ Recording.Start();
+ var results = await context.Employees
+ .Where(_ => _.Tasks.OrderBy(task => task.Title).Count() >= 0)
+ .ToListAsync();
+
+ // Employees keep their default ordering of HireDate descending
+ Assert.That(results.Select(_ => _.Name), Is.EqualTo(new[]
+ {
+ "Frank", // 2024-04-10
+ "Bob", // 2024-03-20
+ "Diana", // 2024-02-05
+ "Alice", // 2024-01-15
+ "Eve", // 2023-11-01
+ "Charlie" // 2023-06-10
+ }));
+ await Verify(results);
+ }
+
+ [Test]
+ public async Task OrderingInsideAnyLambda_StillAppliesDefaultOrder()
+ {
+ await using var database = await ModuleInitializer.SqlInstance.Build();
+ await using var context = database.NewDbContext();
+
+ Recording.Start();
+ var results = await context.Employees
+ .Where(_ => !_.Tasks.OrderBy(task => task.Title).Any(task => task.Priority < 0))
+ .ToListAsync();
+
+ Assert.That(results[0].Name, Is.EqualTo("Frank"));
+ Assert.That(results[^1].Name, Is.EqualTo("Charlie"));
+ await Verify(results);
+ }
+
+ [Test]
+ public async Task OrderingInsideSelectManyLambda_StillAppliesDefaultOrder()
+ {
+ await using var database = await ModuleInitializer.SqlInstance.Build();
+ await using var context = database.NewDbContext();
+
+ Recording.Start();
+ var results = await context.Departments
+ .SelectMany(_ => _.Employees.OrderBy(employee => employee.Name))
+ .ToListAsync();
+
+ // The projected employees get their own default ordering of HireDate descending
+ Assert.That(results[0].Name, Is.EqualTo("Frank"));
+ Assert.That(results[^1].Name, Is.EqualTo("Charlie"));
+ await Verify(results);
+ }
+
+ [Test]
+ public async Task OrderingInsideIncludeFilter_StillOrdersNestedCollection()
+ {
+ await using var database = await ModuleInitializer.SqlInstance.Build();
+ await using var context = database.NewDbContext();
+
+ Recording.Start();
+ var results = await context.Departments
+ .Include(_ => _.Employees.Where(employee => employee.Tasks.OrderBy(task => task.Title).Count() >= 0))
+ .ToListAsync();
+
+ // The ordering inside the filter applies to Tasks, so Employees still get their default
+ var engineering = results[0].Employees;
+ Assert.That(engineering.Select(_ => _.Name), Is.EqualTo(new[]
+ {
+ "Bob", // 2024-03-20
+ "Alice", // 2024-01-15
+ "Charlie" // 2023-06-10
+ }));
+ await Verify(results);
+ }
+}
diff --git a/src/Tests/RedundantOrderByTests.cs b/src/Tests/RedundantOrderByTests.cs
index 32a4fda..965aebf 100644
--- a/src/Tests/RedundantOrderByTests.cs
+++ b/src/Tests/RedundantOrderByTests.cs
@@ -207,6 +207,39 @@ public void Include_WithoutOrdering_DoesNotThrow()
.ToQueryString());
}
+ [Test]
+ public void ExactMatchInsideConcat_DoesNotThrow()
+ {
+ using var context = new RedundantEnabledContext(enabled);
+
+ // The default ordering is not applied to a combined sequence, so ordering one
+ // side of it is not redundant
+ Assert.DoesNotThrow(
+ () => context.Entities
+ .Where(_ => _.Priority > 1)
+ .OrderBy(_ => _.Name)
+ .ThenByDescending(_ => _.Priority)
+ .Concat(context.Entities.Where(_ => _.Priority <= 1))
+ .ToQueryString());
+ }
+
+ [Test]
+ public void ExactMatchInsideJoin_DoesNotThrow()
+ {
+ using var context = new RedundantEnabledContext(enabled);
+
+ Assert.DoesNotThrow(
+ () => context.Entities
+ .OrderBy(_ => _.Name)
+ .ThenByDescending(_ => _.Priority)
+ .Join(
+ context.Children,
+ entity => entity.Id,
+ child => child.RedundantEntityId,
+ (entity, child) => child.Title)
+ .ToQueryString());
+ }
+
[Test]
public void Disabled_DoesNotThrow()
{
diff --git a/src/Tests/RowLimitingTests.Count_DoesNotApplyOrdering.verified.txt b/src/Tests/RowLimitingTests.Count_DoesNotApplyOrdering.verified.txt
new file mode 100644
index 0000000..0440aeb
--- /dev/null
+++ b/src/Tests/RowLimitingTests.Count_DoesNotApplyOrdering.verified.txt
@@ -0,0 +1,9 @@
+{
+ target: 6,
+ sql: {
+ Text:
+select COUNT(*)
+from Employees as e,
+ HasTransaction: false
+ }
+}
\ No newline at end of file
diff --git a/src/Tests/RowLimitingTests.ElementAt_AppliesDefaultOrder.verified.txt b/src/Tests/RowLimitingTests.ElementAt_AppliesDefaultOrder.verified.txt
new file mode 100644
index 0000000..daf581f
--- /dev/null
+++ b/src/Tests/RowLimitingTests.ElementAt_AppliesDefaultOrder.verified.txt
@@ -0,0 +1,24 @@
+{
+ target: {
+ Id: 4,
+ DepartmentId: 2,
+ Name: Diana,
+ HireDate: 2024-02-05,
+ Salary: 70000
+ },
+ sql: {
+ Text:
+select e.Id,
+ e.DepartmentId,
+ e.HireDate,
+ e.Name,
+ e.Salary
+from Employees as e
+order by e.HireDate desc
+offset @p rows fetch next 1 rows only,
+ Parameters: {
+ @p: 2
+ },
+ HasTransaction: false
+ }
+}
\ No newline at end of file
diff --git a/src/Tests/RowLimitingTests.FirstOrDefaultWithPredicate_AppliesDefaultOrder.verified.txt b/src/Tests/RowLimitingTests.FirstOrDefaultWithPredicate_AppliesDefaultOrder.verified.txt
new file mode 100644
index 0000000..96b66b6
--- /dev/null
+++ b/src/Tests/RowLimitingTests.FirstOrDefaultWithPredicate_AppliesDefaultOrder.verified.txt
@@ -0,0 +1,21 @@
+{
+ target: {
+ Id: 2,
+ DepartmentId: 1,
+ Name: Bob,
+ HireDate: 2024-03-20,
+ Salary: 85000
+ },
+ sql: {
+ Text:
+select top (1) e.Id,
+ e.DepartmentId,
+ e.HireDate,
+ e.Name,
+ e.Salary
+from Employees as e
+where e.Salary > 70000
+order by e.HireDate desc,
+ HasTransaction: false
+ }
+}
\ No newline at end of file
diff --git a/src/Tests/RowLimitingTests.First_AppliesDefaultOrder.verified.txt b/src/Tests/RowLimitingTests.First_AppliesDefaultOrder.verified.txt
new file mode 100644
index 0000000..17ae05a
--- /dev/null
+++ b/src/Tests/RowLimitingTests.First_AppliesDefaultOrder.verified.txt
@@ -0,0 +1,20 @@
+{
+ target: {
+ Id: 6,
+ DepartmentId: 3,
+ Name: Frank,
+ HireDate: 2024-04-10,
+ Salary: 65000
+ },
+ sql: {
+ Text:
+select top (1) e.Id,
+ e.DepartmentId,
+ e.HireDate,
+ e.Name,
+ e.Salary
+from Employees as e
+order by e.HireDate desc,
+ HasTransaction: false
+ }
+}
\ No newline at end of file
diff --git a/src/Tests/RowLimitingTests.IncludeThenPage_OrdersBeforeLimiting.verified.txt b/src/Tests/RowLimitingTests.IncludeThenPage_OrdersBeforeLimiting.verified.txt
new file mode 100644
index 0000000..69e2e2f
--- /dev/null
+++ b/src/Tests/RowLimitingTests.IncludeThenPage_OrdersBeforeLimiting.verified.txt
@@ -0,0 +1,52 @@
+{
+ target: [
+ {
+ Id: 2,
+ Name: Sales,
+ DisplayOrder: 2,
+ 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
+ }
+ ]
+ }
+ ],
+ sql: {
+ Text:
+select d0.Id,
+ d0.DisplayOrder,
+ d0.Name,
+ e.Id,
+ e.DepartmentId,
+ e.HireDate,
+ e.Name,
+ e.Salary
+from (select d.Id,
+ d.DisplayOrder,
+ d.Name
+ from Departments as d
+ order by d.DisplayOrder
+ offset @p rows fetch next @p rows only) as d0
+ left outer join
+ Employees as e
+ on d0.Id = e.DepartmentId
+order by d0.DisplayOrder,
+ d0.Id,
+ e.HireDate desc,
+ Parameters: {
+ @p: 1
+ },
+ HasTransaction: false
+ }
+}
\ No newline at end of file
diff --git a/src/Tests/RowLimitingTests.SelectThenFirst_AppliesDefaultOrderToSource.verified.txt b/src/Tests/RowLimitingTests.SelectThenFirst_AppliesDefaultOrderToSource.verified.txt
new file mode 100644
index 0000000..13f97e6
--- /dev/null
+++ b/src/Tests/RowLimitingTests.SelectThenFirst_AppliesDefaultOrderToSource.verified.txt
@@ -0,0 +1,10 @@
+{
+ target: Frank,
+ sql: {
+ Text:
+select top (1) e.Name
+from Employees as e
+order by e.HireDate desc,
+ HasTransaction: false
+ }
+}
\ No newline at end of file
diff --git a/src/Tests/RowLimitingTests.Single_DoesNotApplyOrdering.verified.txt b/src/Tests/RowLimitingTests.Single_DoesNotApplyOrdering.verified.txt
new file mode 100644
index 0000000..bce32dd
--- /dev/null
+++ b/src/Tests/RowLimitingTests.Single_DoesNotApplyOrdering.verified.txt
@@ -0,0 +1,20 @@
+{
+ target: {
+ Id: 5,
+ DepartmentId: 2,
+ Name: Eve,
+ HireDate: 2023-11-01,
+ Salary: 72000
+ },
+ sql: {
+ Text:
+select top (2) e.Id,
+ e.DepartmentId,
+ e.HireDate,
+ e.Name,
+ e.Salary
+from Employees as e
+where e.Name = N'Eve',
+ HasTransaction: false
+ }
+}
\ No newline at end of file
diff --git a/src/Tests/RowLimitingTests.SkipTake_OrdersBeforeLimiting.verified.txt b/src/Tests/RowLimitingTests.SkipTake_OrdersBeforeLimiting.verified.txt
new file mode 100644
index 0000000..8579c9d
--- /dev/null
+++ b/src/Tests/RowLimitingTests.SkipTake_OrdersBeforeLimiting.verified.txt
@@ -0,0 +1,34 @@
+{
+ target: [
+ {
+ Id: 2,
+ DepartmentId: 1,
+ Name: Bob,
+ HireDate: 2024-03-20,
+ Salary: 85000
+ },
+ {
+ Id: 4,
+ DepartmentId: 2,
+ Name: Diana,
+ HireDate: 2024-02-05,
+ Salary: 70000
+ }
+ ],
+ sql: {
+ Text:
+select e.Id,
+ e.DepartmentId,
+ e.HireDate,
+ e.Name,
+ e.Salary
+from Employees as e
+order by e.HireDate desc
+offset @p rows fetch next @p1 rows only,
+ Parameters: {
+ @p: 1,
+ @p1: 2
+ },
+ HasTransaction: false
+ }
+}
\ No newline at end of file
diff --git a/src/Tests/RowLimitingTests.Take_OrdersBeforeLimiting.verified.txt b/src/Tests/RowLimitingTests.Take_OrdersBeforeLimiting.verified.txt
new file mode 100644
index 0000000..0d49d11
--- /dev/null
+++ b/src/Tests/RowLimitingTests.Take_OrdersBeforeLimiting.verified.txt
@@ -0,0 +1,39 @@
+{
+ target: [
+ {
+ Id: 6,
+ DepartmentId: 3,
+ Name: Frank,
+ HireDate: 2024-04-10,
+ Salary: 65000
+ },
+ {
+ Id: 2,
+ DepartmentId: 1,
+ Name: Bob,
+ HireDate: 2024-03-20,
+ Salary: 85000
+ },
+ {
+ Id: 4,
+ DepartmentId: 2,
+ Name: Diana,
+ HireDate: 2024-02-05,
+ Salary: 70000
+ }
+ ],
+ sql: {
+ Text:
+select top (@p) e.Id,
+ e.DepartmentId,
+ e.HireDate,
+ e.Name,
+ e.Salary
+from Employees as e
+order by e.HireDate desc,
+ Parameters: {
+ @p: 3
+ },
+ HasTransaction: false
+ }
+}
\ No newline at end of file
diff --git a/src/Tests/RowLimitingTests.cs b/src/Tests/RowLimitingTests.cs
new file mode 100644
index 0000000..1016707
--- /dev/null
+++ b/src/Tests/RowLimitingTests.cs
@@ -0,0 +1,143 @@
+// The default ordering has to be applied before any operator that chooses rows, otherwise
+// an arbitrary subset is chosen first and only then sorted.
+[TestFixture]
+public class RowLimitingTests
+{
+ // Employees in default order of HireDate descending:
+ // Frank, Bob, Diana, Alice, Eve, Charlie
+
+ [Test]
+ public async Task Take_OrdersBeforeLimiting()
+ {
+ await using var database = await ModuleInitializer.SqlInstance.Build();
+ await using var context = database.NewDbContext();
+
+ Recording.Start();
+ var results = await context.Employees
+ .Take(3)
+ .ToListAsync();
+
+ Assert.That(results.Select(_ => _.Name), Is.EqualTo(new[] { "Frank", "Bob", "Diana" }));
+ await Verify(results);
+ }
+
+ [Test]
+ public async Task SkipTake_OrdersBeforeLimiting()
+ {
+ await using var database = await ModuleInitializer.SqlInstance.Build();
+ await using var context = database.NewDbContext();
+
+ Recording.Start();
+ var results = await context.Employees
+ .Skip(1)
+ .Take(2)
+ .ToListAsync();
+
+ Assert.That(results.Select(_ => _.Name), Is.EqualTo(new[] { "Bob", "Diana" }));
+ await Verify(results);
+ }
+
+ [Test]
+ public async Task IncludeThenPage_OrdersBeforeLimiting()
+ {
+ await using var database = await ModuleInitializer.SqlInstance.Build();
+ await using var context = database.NewDbContext();
+
+ Recording.Start();
+ var results = await context.Departments
+ .Include(_ => _.Employees)
+ .Skip(1)
+ .Take(1)
+ .ToListAsync();
+
+ // Departments in default order are Engineering, Sales, HR
+ Assert.That(results.Single().Name, Is.EqualTo("Sales"));
+
+ // The included collection keeps its own default ordering
+ Assert.That(results[0].Employees.Select(_ => _.Name), Is.EqualTo(new[] { "Diana", "Eve" }));
+ await Verify(results);
+ }
+
+ [Test]
+ public async Task First_AppliesDefaultOrder()
+ {
+ await using var database = await ModuleInitializer.SqlInstance.Build();
+ await using var context = database.NewDbContext();
+
+ Recording.Start();
+ var result = await context.Employees.FirstAsync();
+
+ Assert.That(result.Name, Is.EqualTo("Frank"));
+ await Verify(result);
+ }
+
+ [Test]
+ public async Task FirstOrDefaultWithPredicate_AppliesDefaultOrder()
+ {
+ await using var database = await ModuleInitializer.SqlInstance.Build();
+ await using var context = database.NewDbContext();
+
+ Recording.Start();
+ var result = await context.Employees.FirstOrDefaultAsync(_ => _.Salary > 70000);
+
+ // Of Alice, Bob, Charlie and Eve, Bob was hired most recently
+ Assert.That(result!.Name, Is.EqualTo("Bob"));
+ await Verify(result);
+ }
+
+ [Test]
+ public async Task SelectThenFirst_AppliesDefaultOrderToSource()
+ {
+ await using var database = await ModuleInitializer.SqlInstance.Build();
+ await using var context = database.NewDbContext();
+
+ Recording.Start();
+ var result = await context.Employees
+ .Select(_ => _.Name)
+ .FirstAsync();
+
+ Assert.That(result, Is.EqualTo("Frank"));
+ await Verify(result);
+ }
+
+ [Test]
+ public async Task ElementAt_AppliesDefaultOrder()
+ {
+ await using var database = await ModuleInitializer.SqlInstance.Build();
+ await using var context = database.NewDbContext();
+
+ Recording.Start();
+ var result = await context.Employees.ElementAtAsync(2);
+
+ Assert.That(result.Name, Is.EqualTo("Diana"));
+ await Verify(result);
+ }
+
+ [Test]
+ public async Task Count_DoesNotApplyOrdering()
+ {
+ await using var database = await ModuleInitializer.SqlInstance.Build();
+ await using var context = database.NewDbContext();
+
+ Recording.Start();
+ var result = await context.Employees.CountAsync();
+
+ // Ordering an aggregate is pointless work, so it must be left alone
+ Assert.That(result, Is.EqualTo(6));
+ await Verify(result);
+ }
+
+ [Test]
+ public async Task Single_DoesNotApplyOrdering()
+ {
+ await using var database = await ModuleInitializer.SqlInstance.Build();
+ await using var context = database.NewDbContext();
+
+ Recording.Start();
+ var result = await context.Employees.SingleAsync(_ => _.Name == "Eve");
+
+ // Single matches at most one row, so ordering it is pointless work
+ Assert.That(result.Salary, Is.EqualTo(72000));
+ await Verify(result);
+ }
+}
diff --git a/src/Tests/Snippets.cs b/src/Tests/Snippets.cs
index b144747..149aafd 100644
--- a/src/Tests/Snippets.cs
+++ b/src/Tests/Snippets.cs
@@ -82,6 +82,27 @@ static async Task QueryWithoutOrderBy()
#endregion
}
+ static async Task PagingAndSingleResults()
+ {
+ AppDbContext context = null!;
+
+ #region PagingAndSingleResults
+
+ // The ordering is applied before the page is taken, so the page is
+ // taken from an ordered sequence rather than sorted after the fact
+ var secondPage = await context.Employees
+ .Skip(20)
+ .Take(20)
+ .ToListAsync();
+
+ // Ordered by HireDate, then Salary descending, so this is the
+ // earliest hire rather than an arbitrary row
+ var first = await context.Employees
+ .FirstAsync();
+
+ #endregion
+ }
+
static async Task IncludeSupport()
{
AppDbContext context = null!;