LanguageNormalizingExpressionVisitor for VB.NET string comparisons

[roji]

Fixes #19592

Note that this currently contains no tests, since that would require us to at least reference Microsoft.VisualBasic.dll, which I can do (or even more extreme, create a VB project).

Summary of what this new preprocessing visitor does:

1. Rewrite calls to VB's CompareString (with constant compare param) to the corresponding calls to String.Compare
2. Rewrite String.Compare == 0, with StringComparison.Ordinal or no-param, to equality/inequality expressions
3. Convert String.Compare == 0, with StringComparison.OrdinalIgnoreCase, to String.Equals with StringComparison.OrdinalIgnoreCase

[bricelam]

Do we translate these overloads to SQL? Should false result in the overload without a StringComparison?

[roji]

Do we translate these overloads to SQL?

Not at the moment, but we may at some point in the future... PG actually supports str1 > str2 :)

Should false result in the overload without a StringComparison?

I'm not 100% sure... string.Compare without StringComparison is culture-sensitive (as opposed to when passing StringComparison.Ordinal). The docs for VB's CompareString say that the equality operator produces an expression with TextCompare=False, and if I'm reading everything correctly, the default value of Option Compare is binary, which is culture-insensitive. But it would be good to have another set of eyes to be sure.

[bricelam]

I think we've already put a stake in the ground stating that it's OK to ignore culture while querying. Just honoring the case-sensitivity of Option Compare seems sufficient. So, if using the non-StringComparison overload of StringCompare makes it translate to SQL, that sounds more ideal to me.

But I'm also happy to keep it as-is and wait for #1222.

[roji]

Makes sense, making the change.

[bricelam]

Part of #14572

[roji]

Added support for string.Equals(a1, a2, OrdinalIgnoreCase), will wait for your feedback on the first question.

[bricelam]

Should we handle >, >=, <, and <= too? Or does the compiler never generate those

[roji]

Or does the compiler never generate those

Not that I know of. In C#, string > string doesn't compile, and in VB.NET it compiles to CompareString(s1, s2, false) > 0; we do translate that above to string.Compare(s1, s2), and it turns out EF Core does translate that, so we get:

SELECT [b].[Id], [b].[Name]
FROM [Blogs] AS [b]
WHERE CASE
    WHEN N'hello' = [b].[Name] THEN 0
    WHEN N'hello' > [b].[Name] THEN 1
    WHEN N'hello' < [b].[Name] THEN -1
END > 0

[roji]

Note: opened #19733 to do better with that.

[smitpatel]

This should have converted to string.Compare in C# rather than trying to convert to equality.

[roji]

Any particular reason?

[smitpatel]

causes fragmentation in set of optimizations.

[roji]

I'm sure I'm just being slow or dumb, but could you take a minute and explain exactly what you mean? Fragmentation where, which set of optimizations...

[kev160967]

Should this be fixed in EF6? I am working on migrating a solution to net6 that has a VB project which contains EF queries. I'm getting the EmbeddedOperators.CompareString could not be translated error throughout the VB project, but not in the C# projects. I also note that LanguageNormalizingExpressionVisitor doesn't seem to be present in Microsoft.EntityFrameworkCore.Query.Internal in net6. I'm about to take a look at creating it locally and using it as a solution, but looking at the dates on this item and mention of net5, so a little surprised to find its still an issue?

[kev160967]

Apologies, it seems this is an issue with Linqpad - I copied the query into Linqpad to check it and got the error in there, but on further investigation it appears the error in the application was a different one which I've now tracked back to a mapping issue.