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

rstam · 2025-10-28T12:34:49Z

No description provided.

…luator

rstam · 2025-10-28T12:37:21Z

src/MongoDB.Driver/Linq/Linq3Implementation/Misc/PartialEvaluator.cs

                {
-                    var leftExpression = Visit(node.Left);
-                    if (leftExpression is ConstantExpression constantLeftExpression )
+                    if (node.NodeType == ExpressionType.AndAlso)


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.

rstam · 2025-10-28T12:37:57Z

src/MongoDB.Driver/Linq/Linq3Implementation/Misc/PartialEvaluator.cs

+                {
+                    // true ? A : B => A
+                    // false ? A : B => B
+                    return testValue ? Visit(node.IfTrue) : Visit(node.IfFalse);


This was an existing simplification.

rstam · 2025-10-28T12:40:03Z

tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/Jira/CSharp4337Tests.cs

+            (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 })


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

Copilot

Pull Request Overview

This PR implements boolean expression simplifications for MongoDB LINQ queries to generate more efficient aggregation pipelines. The changes optimize boolean logic patterns (e.g., x ? true : false → x, !!x → x) in both Select and Where clauses.

Key changes:

Enhanced PartialEvaluator to simplify boolean expressions including conditional expressions, logical operators, and double negations
Updated expected MongoDB aggregation stages in existing tests to reflect simplified outputs
Added comprehensive test coverage for 23 different boolean simplification patterns

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 56 comments.

File	Description
`src/MongoDB.Driver/Linq/Linq3Implementation/Misc/PartialEvaluator.cs`	Expanded expression visitor with boolean simplification logic for conditionals, binary operations (AndAlso/OrElse), and unary operations (double negation)
`tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/Jira/CSharp5628Tests.cs`	New test file with comprehensive test cases validating boolean simplifications in Select and Where clauses
`tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/Jira/CSharp4337Tests.cs`	Updated test expectations to match simplified pipeline stages (removed redundant `$cond` expressions)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-10-28T20:35:28Z

tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/Jira/CSharp5628Tests.cs

+        var collection = Fixture.Collection;
+
+        // see: https://codeql.github.com/codeql-query-help/csharp/cs-simplifiable-boolean-expression/#recommendation
+        // not all simplifciations listed there are safe for a database (because of possibly missing fields or tri-valued logic)


Corrected spelling of 'simplifciations' to 'simplifications'.

Suggested change

// not all simplifciations listed there are safe for a database (because of possibly missing fields or tri-valued logic)

// not all simplifications listed there are safe for a database (because of possibly missing fields or tri-valued logic)

Copilot · 2025-10-28T20:35:28Z

src/MongoDB.Driver/Linq/Linq3Implementation/Misc/PartialEvaluator.cs

            {
-                if (node.NodeType == ExpressionType.AndAlso)
+                var leftExpression = node.Left;
+                var rightExpression =  node.Right;


Extra space between '=' and 'node.Right'. Should be single space for consistency.

Suggested change

var rightExpression = node.Right;

var rightExpression = node.Right;

Copilot · 2025-10-28T20:35:29Z

src/MongoDB.Driver/Linq/Linq3Implementation/Misc/PartialEvaluator.cs


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


Extra space between '=' and 'Visit'. Should be single space for consistency.

Suggested change

var ifFalse = Visit(node.IfFalse);

var ifFalse = Visit(node.IfFalse);

Copilot · 2025-10-28T20:35:29Z

src/MongoDB.Driver/Linq/Linq3Implementation/Misc/PartialEvaluator.cs

+                        ? Visit(Expression.Or(test, ifFalse))
+                        : Visit(Expression.And(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.Or(Expression.Not(test), ifTrue))
+                        : Visit(Expression.And(test, ifTrue));


Using Expression.Or and Expression.And instead of Expression.OrElse and Expression.AndAlso may cause issues. These methods create bitwise operations for integral types, not short-circuit logical operations. Use Expression.OrElse and Expression.AndAlso for boolean logic to ensure proper short-circuit evaluation.

Suggested change

? Visit(Expression.Or(test, ifFalse))

: Visit(Expression.And(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.Or(Expression.Not(test), ifTrue))

: Visit(Expression.And(test, ifTrue));

? 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));

Copilot · 2025-10-28T20:35:29Z

src/MongoDB.Driver/Linq/Linq3Implementation/Misc/PartialEvaluator.cs

+                        ? Visit(Expression.Or(test, ifFalse))
+                        : Visit(Expression.And(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.Or(Expression.Not(test), ifTrue))
+                        : Visit(Expression.And(test, ifTrue));


Using Expression.Or and Expression.And instead of Expression.OrElse and Expression.AndAlso may cause issues. These methods create bitwise operations for integral types, not short-circuit logical operations. Use Expression.OrElse and Expression.AndAlso for boolean logic to ensure proper short-circuit evaluation.

Suggested change

? Visit(Expression.Or(test, ifFalse))

: Visit(Expression.And(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.Or(Expression.Not(test), ifTrue))

: Visit(Expression.And(test, ifTrue));

? 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));

tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/Jira/CSharp5628Tests.cs

adelinowona

LGTM

sanych-sun

LGTM + Minor comments. Also should we unit-test the new translations done by the PartialEvaluator?

sanych-sun · 2025-11-10T18:15:57Z

src/MongoDB.Driver/Linq/Linq3Implementation/Misc/PartialEvaluator.cs

+                        {
+                            // true && Q => Q
+                            // false && Q => false
+                            return leftValue ? Visit(rightExpression) : Expression.Constant(false);


Minor: can save a little on allocation probably by returning leftExpression instead of Expression.Constant(false).

Using leftExpression is a bit subtle. It requires understanding why that works.

Instead I added __falseConstantExpression and __trueConstantExpression fields that can be used for both left and right sides.

sanych-sun · 2025-11-10T18:16:37Z

src/MongoDB.Driver/Linq/Linq3Implementation/Misc/PartialEvaluator.cs

+                        {
+                            // P && true => P
+                            // P && false => false
+                            return rightValue ? leftExpression : Expression.Constant(false);


Minor: same as above can return rightExpression instead of Expression.Constant(false).

Used __falseConstantExpression and __trueConstantExpression instead.

sanych-sun · 2025-11-10T18:32:34Z

src/MongoDB.Driver/Linq/Linq3Implementation/Misc/PartialEvaluator.cs

+                        {
+                            // true || Q => true
+                            // false || Q => Q
+                            return leftValue ? Expression.Constant(true) : Visit(rightExpression);


Same as above.

Used __falseConstantExpression and __trueConstantExpression instead.

sanych-sun · 2025-11-10T18:32:43Z

src/MongoDB.Driver/Linq/Linq3Implementation/Misc/PartialEvaluator.cs

+                        {
+                            // P || true => true
+                            // P || false => P
+                            return rightValue ? Expression.Constant(true) : leftExpression;


Same as above.

Used __falseConstantExpression and __trueConstantExpression instead.

CSHARP-5628: Add new boolean expression simplifications to PartialEva…

56a5854

…luator

rstam requested review from adelinowona and sanych-sun October 28, 2025 12:34

rstam requested a review from a team as a code owner October 28, 2025 12:34

rstam added the improvement Optimizations or refactoring (no new features or fixes). label Oct 28, 2025

rstam commented Oct 28, 2025

View reviewed changes

BorisDog requested a review from Copilot October 28, 2025 20:22

Copilot AI reviewed Oct 28, 2025

View reviewed changes

CSHARP-5628: Requested changes.

631be11

adelinowona previously approved these changes Oct 30, 2025

View reviewed changes

sanych-sun previously approved these changes Nov 10, 2025

View reviewed changes

CSHARP-56j28: Requested changes

e29abd4

rstam dismissed stale reviews from sanych-sun and adelinowona via e29abd4 November 10, 2025 21:27

	// not all simplifciations listed there are safe for a database (because of possibly missing fields or tri-valued logic)
	// not all simplifications listed there are safe for a database (because of possibly missing fields or tri-valued logic)

	var rightExpression = node.Right;
	var rightExpression = node.Right;

	var ifFalse = Visit(node.IfFalse);
	var ifFalse = Visit(node.IfFalse);

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

Are you sure you want to change the base?

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

Conversation

rstam commented Oct 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adelinowona left a comment

Choose a reason for hiding this comment

Uh oh!

sanych-sun left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sanych-sun left a comment •

edited

Loading