CSHARP-5730: Support static String.Compare method #1789
Conversation
| var strATranslation = ExpressionToAggregationExpressionTranslator.Translate(context, strAExpression); | ||
| var strBExpression = arguments[1]; | ||
| var strBTranslation = ExpressionToAggregationExpressionTranslator.Translate(context, strBExpression); | ||
| var ast = AstExpression.Cmp(strATranslation.Ast, strBTranslation.Ast); |
There was a problem hiding this comment.
Should we add support for case insensitive/more overloads comparison by translating to $strcasecmp?
There was a problem hiding this comment.
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)
| var strATranslation = ExpressionToAggregationExpressionTranslator.Translate(context, strAExpression); | ||
| var strBExpression = arguments[1]; | ||
| var strBTranslation = ExpressionToAggregationExpressionTranslator.Translate(context, strBExpression); | ||
| var ast = AstExpression.Cmp(strATranslation.Ast, strBTranslation.Ast); |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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?
| throw new ExpressionNotSupportedException(expression); | ||
| } | ||
|
|
||
| private static bool IsStaticCompareMethod(MethodInfo method) |
There was a problem hiding this comment.
The IsStaticCompareMethod doesn't actually check if the method is named "Compare". I doubt this is intentional?
3 participants
No description provided.