CSHARP-5730: Support static String.Compare method #1789

rstam · 2025-10-07T19:42:46Z

No description provided.

sanych-sun · 2025-10-15T17:23:24Z

...ionExpressionTranslators/MethodTranslators/CompareMethodToAggregationExpressionTranslator.cs

+                var strATranslation = ExpressionToAggregationExpressionTranslator.Translate(context, strAExpression);
+                var strBExpression = arguments[1];
+                var strBTranslation = ExpressionToAggregationExpressionTranslator.Translate(context, strBExpression);
+                var ast = AstExpression.Cmp(strATranslation.Ast, strBTranslation.Ast);


Should we add support for case insensitive/more overloads comparison by translating to $strcasecmp?

I suppose if we do this, we will have to support this compare overload -> https://learn.microsoft.com/en-us/dotnet/api/system.string.compare?view=net-9.0#system-string-compare(system-string-system-string-system-boolean)

adelinowona · 2025-10-15T19:25:47Z

...ionExpressionTranslators/MethodTranslators/CompareMethodToAggregationExpressionTranslator.cs

+                var strATranslation = ExpressionToAggregationExpressionTranslator.Translate(context, strAExpression);
+                var strBExpression = arguments[1];
+                var strBTranslation = ExpressionToAggregationExpressionTranslator.Translate(context, strBExpression);
+                var ast = AstExpression.Cmp(strATranslation.Ast, strBTranslation.Ast);


I suppose if we do this, we will have to support this compare overload -> https://learn.microsoft.com/en-us/dotnet/api/system.string.compare?view=net-9.0#system-string-compare(system-string-system-string-system-boolean)

adelinowona · 2025-10-15T20:48:38Z

...ToFilterTranslators/ExpressionTranslators/CompareToComparisonExpressionToFilterTranslator.cs

Given that this class was extended to support the static string.Compare method, it seems its name is a bit outdated now? I would suggest updating the class name though I don't have a good suggestion maybe StringComparisonExpressionToFilterTranslator?

adelinowona · 2025-10-15T20:50:34Z

...ToFilterTranslators/ExpressionTranslators/CompareToComparisonExpressionToFilterTranslator.cs

            throw new ExpressionNotSupportedException(expression);
        }
+
+        private static bool IsStaticCompareMethod(MethodInfo method)


The IsStaticCompareMethod doesn't actually check if the method is named "Compare". I doubt this is intentional?

…sion.

rstam

Did some refactoring and added support for the overload of Compare that has an ignoreCase parameter.

rstam · 2025-10-22T14:38:29Z

src/MongoDB.Driver/Linq/Linq3Implementation/Ast/Filters/AstComparisonFilterOperator.cs


    internal static class AstComparisonFilterOperatorExtensions
    {
+        public static AstComparisonFilterOperator GetComparisonOperatorForSwappedLeftAndRight(this AstComparisonFilterOperator @operator)


Put here so that it can be used from more than one place.

rstam · 2025-10-22T14:39:15Z

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

            return false;
        }

+        public static bool IsInstanceCompareToMethod(this MethodInfo method)


Put here so that it can be used from more than one place.

rstam · 2025-10-22T14:40:23Z

...ionExpressionTranslators/MethodTranslators/CompareMethodToAggregationExpressionTranslator.cs

+
+namespace MongoDB.Driver.Linq.Linq3Implementation.Translators.ExpressionToAggregationExpressionTranslators.MethodTranslators
+{
+    internal static class CompareMethodToAggregationExpressionTranslator


Compare and CompareTo handling is consolidated into a single class.

rstam · 2025-10-22T14:40:42Z

...onToFilterTranslators/ExpressionTranslators/CompareComparisonExpressionToFilterTranslator.cs

+
+namespace MongoDB.Driver.Linq.Linq3Implementation.Translators.ExpressionToFilterTranslators.ExpressionTranslators
+{
+    internal static class CompareComparisonExpressionToFilterTranslator


Compare and CompareTo handling is consolidated into a single class.

rstam · 2025-10-22T14:41:23Z

...ionExpressionTranslators/MethodTranslators/CompareMethodToAggregationExpressionTranslator.cs

+            {
+                Expression value1Expression;
+                Expression value2Expression;
+                if (method.IsStatic)


The main difference between Compare and CompareTo is whether it's a static method or not.

rstam · 2025-10-22T14:41:41Z

...onToFilterTranslators/ExpressionTranslators/CompareComparisonExpressionToFilterTranslator.cs

+
+                Expression fieldExpression;
+                Expression innerValueExpression;
+                if (compareMethod.IsStatic)


The main difference between Compare and CompareTo is whether it's a static method or not.

rstam · 2025-10-22T14:42:28Z

...xpressionToFilterTranslators/ExpressionTranslators/ComparisonExpressionToFilterTranslator.cs

            }
        }
-
-        private static AstComparisonFilterOperator GetComparisonOperatorForSwappedLeftAndRight(Expression expression)


Moved to an extension method so it can be used from more than one class.

rstam · 2025-10-22T14:43:57Z

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

+    [InlineData(11, false, "{ $match : { B : { $gte : 'A' } } }", new int[] { 1, 2, 3, 4, 5, 6 })]
+    [InlineData(12, false, "{ $match : { B : { $lte : 'A' } } }", new int[] { 1, 3 })]
+    [InlineData(1, true, "{ $match : { $expr : { $eq : [{ $strcasecmp : ['A', '$B'] }, -1] } } }", new int[] { 2, 5 })]
+    [InlineData(2, true, "{ $match : { $expr : { $eq : [{ $strcasecmp : ['A', '$B'] }, 0] } } }", new int[] { 1, 3, 4, 6 })]


This one might have been possible without using $expr by using regular expressions but I don't think it's worth the effort.

sanych-sun

LGTM + minor comments

sanych-sun · 2025-10-22T17:20:08Z

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

+
+namespace MongoDB.Driver.Tests.Linq.Linq3Implementation.Jira;
+
+public class CSharp5730Tests : LinqIntegrationTest<CSharp5730Tests.ClassFixture>


Do we have tests for CompareTo and IComparable<T>.CompareTo?

sanych-sun · 2025-10-22T17:24:58Z

...ionExpressionTranslators/MethodTranslators/CompareMethodToAggregationExpressionTranslator.cs

+
+                var value1Translation = ExpressionToAggregationExpressionTranslator.Translate(context, value1Expression);
+                var value2Translation = ExpressionToAggregationExpressionTranslator.Translate(context, value2Expression);
+


Should we check if the value1 and value2 has string representation?

…us commit.

Copilot

Pull Request Overview

This PR adds support for the static String.Compare method in LINQ queries, enabling translation of string comparison operations to MongoDB queries. The implementation extends the existing CompareTo method support to include both static and instance comparison methods, with optional case-insensitive comparison support.

Key Changes:

Unified comparison method translation by consolidating String.Compare, CompareTo, and IComparable<T>.CompareTo handling
Added support for case-insensitive string comparison using MongoDB's $strcasecmp operator
Refactored comparison operator swapping logic into a reusable extension method

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/Jira/CSharp5730Tests.cs	Comprehensive test suite covering all comparison scenarios with static Compare, instance CompareTo, and IComparable methods, testing both case-sensitive and case-insensitive comparisons
src/MongoDB.Driver/Linq/Linq3Implementation/Translators/ExpressionToFilterTranslators/ExpressionTranslators/ComparisonExpressionToFilterTranslator.cs	Refactored to use the new unified CompareComparisonExpressionToFilterTranslator and extracted operator swapping logic
src/MongoDB.Driver/Linq/Linq3Implementation/Translators/ExpressionToFilterTranslators/ExpressionTranslators/CompareToComparisonExpressionToFilterTranslator.cs	Removed file - functionality consolidated into CompareComparisonExpressionToFilterTranslator
src/MongoDB.Driver/Linq/Linq3Implementation/Translators/ExpressionToFilterTranslators/ExpressionTranslators/CompareComparisonExpressionToFilterTranslator.cs	New unified translator handling static Compare, instance CompareTo, and IComparable methods, including ignoreCase parameter support
src/MongoDB.Driver/Linq/Linq3Implementation/Translators/ExpressionToAggregationExpressionTranslators/MethodTranslators/CompareToMethodToAggregationExpressionTranslator.cs	Removed file - functionality consolidated into CompareMethodToAggregationExpressionTranslator
src/MongoDB.Driver/Linq/Linq3Implementation/Translators/ExpressionToAggregationExpressionTranslators/MethodTranslators/CompareMethodToAggregationExpressionTranslator.cs	New unified translator for aggregation expressions supporting both static and instance comparison methods, with conditional $strcasecmp usage
src/MongoDB.Driver/Linq/Linq3Implementation/Translators/ExpressionToAggregationExpressionTranslators/MethodCallExpressionToAggregationExpressionTranslator.cs	Updated routing to use new CompareMethodToAggregationExpressionTranslator for both Compare and CompareTo methods
src/MongoDB.Driver/Linq/Linq3Implementation/Reflection/StringMethod.cs	Added static field definitions for String.Compare methods (with and without ignoreCase parameter)
src/MongoDB.Driver/Linq/Linq3Implementation/Misc/MethodInfoExtensions.cs	Added extension methods to identify static Compare and instance CompareTo methods via reflection
src/MongoDB.Driver/Linq/Linq3Implementation/Ast/Filters/AstComparisonFilterOperator.cs	Added GetComparisonOperatorForSwappedLeftAndRight extension method to centralize operator reversal logic

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

Copilot · 2025-11-10T16:41:32Z

...onToFilterTranslators/ExpressionTranslators/CompareComparisonExpressionToFilterTranslator.cs

+                    var ignoreCase = ignoreCaseExpression.GetConstantValue<bool>(containingExpression: compareMethodCallExpression);
+                    if (ignoreCase)
+                    {
+                        throw new ExpressionNotSupportedException(compareMethodCallExpression, because: "ignoreCase must be false");


The error message 'ignoreCase must be false' is misleading in a filter context. The code should not throw an exception when ignoreCase is true; instead it should allow ignoreCase but not generate a simplified field comparison. The current implementation incorrectly restricts functionality that should work (though it may require $expr). Consider either supporting ignoreCase:true with $expr or clarifying the error message.

Suggested change

throw new ExpressionNotSupportedException(compareMethodCallExpression, because: "ignoreCase must be false");

throw new ExpressionNotSupportedException(compareMethodCallExpression, because: "Case-insensitive comparisons (ignoreCase: true) are not supported in this filter context.");

sanych-sun

LGTM

CSHARP-5730: Support static String.Compare method

fb728dc

rstam requested review from adelinowona and sanych-sun October 7, 2025 19:42

rstam requested a review from a team as a code owner October 7, 2025 19:42

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

sanych-sun requested changes Oct 15, 2025

View reviewed changes

adelinowona requested changes Oct 15, 2025

View reviewed changes

rstam added 6 commits October 18, 2025 19:07

CSHARP-5730: Work in progress to fully support Compare.

3a85339

CSHARP-5730: Add support for swapping left and right sides of compari…

f4e62c6

…sion.

CSHARP-5730: Added new tests for Select.

eea6c74

CSHARP-5730: Added tests for Select Compare field to field.

3852a30

CSHARP-5730: Added support for Compare with ignoreCase overloads.

d34f3d3

CSHARP-5730: Minor cleanup before requesting re-review.

b429c2e

rstam commented Oct 22, 2025

View reviewed changes

rstam requested review from adelinowona and sanych-sun October 22, 2025 14:46

sanych-sun previously approved these changes Oct 22, 2025

View reviewed changes

CSHARP-5730: Added tests for CompareTo.

bc8cb71

rstam dismissed sanych-sun’s stale review via bc8cb71 October 23, 2025 14:36

rstam requested a review from sanych-sun October 23, 2025 14:36

CSHARP-5730: Comitting a file that somehow got left out of the previo…

0d10969

…us commit.

adelinowona requested a review from Copilot November 10, 2025 16:40

Copilot AI reviewed Nov 10, 2025

View reviewed changes

sanych-sun approved these changes Nov 10, 2025

View reviewed changes


		namespace MongoDB.Driver.Tests.Linq.Linq3Implementation.Jira;

		public class CSharp5730Tests : LinqIntegrationTest<CSharp5730Tests.ClassFixture>


		var value1Translation = ExpressionToAggregationExpressionTranslator.Translate(context, value1Expression);
		var value2Translation = ExpressionToAggregationExpressionTranslator.Translate(context, value2Expression);

	throw new ExpressionNotSupportedException(compareMethodCallExpression, because: "ignoreCase must be false");
	throw new ExpressionNotSupportedException(compareMethodCallExpression, because: "Case-insensitive comparisons (ignoreCase: true) are not supported in this filter context.");

CSHARP-5730: Support static String.Compare method #1789

Are you sure you want to change the base?

CSHARP-5730: Support static String.Compare method #1789

Conversation

rstam commented Oct 7, 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!

adelinowona Oct 15, 2025 • 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!

rstam left a comment

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!

sanych-sun left a comment

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 Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

sanych-sun left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

adelinowona Oct 15, 2025 •

edited

Loading