Fix to #474 - Query: Improve translation of String's StartsWith, Ends…

…With and Contains

We were returning wrong results for StartsWith, EndsWith and Contains with patters containing wildcard characters (%, _)

Fix is to improve the translation as follows:

Contains:
(CHARINDEX(pattern, string) > 0) OR pattern = ''

StartsWith:
(string LIKE pattern + "%" AND CHARINDEX(pattern, string) = 1) OR pattern = ''

EndsWith:
(RIGHT(string, LEN(pattern) = pattern) OR pattern = ''

We can sometimes remove the final term (pattern = ''), i.e. when the pattern is a constant. If it's value is '', we just return true, otherwise the final term is redundant and will not be generated.
We can't really do same trick for parameters without changing the way caching works (currently it only looks at the nullability of the parameter, but in this case the value itself is important)

src/Microsoft.EntityFrameworkCore.Relational/Query/ExpressionTranslators/Internal/StartsWithTranslator.cs

@@ -30,6 +30,7 @@ public virtual Expression Translate(MethodCallExpression methodCallExpression)
 
             return ReferenceEquals(methodCallExpression.Method, _methodInfo)
                 ? new LikeExpression(
+                    // ReSharper disable once AssignNullToNotNullAttribute
                     methodCallExpression.Object,
                     Expression.Add(methodCallExpression.Arguments[0], Expression.Constant("%", typeof(string)), _concat))
                 : null;

src/Microsoft.EntityFrameworkCore.Relational/Query/ExpressionTranslators/RelationalCompositeMethodCallTranslator.cs

@@ -55,6 +55,6 @@ public virtual Expression Translate(MethodCallExpression methodCallExpression)
         /// </summary>
         /// <param name="translators"> The translators. </param>
         protected virtual void AddTranslators([NotNull] IEnumerable<IMethodCallTranslator> translators)
-            => _methodCallTranslators.AddRange(translators);
+            => _methodCallTranslators.InsertRange(0, translators);
     }
 }

src/Microsoft.EntityFrameworkCore.Relational/Query/ExpressionVisitors/Internal/PredicateNegationExpressionOptimizer.cs

@@ -3,6 +3,7 @@
 
 using System.Collections.Generic;
 using System.Linq.Expressions;
+using Microsoft.EntityFrameworkCore.Query.Expressions;
 using Remotion.Linq.Parsing;
 
 namespace Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal
@@ -98,25 +99,27 @@ protected override Expression VisitUnary(UnaryExpression node)
                     return Visit(innerUnary.Operand);
                 }
 
-                var innerBinary = node.Operand as BinaryExpression;
+                var notNullableExpression = node.Operand as NotNullableExpression;
+                var innerBinary = (notNullableExpression?.Operand ?? node.Operand) as BinaryExpression;
                 if (innerBinary != null)
                 {
+                    Expression result = null;
                     if ((innerBinary.NodeType == ExpressionType.Equal)
                         || (innerBinary.NodeType == ExpressionType.NotEqual))
                     {
                         // TODO: this is only valid for non-nullable terms, or if null semantics expansion is performed
                         // if user opts-out of the null semantics, we should not apply this rule
                         // !(a == b) -> a != b
                         // !(a != b) -> a == b
-                        return innerBinary.NodeType == ExpressionType.Equal
+                        result = innerBinary.NodeType == ExpressionType.Equal
                             ? Visit(Expression.NotEqual(innerBinary.Left, innerBinary.Right))
                             : Visit(Expression.Equal(innerBinary.Left, innerBinary.Right));
                     }
 
                     if (innerBinary.NodeType == ExpressionType.AndAlso)
                     {
                         // !(a && b) -> !a || !b
-                        return Visit(
+                        result = Visit(
                             Expression.MakeBinary(
                                 ExpressionType.OrElse,
                                 Expression.Not(innerBinary.Left),
@@ -126,7 +129,7 @@ protected override Expression VisitUnary(UnaryExpression node)
                     if (innerBinary.NodeType == ExpressionType.OrElse)
                     {
                         // !(a || b) -> !a && !b
-                        return Visit(
+                        result = Visit(
                             Expression.MakeBinary(
                                 ExpressionType.AndAlso,
                                 Expression.Not(innerBinary.Left),
@@ -136,12 +139,19 @@ protected override Expression VisitUnary(UnaryExpression node)
                     if (_nodeTypeMapping.ContainsKey(innerBinary.NodeType))
                     {
                         // e.g. !(a > b) -> a <= b
-                        return Visit(
+                        result = Visit(
                             Expression.MakeBinary(
                                 _nodeTypeMapping[innerBinary.NodeType],
                                 innerBinary.Left,
                                 innerBinary.Right));
                     }
+
+                    if (result != null)
+                    {
+                        return notNullableExpression != null
+                            ? new NotNullableExpression(result)
+                            : result;
+                    }
                 }
             }
 

src/Microsoft.EntityFrameworkCore.Relational/Query/ExpressionVisitors/Internal/RelationalNullsExpandingVisitor.cs

@@ -114,13 +114,9 @@ var rightOperand
         ///     directly from your code. This API may change or be removed in future releases.
         /// </summary>
         protected override Expression VisitExtension(Expression node)
-        {
-            var notNullableExpression = node as NotNullableExpression;
-
-            return notNullableExpression != null
+            => node is NotNullableExpression
                 ? node
                 : base.VisitExtension(node);
-        }
 
         private static Expression UnwrapConvertExpression(Expression expression, out Type conversionResultType)
         {

src/Microsoft.EntityFrameworkCore.Relational/Query/Sql/DefaultQuerySqlGenerator.cs

@@ -1632,21 +1632,27 @@ protected override Expression VisitBinary(BinaryExpression expression)
                 }
                 else
                 {
+                    Expression newLeft;
+                    Expression newRight;
                     if (expression.IsLogicalOperation())
                     {
                         var parentIsSearchCondition = _isSearchCondition;
                         _isSearchCondition = true;
-                        var left = Visit(expression.Left);
-                        var right = Visit(expression.Right);
+                        newLeft = Visit(expression.Left);
+                        newRight = Visit(expression.Right);
                         _isSearchCondition = parentIsSearchCondition;
-
-                        return Expression.MakeBinary(expression.NodeType, left, right);
+                    }
+                    else
+                    {
+                        newLeft = Visit(expression.Left);
+                        newRight = Visit(expression.Right);
                     }
 
-                    if (IsSearchCondition(expression))
+                    var newExpression = expression.Update(newLeft, expression.Conversion, newRight);
+                    if (IsSearchCondition(newExpression))
                     {
                         return Expression.Condition(
-                            expression,
+                            newExpression,
                             Expression.Constant(true, typeof(bool)),
                             Expression.Constant(false, typeof(bool)));
                     }

src/Microsoft.EntityFrameworkCore.Specification.Tests/QueryTestBase.cs

@@ -426,6 +426,13 @@ public virtual void All_top_level()
                 cs => cs.All(c => c.ContactName.StartsWith("A")));
         }
 
+        [ConditionalFact]
+        public virtual void All_top_level_column()
+        {
+            AssertQuery<Customer>(
+                cs => cs.All(c => c.ContactName.StartsWith(c.ContactName)));
+        }
+
         [ConditionalFact]
         public virtual void All_top_level_subquery()
         {

src/Microsoft.EntityFrameworkCore.SqlServer/Microsoft.EntityFrameworkCore.SqlServer.csproj

@@ -106,16 +106,19 @@
     <Compile Include="Query\Expressions\Internal\RowNumberExpression.cs" />
     <Compile Include="Query\ExpressionTranslators\Internal\SqlServerCompositeMemberTranslator.cs" />
     <Compile Include="Query\ExpressionTranslators\Internal\SqlServerCompositeMethodCallTranslator.cs" />
+    <Compile Include="Query\ExpressionTranslators\Internal\SqlServerContainsCharIndexTranslator.cs" />
     <Compile Include="Query\ExpressionTranslators\Internal\SqlServerConvertTranslator.cs" />
     <Compile Include="Query\ExpressionTranslators\Internal\SqlServerDateTimeDateComponentTranslator.cs" />
     <Compile Include="Query\ExpressionTranslators\Internal\SqlServerDateTimeNowTranslator.cs" />
+    <Compile Include="Query\ExpressionTranslators\Internal\SqlServerEndsWithCharIndexTranslator.cs" />
     <Compile Include="Query\ExpressionTranslators\Internal\SqlServerMathAbsTranslator.cs" />
     <Compile Include="Query\ExpressionTranslators\Internal\SqlServerMathCeilingTranslator.cs" />
     <Compile Include="Query\ExpressionTranslators\Internal\SqlServerMathFloorTranslator.cs" />
     <Compile Include="Query\ExpressionTranslators\Internal\SqlServerMathPowerTranslator.cs" />
     <Compile Include="Query\ExpressionTranslators\Internal\SqlServerMathRoundTranslator.cs" />
     <Compile Include="Query\ExpressionTranslators\Internal\SqlServerMathTruncateTranslator.cs" />
     <Compile Include="Query\ExpressionTranslators\Internal\SqlServerNewGuidTranslator.cs" />
+    <Compile Include="Query\ExpressionTranslators\Internal\SqlServerStartsWithCharIndexTranslator.cs" />
     <Compile Include="Query\ExpressionTranslators\Internal\SqlServerStringLengthTranslator.cs" />
     <Compile Include="Query\ExpressionTranslators\Internal\SqlServerStringReplaceTranslator.cs" />
     <Compile Include="Query\ExpressionTranslators\Internal\SqlServerStringTrimEndTranslator.cs" />
@@ -192,4 +195,4 @@
   <Target Name="AfterBuild">
   </Target>
   -->
-</Project>
+</Project>

src/Microsoft.EntityFrameworkCore.SqlServer/Query/ExpressionTranslators/Internal/SqlServerCompositeMethodCallTranslator.cs

@@ -29,7 +29,10 @@ public class SqlServerCompositeMethodCallTranslator : RelationalCompositeMethodC
             new SqlServerStringTrimEndTranslator(),
             new SqlServerStringTrimStartTranslator(),
             new SqlServerStringTrimTranslator(),
-            new SqlServerConvertTranslator()
+            new SqlServerConvertTranslator(),
+            new SqlServerContainsCharIndexTranslator(),
+            new SqlServerEndsWithCharIndexTranslator(),
+            new SqlServerStartsWithCharIndexTranslator(),
         };
 
         // ReSharper disable once SuggestBaseTypeForParameter

src/Microsoft.EntityFrameworkCore.SqlServer/Query/ExpressionTranslators/Internal/SqlServerContainsCharIndexTranslator.cs

@@ -0,0 +1,43 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Linq.Expressions;
+using System.Reflection;
+using Microsoft.EntityFrameworkCore.Query.Expressions;
+
+namespace Microsoft.EntityFrameworkCore.Query.ExpressionTranslators.Internal
+{
+    /// <summary>
+    ///     This API supports the Entity Framework Core infrastructure and is not intended to be used
+    ///     directly from your code. This API may change or be removed in future releases.
+    /// </summary>
+    public class SqlServerContainsCharIndexTranslator : IMethodCallTranslator
+    {
+        private static readonly MethodInfo _methodInfo
+            = typeof(string).GetRuntimeMethod(nameof(string.Contains), new[] { typeof(string) });
+
+        public virtual Expression Translate(MethodCallExpression methodCallExpression)
+        {
+            if (ReferenceEquals(methodCallExpression.Method, _methodInfo))
+            {
+                var patternExpression = methodCallExpression.Arguments[0];
+                var patternConstantExpression = patternExpression as ConstantExpression;
+
+                var charIndexExpression = Expression.GreaterThan(
+                    new SqlFunctionExpression("CHARINDEX", typeof(int), new[] { patternExpression, methodCallExpression.Object }),
+                    Expression.Constant(0));
+
+                return
+                    patternConstantExpression != null
+                    ? (string)patternConstantExpression.Value == string.Empty
+                        ? (Expression)Expression.Constant(true)
+                        : charIndexExpression
+                    : Expression.OrElse(
+                        charIndexExpression,
+                        Expression.Equal(patternExpression, Expression.Constant(string.Empty)));
+            }
+
+            return null;
+        }
+    }
+}

src/Microsoft.EntityFrameworkCore.SqlServer/Query/ExpressionTranslators/Internal/SqlServerEndsWithCharIndexTranslator.cs

@@ -0,0 +1,51 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Linq.Expressions;
+using System.Reflection;
+using Microsoft.EntityFrameworkCore.Query.Expressions;
+
+namespace Microsoft.EntityFrameworkCore.Query.ExpressionTranslators.Internal
+{
+    /// <summary>
+    ///     This API supports the Entity Framework Core infrastructure and is not intended to be used
+    ///     directly from your code. This API may change or be removed in future releases.
+    /// </summary>
+    public class SqlServerEndsWithCharIndexTranslator : IMethodCallTranslator
+    {
+        private static readonly MethodInfo _methodInfo
+            = typeof(string).GetRuntimeMethod(nameof(string.EndsWith), new[] { typeof(string) });
+
+        public virtual Expression Translate(MethodCallExpression methodCallExpression)
+        {
+            if (ReferenceEquals(methodCallExpression.Method, _methodInfo))
+            {
+                var patternExpression = methodCallExpression.Arguments[0];
+                var patternConstantExpression = patternExpression as ConstantExpression;
+
+                var endsWithExpression = Expression.Equal(
+                    new SqlFunctionExpression(
+                        "RIGHT",
+                        // ReSharper disable once PossibleNullReferenceException
+                        methodCallExpression.Object.Type,
+                        new[]
+                        {
+                            methodCallExpression.Object,
+                            new SqlFunctionExpression("LEN", typeof(int), new[] { patternExpression })
+                        }),
+                    patternExpression);
+
+                return new NotNullableExpression(
+                        patternConstantExpression != null
+                        ? (string)patternConstantExpression.Value == string.Empty
+                            ? (Expression)Expression.Constant(true)
+                            : endsWithExpression
+                        : Expression.OrElse(
+                            endsWithExpression,
+                            Expression.Equal(patternExpression, Expression.Constant(string.Empty))));
+            }
+
+            return null;
+        }
+    }
+}

src/Microsoft.EntityFrameworkCore.SqlServer/Query/ExpressionTranslators/Internal/SqlServerStartsWithCharIndexTranslator.cs

@@ -0,0 +1,50 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Linq.Expressions;
+using System.Reflection;
+using Microsoft.EntityFrameworkCore.Query.Expressions;
+
+namespace Microsoft.EntityFrameworkCore.Query.ExpressionTranslators.Internal
+{
+    /// <summary>
+    ///     This API supports the Entity Framework Core infrastructure and is not intended to be used
+    ///     directly from your code. This API may change or be removed in future releases.
+    /// </summary>
+    public class SqlServerStartsWithCharIndexTranslator : IMethodCallTranslator
+    {
+        private static readonly MethodInfo _methodInfo
+            = typeof(string).GetRuntimeMethod(nameof(string.StartsWith), new[] { typeof(string) });
+
+        private static readonly MethodInfo _concat
+            = typeof(string).GetRuntimeMethod(nameof(string.Concat), new[] { typeof(string), typeof(string) });
+
+        public virtual Expression Translate(MethodCallExpression methodCallExpression)
+        {
+            if (ReferenceEquals(methodCallExpression.Method, _methodInfo))
+            {
+                var patternExpression = methodCallExpression.Arguments[0];
+                var patternConstantExpression = patternExpression as ConstantExpression;
+
+                var startsWithExpression = Expression.AndAlso(
+                    new LikeExpression(
+                        // ReSharper disable once AssignNullToNotNullAttribute
+                        methodCallExpression.Object,
+                        Expression.Add(methodCallExpression.Arguments[0], Expression.Constant("%", typeof(string)), _concat)),
+                    Expression.Equal(
+                        new SqlFunctionExpression("CHARINDEX", typeof(int), new[] { patternExpression, methodCallExpression.Object }),
+                        Expression.Constant(1)));
+
+                return patternConstantExpression != null
+                    ? (string)patternConstantExpression.Value == string.Empty
+                        ? (Expression)Expression.Constant(true)
+                        : startsWithExpression
+                    : Expression.OrElse(
+                        startsWithExpression,
+                        Expression.Equal(patternExpression, Expression.Constant(string.Empty)));
+            }
+
+            return null;
+        }
+    }
+}

test/Microsoft.EntityFrameworkCore.SqlServer.FunctionalTests/ComplexNavigationsQuerySqlServerTest.cs

@@ -288,7 +288,8 @@ public override void Navigation_inside_method_call_translated_to_join()
                 @"SELECT [e1].[Id], [e1].[Name], [e1].[OneToMany_Optional_Self_InverseId], [e1].[OneToMany_Required_Self_InverseId], [e1].[OneToOne_Optional_SelfId]
 FROM [Level1] AS [e1]
 INNER JOIN [Level2] AS [e1.OneToOne_Required_FK] ON [e1].[Id] = [e1.OneToOne_Required_FK].[Level1_Required_Id]
-WHERE [e1.OneToOne_Required_FK].[Name] LIKE N'L' + N'%'", Sql);
+WHERE [e1.OneToOne_Required_FK].[Name] LIKE N'L' + N'%' AND (CHARINDEX(N'L', [e1.OneToOne_Required_FK].[Name]) = 1)", 
+                Sql);
         }
 
         public override void Join_navigation_in_outer_selector_translated_to_extra_join()

test/Microsoft.EntityFrameworkCore.SqlServer.FunctionalTests/FromSqlQuerySqlServerTest.cs

@@ -48,7 +48,7 @@ public override void From_sql_queryable_composed()
 FROM (
     SELECT * FROM ""Customers""
 ) AS [c]
-WHERE [c].[ContactName] LIKE (N'%' + N'z') + N'%'",
+WHERE CHARINDEX(N'z', [c].[ContactName]) > 0",
                 Sql);
         }
 

test/Microsoft.EntityFrameworkCore.SqlServer.FunctionalTests/GearsOfWarQuerySqlServerTest.cs

@@ -882,7 +882,7 @@ public override void Null_propagation_optimization2()
             Assert.Equal(
                 @"SELECT [g].[Nickname], [g].[SquadId], [g].[AssignedCityName], [g].[CityOrBirthName], [g].[Discriminator], [g].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[Rank]
 FROM [Gear] AS [g]
-WHERE [g].[Discriminator] IN (N'Officer', N'Gear') AND [g].[LeaderNickname] LIKE N'%' + N'us'",
+WHERE [g].[Discriminator] IN (N'Officer', N'Gear') AND RIGHT([g].[LeaderNickname], LEN(N'us')) = N'us'",
                 Sql);
         }
 
@@ -894,7 +894,7 @@ public override void Null_propagation_optimization3()
             Assert.Equal(
                 @"SELECT [g].[Nickname], [g].[SquadId], [g].[AssignedCityName], [g].[CityOrBirthName], [g].[Discriminator], [g].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[Rank]
 FROM [Gear] AS [g]
-WHERE [g].[Discriminator] IN (N'Officer', N'Gear') AND [g].[LeaderNickname] LIKE N'%' + N'us'",
+WHERE [g].[Discriminator] IN (N'Officer', N'Gear') AND RIGHT([g].[LeaderNickname], LEN(N'us')) = N'us'",
                 Sql);
         }
 
@@ -1248,7 +1248,7 @@ public override void Non_unicode_string_literals_is_used_for_non_unicode_column_
             Assert.Equal(
                 @"SELECT [c].[Name], [c].[Location]
 FROM [City] AS [c]
-WHERE [c].[Location] LIKE ('%' + 'Jacinto') + '%'",
+WHERE CHARINDEX(N'Jacinto', [c].[Location]) > 0",
                 Sql);
         }