Skip to content

CSHARP-5785: Optimize LINQ translation for First() and FirstOrDefault() methods with predicates #1817

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Nov 21, 2025
Merged

CSHARP-5785: Optimize LINQ translation for First() and FirstOrDefault() methods with predicates #1817

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
984874b
CSHARP-5785: Optimize LINQ translation for First() and FirstOrDefault…
adelinowona Nov 18, 2025
d5f7846
fix some more tests
adelinowona Nov 18, 2025
e13fd75
gate $filter limit optimization based on server version
adelinowona Nov 19, 2025
8cc0d37
Restore UTF-8 BOM to edited files
adelinowona Nov 19, 2025
fed12d1
adress review comments
adelinowona Nov 20, 2025
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
23 changes: 19 additions & 4 deletions ...essionTranslators/MethodTranslators/FirstOrLastMethodToAggregationExpressionTranslator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* Copyright 2010-present MongoDB Inc.
/* Copyright 2010-present MongoDB Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -95,16 +95,31 @@ public static TranslatedExpression Translate(TranslationContext context, MethodC
var sourceAst = sourceTranslation.Ast;
var itemSerializer = ArraySerializerHelper.GetItemSerializer(sourceTranslation.Serializer);

var isFirstMethod = method.Name.StartsWith("First");
Copy link
Contributor

@rstam rstam Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general we prefer to test which method it is like this:

var isFirstMethod = method.IsOneOf(__firstMethods);

It is less likely to have false matches than string matching (partial or otherwise).

Copy link
Contributor Author

@adelinowona adelinowona Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did it this way since I reasoned that if we passed the initial if statement:
if (method.IsOneOf(__firstOrLastMethods)) then we already know that the method has matched one of the first methods so no need to test the methods again.

Copy link
Contributor

@rstam rstam Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The statement you make is true, but even so we prefer to stick to method.IsOneOf instead of string matching.

The only time we don't use method.IsOneOf is when we are translating a method that might be implemented by classes we don't even know about (e.g. CompareTo).


if (method.IsOneOf(__withPredicateMethods))
{
var predicateLambda = ExpressionHelper.UnquoteLambdaIfQueryableMethod(method, arguments[1]);
var parameterExpression = predicateLambda.Parameters.Single();
var parameterSymbol = context.CreateSymbol(parameterExpression, itemSerializer, isCurrent: false);
var predicateTranslation = ExpressionToAggregationExpressionTranslator.TranslateLambdaBody(context, predicateLambda, parameterSymbol);

// The $filter limit parameter was introduced in MongoDB 6.0
Copy link
Contributor

@rstam rstam Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment becomes redundant and can be removed if you accept the change recommended on line 112 as then the code becomes self documenting.

AstExpression limit = null;
if (isFirstMethod)
{
var compatibilityLevel = context.TranslationOptions.CompatibilityLevel;
if (compatibilityLevel is >= ServerVersion.Server60)
Copy link
Contributor

@rstam rstam Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather this line say:

if (Feature.FilterLimit.IsSupported(compatibilityLevel.ToWireVersion()))

{
limit = AstExpression.Constant(1);
}
}

sourceAst = AstExpression.Filter(
input: sourceAst,
cond: predicateTranslation.Ast,
@as: parameterSymbol.Var.Name);
@as: parameterSymbol.Var.Name,
limit: limit);
}

AstExpression ast;
Expand All @@ -119,11 +134,11 @@ public static TranslatedExpression Translate(TranslationContext context, MethodC
@in: AstExpression.Cond(
@if: AstExpression.Eq(AstExpression.Size(valuesAst), 0),
then: serializedDefaultValue,
@else: method.IsOneOf(__firstMethods) ? AstExpression.First(valuesAst) : AstExpression.Last(valuesAst)));
@else: isFirstMethod ? AstExpression.First(valuesAst) : AstExpression.Last(valuesAst)));
}
else
{
ast = method.Name == "First" ? AstExpression.First(sourceAst) : AstExpression.Last(sourceAst);
ast = isFirstMethod ? AstExpression.First(sourceAst) : AstExpression.Last(sourceAst);
}

return new TranslatedExpression(expression, ast, itemSerializer);
Expand Down
89 changes: 64 additions & 25 deletions tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/Jira/CSharp4048Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using System.Collections.Generic;
using System.Linq;
using FluentAssertions;
using MongoDB.Driver.Core.Misc;
using MongoDB.Driver.Linq;
using MongoDB.Driver.TestHelpers;
using Xunit;
Expand All @@ -24,6 +25,8 @@ namespace MongoDB.Driver.Tests.Linq.Linq3Implementation.Jira
{
public class CSharp4048Tests : LinqIntegrationTest<CSharp4048Tests.ClassFixture>
{
private static readonly bool FilterLimitIsSupported = Feature.FilterLimit.IsSupported(CoreTestConfiguration.MaxWireVersion);

public CSharp4048Tests(ClassFixture fixture)
: base(fixture)
{
Expand Down Expand Up @@ -118,7 +121,7 @@ public void List_get_Item_of_scalar_should_work()
var expectedStages = new[]
{
"{ $group : { _id : '$_id', _elements : { $push: '$X' } } }",
"{ $project : { _id : '$_id' Result : { $arrayElemAt : ['$_elements', 0] } } }",
Copy link
Contributor

@rstam rstam Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How was this test not failing?

Copy link
Contributor Author

@adelinowona adelinowona Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea. I just happened to catch it.

"{ $project : { _id : '$_id', Result : { $arrayElemAt : ['$_elements', 0] } } }",
"{ $sort : { _id : 1 } }"
};
AssertStages(stages, expectedStages);
Expand Down Expand Up @@ -1040,12 +1043,21 @@ public void IGrouping_First_with_predicate_of_root_should_work()
.OrderBy(x => x.Id);

var stages = Translate(collection, queryable);
var expectedStages = new[]
{
"{ $group : { _id : '$_id', _elements : { $push : '$$ROOT' } } }",
"{ $project : { _id : '$_id', Result : { $arrayElemAt : [{ $filter : { input : '$_elements', as : 'e', cond : { $ne : ['$$e.X', 1] } } }, 0] } } }",
"{ $sort : { _id : 1 } }"
};

var expectedStages = FilterLimitIsSupported
Copy link
Contributor

@rstam rstam Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I almost find the longer version more readable:

var expectedStages = Feature.FilterLimit.IsSupported(CoreTestConfiguration.MaxWireVersion)

because I don't have to chase down how FilterLimitIsSupported is computed.

In some other files you still use the long form.

Copy link
Contributor Author

@adelinowona adelinowona Nov 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather keep the FilterLimitIsSupported. I didn't want to have to keep repeating the longer form everywhere especially when it is the same information that we can get once and just reuse everywhere.

In some other files. there is only one use case, which is why I just used the longer form directly.

? new[]
{
"{ $group : { _id : '$_id', _elements : { $push : '$$ROOT' } } }",
"{ $project : { _id : '$_id', Result : { $arrayElemAt : [{ $filter : { input : '$_elements', as : 'e', cond : { $ne : ['$$e.X', 1] }, limit : 1 } }, 0] } } }",
"{ $sort : { _id : 1 } }"
}
: new[]
{
"{ $group : { _id : '$_id', _elements : { $push : '$$ROOT' } } }",
"{ $project : { _id : '$_id', Result : { $arrayElemAt : [{ $filter : { input : '$_elements', as : 'e', cond : { $ne : ['$$e.X', 1] } } }, 0] } } }",
"{ $sort : { _id : 1 } }"
};

AssertStages(stages, expectedStages);

var results = queryable.ToList();
Expand All @@ -1065,12 +1077,21 @@ public void IGrouping_First_with_predicate_of_scalar_should_work()
.OrderBy(x => x.Id);

var stages = Translate(collection, queryable);
var expectedStages = new[]
{
"{ $group : { _id : '$_id', _elements : { $push : '$X' } } }",
"{ $project : { _id : '$_id', Result : { $arrayElemAt : [{ $filter : { input : '$_elements', as : 'e', cond : { $ne : ['$$e', 1] } } }, 0] } } }",
"{ $sort : { _id : 1 } }"
};

var expectedStages = FilterLimitIsSupported
? new[]
{
"{ $group : { _id : '$_id', _elements : { $push : '$X' } } }",
"{ $project : { _id : '$_id', Result : { $arrayElemAt : [{ $filter : { input : '$_elements', as : 'e', cond : { $ne : ['$$e', 1] }, limit : 1 } }, 0] } } }",
"{ $sort : { _id : 1 } }"
}
: new[]
{
"{ $group : { _id : '$_id', _elements : { $push : '$X' } } }",
"{ $project : { _id : '$_id', Result : { $arrayElemAt : [{ $filter : { input : '$_elements', as : 'e', cond : { $ne : ['$$e', 1] } } }, 0] } } }",
"{ $sort : { _id : 1 } }"
};

AssertStages(stages, expectedStages);

var results = queryable.ToList();
Expand Down Expand Up @@ -1140,12 +1161,21 @@ public void IGrouping_FirstOrDefault_with_predicate_of_root_should_work()
.OrderBy(x => x.Id);

var stages = Translate(collection, queryable);
var expectedStages = new[]
{
"{ $group : { _id : '$_id', _elements : { $push : '$$ROOT' } } }",
"{ $project : { _id : '$_id', Result : { $let : { vars : { values : { $filter : { input : '$_elements', as : 'e', cond : { $ne : ['$$e.X', 1] } } } }, in : { $cond : { if : { $eq : [{ $size : '$$values' }, 0] }, then : null, else : { $arrayElemAt : ['$$values', 0] } } } } } } }",
"{ $sort : { _id : 1 } }"
};

var expectedStages = FilterLimitIsSupported
? new[]
{
"{ $group : { _id : '$_id', _elements : { $push : '$$ROOT' } } }",
"{ $project : { _id : '$_id', Result : { $let : { vars : { values : { $filter : { input : '$_elements', as : 'e', cond : { $ne : ['$$e.X', 1] }, limit : 1 } } }, in : { $cond : { if : { $eq : [{ $size : '$$values' }, 0] }, then : null, else : { $arrayElemAt : ['$$values', 0] } } } } } } }",
"{ $sort : { _id : 1 } }"
}
: new[]
{
"{ $group : { _id : '$_id', _elements : { $push : '$$ROOT' } } }",
"{ $project : { _id : '$_id', Result : { $let : { vars : { values : { $filter : { input : '$_elements', as : 'e', cond : { $ne : ['$$e.X', 1] } } } }, in : { $cond : { if : { $eq : [{ $size : '$$values' }, 0] }, then : null, else : { $arrayElemAt : ['$$values', 0] } } } } } } }",
"{ $sort : { _id : 1 } }"
};

AssertStages(stages, expectedStages);

var results = queryable.ToList();
Expand All @@ -1166,12 +1196,21 @@ public void IGrouping_FirstOrDefault_with_predicate_of_scalar_should_work()
.OrderBy(x => x.Id);

var stages = Translate(collection, queryable);
var expectedStages = new[]
{
"{ $group : { _id : '$_id', _elements : { $push : '$X' } } }",
"{ $project : { _id : '$_id', Result : { $let : { vars : { values : { $filter : { input : '$_elements', as : 'e', cond : { $ne : ['$$e', 1] } } } }, in : { $cond : { if : { $eq : [{ $size : '$$values' }, 0] }, then : 0, else : { $arrayElemAt : ['$$values', 0] } } } } } } }",
"{ $sort : { _id : 1 } }"
};

var expectedStages = FilterLimitIsSupported
? new[]
{
"{ $group : { _id : '$_id', _elements : { $push : '$X' } } }",
"{ $project : { _id : '$_id', Result : { $let : { vars : { values : { $filter : { input : '$_elements', as : 'e', cond : { $ne : ['$$e', 1] }, limit : 1 } } }, in : { $cond : { if : { $eq : [{ $size : '$$values' }, 0] }, then : 0, else : { $arrayElemAt : ['$$values', 0] } } } } } } }",
"{ $sort : { _id : 1 } }"
}
: new[]
{
"{ $group : { _id : '$_id', _elements : { $push : '$X' } } }",
"{ $project : { _id : '$_id', Result : { $let : { vars : { values : { $filter : { input : '$_elements', as : 'e', cond : { $ne : ['$$e', 1] } } } }, in : { $cond : { if : { $eq : [{ $size : '$$values' }, 0] }, then : 0, else : { $arrayElemAt : ['$$values', 0] } } } } } } }",
"{ $sort : { _id : 1 } }"
};

AssertStages(stages, expectedStages);

var results = queryable.ToList();
Expand Down
11 changes: 10 additions & 1 deletion tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/Jira/CSharp5258Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using System.Collections.Generic;
using System.Linq;
using FluentAssertions;
using MongoDB.Driver.Core.Misc;
using MongoDB.Driver.TestHelpers;
using Xunit;

Expand All @@ -37,7 +38,15 @@ public void Select_First_with_predicate_should_work()
.Select(_x => _x.List.First(_y => _y > 2));

var stages = Translate(collection, queryable);
AssertStages(stages, "{ $project : { _v : { $arrayElemAt : [{ $filter : { input : '$List', as : 'v__0', cond : { $gt : ['$$v__0', 2] } } }, 0] }, _id : 0 } }");

if (Feature.FilterLimit.IsSupported(CoreTestConfiguration.MaxWireVersion))
{
AssertStages(stages, "{ $project : { _v : { $arrayElemAt : [{ $filter : { input : '$List', as : 'v__0', cond : { $gt : ['$$v__0', 2] }, limit : 1 } }, 0] }, _id : 0 } }");
}
else
{
AssertStages(stages, "{ $project : { _v : { $arrayElemAt : [{ $filter : { input : '$List', as : 'v__0', cond : { $gt : ['$$v__0', 2] } } }, 0] }, _id : 0 } }");
}

var results = queryable.ToList();
results.Should().Equal(3, 4);
Expand Down
Loading