Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ private static bool IsNullable(LambdaExpression expression)
}

// For computed key expressions (method calls, concatenation, etc.) we cannot inspect
// NRT annotations at runtime. Treat as non-nullable — the safe default that avoids
// NRT annotations at runtime. Treat as non-nullable, the safe default that avoids
// injecting spurious null-handling into the generated WHERE clause.
return false;
}
Expand Down Expand Up @@ -769,7 +769,6 @@ private sealed class OrderByRemovalRewriter : ExpressionVisitor
{
private readonly List<LambdaExpression> _orderExpressions = [];
private readonly List<string> _orderMethods = [];
private bool _insideSelectProjection;

public (Expression, List<LambdaExpression>, List<string>) Rewrite(Expression expression)
{
Expand All @@ -783,27 +782,11 @@ private sealed class OrderByRemovalRewriter : ExpressionVisitor

protected override Expression VisitMethodCall(MethodCallExpression node)
{
// we are not interested in nested order by calls
if (node.Method.DeclaringType == typeof(Queryable) && node.Method.Name == nameof(Queryable.Select))
{
// We first visit our parent. When we visit an expression
// like "OrderBy().Select()", we want to visit the OrderBy first.
var source = Visit(node.Arguments[0]);

var previousState = _insideSelectProjection;
_insideSelectProjection = true;
var projection = Visit(node.Arguments[1]);
_insideSelectProjection = previousState;

return node.Update(null, [source, projection]);
}

if (node.Method.DeclaringType == typeof(Queryable)
&& (node.Method.Name == nameof(Queryable.OrderBy)
|| node.Method.Name == nameof(Queryable.OrderByDescending)
|| node.Method.Name == nameof(Queryable.ThenBy)
|| node.Method.Name == nameof(Queryable.ThenByDescending))
&& !_insideSelectProjection)
|| node.Method.Name == nameof(Queryable.ThenByDescending)))
{
var lambda = (LambdaExpression)StripQuotes(node.Arguments[1]);
_orderExpressions.Add(lambda);
Expand All @@ -814,6 +797,8 @@ protected override Expression VisitMethodCall(MethodCallExpression node)
return base.VisitMethodCall(node);
}

protected override Expression VisitLambda<T>(Expression<T> node) => node;

private static Expression StripQuotes(Expression e)
{
while (e.NodeType == ExpressionType.Quote)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ internal sealed class ExtractOrderPropertiesVisitor : ExpressionVisitor
private const string ThenByMethod = "ThenBy";
private const string OrderByDescendingMethod = "OrderByDescending";
private const string ThenByDescendingMethod = "ThenByDescending";
private bool _isOrderScope;

public List<MemberExpression> OrderProperties { get; } = [];

protected override Expression VisitMethodCall(MethodCallExpression node)
Expand All @@ -19,26 +17,29 @@ protected override Expression VisitMethodCall(MethodCallExpression node)
|| node.Method.Name == OrderByDescendingMethod
|| node.Method.Name == ThenByDescendingMethod)
{
_isOrderScope = true;

var lambda = StripQuotes(node.Arguments[1]);
Visit(lambda.Body);

_isOrderScope = false;
new OrderPropertyVisitor(lambda.Parameters[0], OrderProperties).Visit(lambda.Body);
}

return base.VisitMethodCall(node);
}

protected override Expression VisitMember(MemberExpression node)
protected override Expression VisitLambda<T>(Expression<T> node) => node;

private sealed class OrderPropertyVisitor(
ParameterExpression orderKeyParameter,
List<MemberExpression> orderProperties)
: ExpressionVisitor
{
if (_isOrderScope)
protected override Expression VisitMember(MemberExpression node)
{
// we only collect members that are within an order method.
OrderProperties.Add(node);
}
if (node.Expression == orderKeyParameter)
{
orderProperties.Add(node);
}

return base.VisitMember(node);
return base.VisitMember(node);
}
}

private static LambdaExpression StripQuotes(Expression expression)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ protected override Expression VisitMethodCall(MethodCallExpression node)
return base.VisitMethodCall(node);
}

protected override Expression VisitLambda<T>(Expression<T> node) => node;

public static IQueryable<T> Rewrite<T>(IQueryable<T> query)
{
var reversedExpression = new ReverseOrderExpressionRewriter().Visit(query.Expression);
Expand Down
2 changes: 2 additions & 0 deletions src/GreenDonut/src/GreenDonut.Data/Cursors/CursorKeyParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ public sealed class CursorKeyParser : ExpressionVisitor
protected override Expression VisitExtension(Expression node)
=> node.CanReduce ? base.VisitExtension(node) : node;

protected override Expression VisitLambda<T>(Expression<T> node) => node;

protected override Expression VisitMethodCall(MethodCallExpression node)
{
if (IsOrderBy(node))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,51 @@ public async Task BatchPaging_First_5()
snapshot.MatchMarkdownSnapshot();
}

[Fact]
public async Task ToBatchPageAsync_Should_PreserveNestedOrdering_When_PredicateContainsOrderBy()
{
// Arrange
var snapshot = Snapshot.Create(postFix: TestEnvironment.TargetFramework);

var connectionString = CreateConnectionString();
await SeedAsync(connectionString);
using var capture = new CapturePagingQueryInterceptor();

// Act
await using var context = new CatalogContext(connectionString);

var results = await context.Products
.Where(t =>
(t.BrandId == 1 || t.BrandId == 2 || t.BrandId == 3)
&& t.Brand!.Products
.AsQueryable()
.OrderByDescending(p => p.Name)
.First()
.Id > 0)
.OrderBy(p => p.Name)
.ThenBy(p => p.Id)
.ToBatchPageAsync(
k => k.BrandId,
new PagingArguments { First = 2 },
Xunit.TestContext.Current.CancellationToken);

// Assert
foreach (var page in results)
{
snapshot.Add(
new
{
First = page.Value.CreateStartCursor(),
Last = page.Value.CreateEndCursor(),
Items = page.Value.Items.Select(t => new { t.Id, t.Name })
},
name: page.Key.ToString());
}

snapshot.AddQueries(capture.Queries);
snapshot.MatchMarkdownSnapshot();
}

[Fact]
public async Task BatchPaging_Last_5()
{
Expand Down
Loading