Skip to content

CSHARP-5628: Add new boolean expression simplifications to PartialEvaluator #1803

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+318 −34
Open

CSHARP-5628: Add new boolean expression simplifications to PartialEvaluator #1803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
56a5854
CSHARP-5628: Add new boolean expression simplifications to PartialEva…
rstam Oct 28, 2025
631be11
CSHARP-5628: Requested changes.
rstam Oct 28, 2025
e29abd4
CSHARP-56j28: Requested changes
rstam Nov 10, 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
154 changes: 124 additions & 30 deletions src/MongoDB.Driver/Linq/Linq3Implementation/Misc/PartialEvaluator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ public static Expression EvaluatePartially(Expression expression)
// nested types
private class SubtreeEvaluator : ExpressionVisitor
{
// private static fields
private static readonly Expression __falseConstantExpression = Expression.Constant(false, typeof(bool));
private static readonly Expression __trueConstantExpression = Expression.Constant(true, typeof(bool));

// private fields
private readonly HashSet<Expression> _candidates;

Expand All @@ -75,60 +79,137 @@ public override Expression Visit(Expression expression)

protected override Expression VisitBinary(BinaryExpression node)
{
if (node.NodeType == ExpressionType.AndAlso)
var leftExpression = node.Left;
var rightExpression = node.Right;

if (leftExpression.Type == typeof(bool) && rightExpression.Type == typeof(bool))
{
var leftExpression = Visit(node.Left);
if (leftExpression is ConstantExpression constantLeftExpression )
if (node.NodeType == ExpressionType.AndAlso)
Copy link
Contributor Author

@rstam rstam Oct 28, 2025

Choose a reason for hiding this comment

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

Normally we like to do simplifications in the AstSimplifier but the existing simplifications for && and || were already being handled here.

VisitBinary is essentially the same as before but modified to use the new IsConstant helper method.

{
var value = (bool)constantLeftExpression.Value;
return value ? Visit(node.Right) : Expression.Constant(false);
leftExpression = Visit(leftExpression);
if (IsConstant<bool>(leftExpression, out var leftValue))
{
// true && Q => Q
// false && Q => false
return leftValue ? Visit(rightExpression) : __falseConstantExpression;
}

rightExpression = Visit(rightExpression);
if (IsConstant<bool>(rightExpression, out var rightValue))
{
// P && true => P
// P && false => false
return rightValue ? leftExpression : __falseConstantExpression;
}

return node.Update(leftExpression, conversion: null, rightExpression);
}

var rightExpression = Visit(node.Right);
if (rightExpression is ConstantExpression constantRightExpression)
if (node.NodeType == ExpressionType.OrElse)
{
var value = (bool)constantRightExpression.Value;
return value ? leftExpression : Expression.Constant(false);
leftExpression = Visit(leftExpression);
if (IsConstant<bool>(leftExpression, out var leftValue))
{
// true || Q => true
// false || Q => Q
return leftValue ? __trueConstantExpression : Visit(rightExpression);
}

rightExpression = Visit(rightExpression);
if (IsConstant<bool>(rightExpression, out var rightValue))
{
// P || true => true
// P || false => P
return rightValue ? __trueConstantExpression : leftExpression;
}

return node.Update(leftExpression, conversion: null, rightExpression);
}
}

return base.VisitBinary(node);
}

return node.Update(leftExpression, conversion: null, rightExpression);
protected override Expression VisitConditional(ConditionalExpression node)
{
var test = Visit(node.Test);

if (IsConstant<bool>(test, out var testValue))
{
// true ? A : B => A
// false ? A : B => B
return testValue ? Visit(node.IfTrue) : Visit(node.IfFalse);
Copy link
Contributor Author

@rstam rstam Oct 28, 2025

Choose a reason for hiding this comment

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

This was an existing simplification.

}

if (node.NodeType == ExpressionType.OrElse)
var ifTrue = Visit(node.IfTrue);
var ifFalse = Visit(node.IfFalse);

if (BothAreConstant<bool>(ifTrue, ifFalse, out var ifTrueValue, out var ifFalseValue))
{
var leftExpression = Visit(node.Left);
if (leftExpression is ConstantExpression constantLeftExpression)
return (ifTrueValue, ifFalseValue) switch
{
var value = (bool)constantLeftExpression.Value;
return value ? Expression.Constant(true) : Visit(node.Right);
}
(false, false) => __falseConstantExpression, // T ? false : false => false
(false, true) => Expression.Not(test), // T ? false : true => !T
(true, false) => test, // T ? true : false => T
(true, true) => __trueConstantExpression // T ? true : true => true
};
}
else if (IsConstant<bool>(ifTrue, out ifTrueValue))
{
// T ? true : Q => T || Q
// T ? false : Q => !T && Q
return ifTrueValue
? Visit(Expression.OrElse(test, ifFalse))
: Visit(Expression.AndAlso(Expression.Not(test), ifFalse));
}
else if (IsConstant<bool>(ifFalse, out ifFalseValue))
{
// T ? P : true => !T || P
// T ? P : false => T && P
return ifFalseValue
? Visit(Expression.OrElse(Expression.Not(test), ifTrue))
: Visit(Expression.AndAlso(test, ifTrue));
}

var rightExpression = Visit(node.Right);
if (rightExpression is ConstantExpression constantRightExpression)
return node.Update(test, ifTrue, ifFalse);
}

protected override Expression VisitUnary(UnaryExpression node)
{
var operand = Visit(node.Operand);

if (node.Type == typeof(bool) &&
node.NodeType == ExpressionType.Not)
{
if (operand is UnaryExpression innerUnaryExpressionOperand &&
innerUnaryExpressionOperand.NodeType == ExpressionType.Not)
{
var value = (bool)constantRightExpression.Value;
return value ? Expression.Constant(true) : leftExpression;
// !!P => P
return innerUnaryExpressionOperand.Operand;
}

return node.Update(leftExpression, conversion: null, rightExpression);
}

return base.VisitBinary(node);
return node.Update(operand);
}

protected override Expression VisitConditional(ConditionalExpression node)
// private methods
private bool BothAreConstant<T>(Expression expression1, Expression expression2, out T constantValue1, out T constantValue2)
{
var test = Visit(node.Test);
if (test is ConstantExpression constantTestExpression)
if (expression1 is ConstantExpression constantExpression1 &&
expression2 is ConstantExpression constantExpression2 &&
constantExpression1.Type == typeof(T) &&
constantExpression2.Type == typeof(T))
{
var value = (bool)constantTestExpression.Value;
return value ? Visit(node.IfTrue) : Visit(node.IfFalse);
constantValue1 = (T)constantExpression1.Value;
constantValue2 = (T)constantExpression2.Value;
return true;
}

return node.Update(test, Visit(node.IfTrue), Visit(node.IfFalse));
constantValue1 = default;
constantValue2 = default;
return false;
}

// private methods
private Expression Evaluate(Expression expression)
{
if (expression.NodeType == ExpressionType.Constant)
Expand All @@ -139,6 +220,19 @@ private Expression Evaluate(Expression expression)
Delegate fn = lambda.Compile();
return Expression.Constant(fn.DynamicInvoke(null), expression.Type);
}

private bool IsConstant<T>(Expression expression, out T constantValue)
{
if (expression is ConstantExpression constantExpression1 &&
constantExpression1.Type == typeof(T))
{
constantValue = (T)constantExpression1.Value;
return true;
}

constantValue = default;
return false;
}
}

private class Nominator : ExpressionVisitor
Expand Down
8 changes: 4 additions & 4 deletions tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/Jira/CSharp4337Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ public class CSharp4337Tests : LinqIntegrationTest<CSharp4337Tests.ClassFixture>
{
private static (Expression<Func<C, R<bool>>> Projection, string ExpectedStage, bool[] ExpectedResults)[] __predicate_should_use_correct_representation_test_cases = new (Expression<Func<C, R<bool>>> Projection, string ExpectedStage, bool[] ExpectedResults)[]
{
(d => new R<bool> { N = d.Id, V = d.I1 == E.E1 ? true : false }, "{ $project : { N : '$_id', V : { $cond : { if : { $eq : ['$I1', 1] }, then : true, else : false } }, _id : 0 } }", new[] { true, false }),
(d => new R<bool> { N = d.Id, V = d.S1 == E.E1 ? true : false }, "{ $project : { N : '$_id', V : { $cond : { if : { $eq : ['$S1', 'E1'] }, then : true, else : false } }, _id : 0 } }", new[] { true, false }),
(d => new R<bool> { N = d.Id, V = E.E1 == d.I1 ? true : false }, "{ $project : { N : '$_id', V : { $cond : { if : { $eq : [1, '$I1'] }, then : true, else : false } }, _id : 0 } }", new[] { true, false }),
(d => new R<bool> { N = d.Id, V = E.E1 == d.S1 ? true : false }, "{ $project : { N : '$_id', V : { $cond : { if : { $eq : ['E1', '$S1'] }, then : true, else : false } }, _id : 0 } }", new[] { true, false })
(d => new R<bool> { N = d.Id, V = d.I1 == E.E1 ? true : false }, "{ $project : { N : '$_id', V : { $eq : ['$I1', 1] }, _id : 0 } }", new[] { true, false }),
(d => new R<bool> { N = d.Id, V = d.S1 == E.E1 ? true : false }, "{ $project : { N : '$_id', V : { $eq : ['$S1', 'E1'] }, _id : 0 } }", new[] { true, false }),
(d => new R<bool> { N = d.Id, V = E.E1 == d.I1 ? true : false }, "{ $project : { N : '$_id', V : { $eq : [1, '$I1'] }, _id : 0 } }", new[] { true, false }),
(d => new R<bool> { N = d.Id, V = E.E1 == d.S1 ? true : false }, "{ $project : { N : '$_id', V : { $eq : ['E1', '$S1'] }, _id : 0 } }", new[] { true, false })
Copy link
Contributor Author

@rstam rstam Oct 28, 2025

Choose a reason for hiding this comment

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

This is the only existing test affected by these new simplifications.

};

public CSharp4337Tests(ClassFixture fixture)
Expand Down
Loading