Various string method translation fixes

* Fix broken {Starts,Ends}With
* Support citext properly
* Contains succeeds for empty pattern

Fixes #1146
Fixes #388
Fixes #996

src/EFCore.PG/Query/ExpressionTranslators/Internal/NpgsqlStringMethodTranslator.cs

@@ -6,6 +6,7 @@
 using JetBrains.Annotations;
 using Microsoft.EntityFrameworkCore.Query;
 using Microsoft.EntityFrameworkCore.Query.SqlExpressions;
+using Microsoft.EntityFrameworkCore.Storage;
 using Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal;
 
 namespace Npgsql.EntityFrameworkCore.PostgreSQL.Query.ExpressionTranslators.Internal
@@ -23,6 +24,7 @@ public class NpgsqlStringMethodTranslator : IMethodCallTranslator
 
         readonly ISqlExpressionFactory _sqlExpressionFactory;
         readonly SqlConstantExpression _whitespace;
+        readonly RelationalTypeMapping _textTypeMapping;
 
         #region MethodInfo
 
@@ -59,6 +61,7 @@ public NpgsqlStringMethodTranslator(ISqlExpressionFactory sqlExpressionFactory,
             _whitespace = _sqlExpressionFactory.Constant(
                 @" \t\n\r",  // TODO: Complete this
                 npgsqlTypeMappingSource.EStringTypeMapping);
+            _textTypeMapping = _sqlExpressionFactory.FindMapping(typeof(string));
         }
 
         public SqlExpression Translate(SqlExpression instance, MethodInfo method, IReadOnlyList<SqlExpression> arguments)
@@ -179,7 +182,7 @@ public SqlExpression Translate(SqlExpression instance, MethodInfo method, IReadO
                 instance = _sqlExpressionFactory.ApplyTypeMapping(instance, stringTypeMapping);
                 pattern = _sqlExpressionFactory.ApplyTypeMapping(pattern, stringTypeMapping);
 
-                return _sqlExpressionFactory.GreaterThan(
+                var strposCheck = _sqlExpressionFactory.GreaterThan(
                     _sqlExpressionFactory.Function(
                         "STRPOS",
                         new[]
@@ -189,6 +192,19 @@ public SqlExpression Translate(SqlExpression instance, MethodInfo method, IReadO
                         },
                         typeof(int)),
                     _sqlExpressionFactory.Constant(0));
+
+                if (pattern is SqlConstantExpression constantPattern)
+                {
+                    return (string)constantPattern.Value == string.Empty
+                        ? (SqlExpression)_sqlExpressionFactory.Constant(true)
+                        : strposCheck;
+                }
+
+                return _sqlExpressionFactory.OrElse(
+                    _sqlExpressionFactory.Equal(
+                        pattern,
+                        _sqlExpressionFactory.Constant(string.Empty, stringTypeMapping)),
+                    strposCheck);
             }
 
             if (method == PadLeft || method == PadLeftWithChar || method == PadRight || method == PadRightWithChar)
@@ -224,50 +240,48 @@ SqlExpression TranslateStartsEndsWith(SqlExpression instance, SqlExpression patt
             {
                 // The pattern is constant. Aside from null, we escape all special characters (%, _, \)
                 // in C# and send a simple LIKE
-                if (!(constantExpression.Value is string constantString))
-                    return _sqlExpressionFactory.Like(instance, _sqlExpressionFactory.Constant(null, stringTypeMapping));
-
-                return _sqlExpressionFactory.Like(
-                    instance,
-                    _sqlExpressionFactory.Constant(
-                        startsWith
-                            ? EscapeLikePattern(constantString) + '%'
-                            : '%' + EscapeLikePattern(constantString)));
+                return constantExpression.Value is string constantPattern
+                    ? _sqlExpressionFactory.Like(
+                        instance,
+                        _sqlExpressionFactory.Constant(
+                            startsWith
+                                ? EscapeLikePattern(constantPattern) + '%'
+                                : '%' + EscapeLikePattern(constantPattern)))
+                    : _sqlExpressionFactory.Like(instance, _sqlExpressionFactory.Constant(null, stringTypeMapping));
             }
 
             // The pattern is non-constant, we use LEFT or RIGHT to extract substring and compare.
             // For StartsWith we also first run a LIKE to quickly filter out most non-matching results (sargable, but imprecise
             // because of wildchars).
-            if (startsWith)
-            {
-                return _sqlExpressionFactory.AndAlso(
+            SqlExpression leftRight = _sqlExpressionFactory.Function(
+                startsWith ? "LEFT" : "RIGHT",
+                new[]
+                {
+                    instance,
+                    _sqlExpressionFactory.Function("LENGTH", new[] { pattern }, typeof(int))
+                },
+                typeof(string),
+                stringTypeMapping);
+
+            // LEFT/RIGHT of a citext return a text, so for non-default text mappings we apply an explicit cast.
+            if (instance.TypeMapping != _textTypeMapping)
+                leftRight = _sqlExpressionFactory.Convert(leftRight, typeof(string), instance.TypeMapping);
+
+            // Also add an explicit cast on the pattern; this is only required because of
+            // The following is only needed because of https://github.com/aspnet/EntityFrameworkCore/issues/19120
+            var castPattern = pattern.TypeMapping == _textTypeMapping
+                ? pattern
+                : _sqlExpressionFactory.Convert(pattern, typeof(string), pattern.TypeMapping);
+
+            return startsWith
+                ? _sqlExpressionFactory.AndAlso(
                     _sqlExpressionFactory.Like(
                         instance,
                         _sqlExpressionFactory.Add(
-                            instance,
+                            pattern,
                             _sqlExpressionFactory.Constant("%"))),
-                    _sqlExpressionFactory.Equal(
-                        _sqlExpressionFactory.Function(
-                            "LEFT",
-                            new[] {
-                                instance,
-                                _sqlExpressionFactory.Function("LENGTH", new[] { pattern }, typeof(int))
-                            },
-                            typeof(string),
-                            stringTypeMapping),
-                        pattern));
-            }
-
-            return _sqlExpressionFactory.Equal(
-                _sqlExpressionFactory.Function(
-                    "RIGHT",
-                    new[] {
-                        instance,
-                        _sqlExpressionFactory.Function("LENGTH", new[] { pattern }, typeof(int))
-                    },
-                    typeof(string),
-                    stringTypeMapping),
-                pattern);
+                    _sqlExpressionFactory.Equal(leftRight, castPattern))
+                : _sqlExpressionFactory.Equal(leftRight, castPattern);
         }
 
         bool IsLikeWildChar(char c) => c == '%' || c == '_';

test/EFCore.PG.FunctionalTests/Query/CitextQueryTest.cs

+using System.ComponentModel.DataAnnotations.Schema;
+using System.Linq;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.EntityFrameworkCore.TestUtilities;
+using Npgsql.EntityFrameworkCore.PostgreSQL.TestUtilities;
+using Xunit;
+using Xunit.Abstractions;
+
+namespace Npgsql.EntityFrameworkCore.PostgreSQL.Query
+{
+    /// <summary>
+    /// Tests operations on the PostgreSQL citext type.
+    /// </summary>
+    public class CitextQueryTest : IClassFixture<CitextQueryTest.CitextQueryFixture>
+    {
+        CitextQueryFixture Fixture { get; }
+
+        public CitextQueryTest(CitextQueryFixture fixture, ITestOutputHelper testOutputHelper)
+        {
+            Fixture = fixture;
+            Fixture.TestSqlLoggerFactory.Clear();
+            //Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
+        }
+
+        [Fact]
+        public void StartsWith_literal()
+        {
+            using var ctx = CreateContext();
+            var result = ctx.SomeEntities.Single(s => s.CaseInsensitiveText.StartsWith("some"));
+
+            Assert.Equal(1, result.Id);
+            AssertSql(
+                @"SELECT s.""Id"", s.""CaseInsensitiveText""
+FROM ""SomeEntities"" AS s
+WHERE (s.""CaseInsensitiveText"" IS NOT NULL) AND (s.""CaseInsensitiveText"" LIKE 'some%')
+LIMIT 2");
+        }
+
+        [Fact]
+        public void StartsWith_param_pattern()
+        {
+            using var ctx = CreateContext();
+            var param = "some";
+            var result = ctx.SomeEntities.Single(s => s.CaseInsensitiveText.StartsWith(param));
+
+            Assert.Equal(1, result.Id);
+            AssertSql(
+                @"@__param_0='some'
+
+SELECT s.""Id"", s.""CaseInsensitiveText""
+FROM ""SomeEntities"" AS s
+WHERE (@__param_0 = '') OR ((s.""CaseInsensitiveText"" IS NOT NULL) AND ((s.""CaseInsensitiveText"" LIKE @__param_0 || '%') AND (LEFT(s.""CaseInsensitiveText"", LENGTH(@__param_0))::citext = @__param_0::citext)))
+LIMIT 2");
+        }
+
+        [Fact]
+        public void StartsWith_param_instance()
+        {
+            using var ctx = CreateContext();
+            var param = "SomeTextWithExtraStuff";
+            var result = ctx.SomeEntities.Single(s => param.StartsWith(s.CaseInsensitiveText));
+
+            Assert.Equal(1, result.Id);
+            AssertSql(
+                @"@__param_0='SomeTextWithExtraStuff'
+
+SELECT s.""Id"", s.""CaseInsensitiveText""
+FROM ""SomeEntities"" AS s
+WHERE (s.""CaseInsensitiveText"" = '') OR ((s.""CaseInsensitiveText"" IS NOT NULL) AND ((@__param_0 LIKE s.""CaseInsensitiveText"" || '%') AND (LEFT(@__param_0, LENGTH(s.""CaseInsensitiveText""))::citext = CAST(s.""CaseInsensitiveText"" AS citext))))
+LIMIT 2");
+        }
+
+        [Fact]
+        public void EndsWith_literal()
+        {
+            using var ctx = CreateContext();
+            var result = ctx.SomeEntities.Single(s => s.CaseInsensitiveText.EndsWith("sometext"));
+
+            Assert.Equal(1, result.Id);
+            AssertSql(
+                @"SELECT s.""Id"", s.""CaseInsensitiveText""
+FROM ""SomeEntities"" AS s
+WHERE (s.""CaseInsensitiveText"" IS NOT NULL) AND (s.""CaseInsensitiveText"" LIKE '%sometext')
+LIMIT 2");
+        }
+
+        [Fact]
+        public void EndsWith_param_pattern()
+        {
+            using var ctx = CreateContext();
+            var param = "sometext";
+            var result = ctx.SomeEntities.Single(s => s.CaseInsensitiveText.EndsWith(param));
+
+            Assert.Equal(1, result.Id);
+            AssertSql(
+                @"@__param_0='sometext'
+
+SELECT s.""Id"", s.""CaseInsensitiveText""
+FROM ""SomeEntities"" AS s
+WHERE (@__param_0 = '') OR ((s.""CaseInsensitiveText"" IS NOT NULL) AND (RIGHT(s.""CaseInsensitiveText"", LENGTH(@__param_0))::citext = @__param_0::citext))
+LIMIT 2");
+        }
+
+        [Fact]
+        public void EndsWith_param_instance()
+        {
+            using var ctx = CreateContext();
+            var param = "ExtraStuffThenSomeText";
+            var result = ctx.SomeEntities.Single(s => param.EndsWith(s.CaseInsensitiveText));
+
+            Assert.Equal(1, result.Id);
+            AssertSql(
+                @"@__param_0='ExtraStuffThenSomeText'
+
+SELECT s.""Id"", s.""CaseInsensitiveText""
+FROM ""SomeEntities"" AS s
+WHERE (s.""CaseInsensitiveText"" = '') OR ((s.""CaseInsensitiveText"" IS NOT NULL) AND (RIGHT(@__param_0, LENGTH(s.""CaseInsensitiveText""))::citext = CAST(s.""CaseInsensitiveText"" AS citext)))
+LIMIT 2");
+        }
+
+        [Fact]
+        public void Contains_literal()
+        {
+            using var ctx = CreateContext();
+            var result = ctx.SomeEntities.Single(s => s.CaseInsensitiveText.Contains("ometex"));
+
+            Assert.Equal(1, result.Id);
+            AssertSql(
+                @"SELECT s.""Id"", s.""CaseInsensitiveText""
+FROM ""SomeEntities"" AS s
+WHERE STRPOS(s.""CaseInsensitiveText"", 'ometex') > 0
+LIMIT 2");
+        }
+
+        [Fact]
+        public void Contains_param_pattern()
+        {
+            using var ctx = CreateContext();
+            var param = "ometex";
+            var result = ctx.SomeEntities.Single(s => s.CaseInsensitiveText.Contains(param));
+
+            Assert.Equal(1, result.Id);
+            AssertSql(
+                @"@__param_0='ometex'
+
+SELECT s.""Id"", s.""CaseInsensitiveText""
+FROM ""SomeEntities"" AS s
+WHERE (@__param_0 = '') OR (STRPOS(s.""CaseInsensitiveText"", @__param_0) > 0)
+LIMIT 2");
+        }
+
+        [Fact]
+        public void Contains_param_instance()
+        {
+            using var ctx = CreateContext();
+            var param = "ExtraSometextExtra";
+            var result = ctx.SomeEntities.Single(s => param.Contains(s.CaseInsensitiveText));
+
+            Assert.Equal(1, result.Id);
+            AssertSql(
+                @"@__param_0='ExtraSometextExtra'
+
+SELECT s.""Id"", s.""CaseInsensitiveText""
+FROM ""SomeEntities"" AS s
+WHERE (s.""CaseInsensitiveText"" = '') OR (STRPOS(@__param_0, s.""CaseInsensitiveText"") > 0)
+LIMIT 2");
+        }
+
+        [Fact]
+        public void IndexOf_literal()
+        {
+            using var ctx = CreateContext();
+            var result = ctx.SomeEntities.Single(s => s.CaseInsensitiveText.IndexOf("ometex") == 1);
+
+            Assert.Equal(1, result.Id);
+            AssertSql(
+                @"SELECT s.""Id"", s.""CaseInsensitiveText""
+FROM ""SomeEntities"" AS s
+WHERE (STRPOS(s.""CaseInsensitiveText"", 'ometex') - 1) = 1
+LIMIT 2");
+        }
+
+        [Fact]
+        public void IndexOf_param_pattern()
+        {
+            using var ctx = CreateContext();
+            var param = "ometex";
+            var result = ctx.SomeEntities.Single(s => s.CaseInsensitiveText.IndexOf(param) == 1);
+
+            Assert.Equal(1, result.Id);
+            AssertSql(
+                @"@__param_0='ometex'
+
+SELECT s.""Id"", s.""CaseInsensitiveText""
+FROM ""SomeEntities"" AS s
+WHERE (STRPOS(s.""CaseInsensitiveText"", @__param_0) - 1) = 1
+LIMIT 2");
+        }
+
+        [Fact]
+        public void IndexOf_param_instance()
+        {
+            using var ctx = CreateContext();
+            var param = "ExtraSometextExtra";
+            var result = ctx.SomeEntities.Single(s => param.IndexOf(s.CaseInsensitiveText) == 5);
+
+            Assert.Equal(1, result.Id);
+            AssertSql(
+                @"@__param_0='ExtraSometextExtra'
+
+SELECT s.""Id"", s.""CaseInsensitiveText""
+FROM ""SomeEntities"" AS s
+WHERE (STRPOS(@__param_0, s.""CaseInsensitiveText"") - 1) = 5
+LIMIT 2");
+        }
+
+        [Fact]
+        public void Replace_literal()
+        {
+            using var ctx = CreateContext();
+            var result = ctx.SomeEntities.Single(s => s.CaseInsensitiveText.Replace("Te", "Ne") == "SomeNext");
+
+            Assert.Equal(1, result.Id);
+            AssertSql(
+                @"SELECT s.""Id"", s.""CaseInsensitiveText""
+FROM ""SomeEntities"" AS s
+WHERE REPLACE(s.""CaseInsensitiveText"", 'Te', 'Ne') = 'SomeNext'
+LIMIT 2");
+        }
+
+        [Fact]
+        public void Replace_param_pattern()
+        {
+            using var ctx = CreateContext();
+            var param = "Te";
+            var result = ctx.SomeEntities.Single(s => s.CaseInsensitiveText.Replace(param, "Ne") == "SomeNext");
+
+            Assert.Equal(1, result.Id);
+            AssertSql(
+                @"@__param_0='Te'
+
+SELECT s.""Id"", s.""CaseInsensitiveText""
+FROM ""SomeEntities"" AS s
+WHERE REPLACE(s.""CaseInsensitiveText"", @__param_0, 'Ne') = 'SomeNext'
+LIMIT 2");
+        }
+
+        [Fact]
+        public void Replace_param_instance()
+        {
+            using var ctx = CreateContext();
+            var param = "ExtraSometextExtra";
+            var result = ctx.SomeEntities.Single(s => param.Replace(s.CaseInsensitiveText, "NewStuff") == "ExtraNewStuffExtra");
+
+            Assert.Equal(1, result.Id);
+            AssertSql(
+                @"@__param_0='ExtraSometextExtra'
+
+SELECT s.""Id"", s.""CaseInsensitiveText""
+FROM ""SomeEntities"" AS s
+WHERE REPLACE(@__param_0, s.""CaseInsensitiveText"", 'NewStuff') = 'ExtraNewStuffExtra'
+LIMIT 2");
+        }
+
+        protected CitextQueryContext CreateContext() => Fixture.CreateContext();
+
+        void AssertSql(params string[] expected)
+            => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
+
+        public class CitextQueryContext : PoolableDbContext
+        {
+            public DbSet<SomeArrayEntity> SomeEntities { get; set; }
+
+            public CitextQueryContext(DbContextOptions options) : base(options) {}
+
+            protected override void OnModelCreating(ModelBuilder modelBuilder)
+                => modelBuilder.HasPostgresExtension("citext");
+
+            public static void Seed(CitextQueryContext context)
+            {
+                context.SomeEntities.AddRange(
+                    new SomeArrayEntity { Id = 1, CaseInsensitiveText = "SomeText" },
+                    new SomeArrayEntity { Id = 2, CaseInsensitiveText = "AnotherText" });
+                context.SaveChanges();
+            }
+        }
+
+        public class SomeArrayEntity
+        {
+            public int Id { get; set; }
+            [Column(TypeName = "citext")]
+            public string CaseInsensitiveText { get; set; }
+        }
+
+        public class CitextQueryFixture : SharedStoreFixtureBase<CitextQueryContext>
+        {
+            protected override string StoreName => "CitextQueryTest";
+            protected override ITestStoreFactory TestStoreFactory => NpgsqlTestStoreFactory.Instance;
+            public TestSqlLoggerFactory TestSqlLoggerFactory => (TestSqlLoggerFactory)ListLoggerFactory;
+            protected override void Seed(CitextQueryContext context) => CitextQueryContext.Seed(context);
+        }
+    }
+}

test/EFCore.PG.FunctionalTests/Query/FunkyDataQueryNpgsqlTest.cs

-using System.Threading.Tasks;
 using Microsoft.EntityFrameworkCore;
 using Microsoft.EntityFrameworkCore.Query;
 using Microsoft.EntityFrameworkCore.TestModels.FunkyDataModel;
 using Microsoft.EntityFrameworkCore.TestUtilities;
 using Npgsql.EntityFrameworkCore.PostgreSQL.TestUtilities;
-using Xunit;
 using Xunit.Abstractions;
 
 namespace Npgsql.EntityFrameworkCore.PostgreSQL.Query
@@ -18,22 +16,6 @@ public FunkyDataQueryNpgsqlTest(FunkyDataQueryNpgsqlFixture fixture, ITestOutput
             //Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
         }
 
-        [ConditionalTheory(Skip = "https://github.com/npgsql/Npgsql.EntityFrameworkCore.PostgreSQL/issues/996")]
-        public override Task String_contains_on_argument_with_wildcard_column(bool isAsync)
-            => base.String_contains_on_argument_with_wildcard_column(isAsync);
-
-        [ConditionalTheory(Skip = "https://github.com/npgsql/Npgsql.EntityFrameworkCore.PostgreSQL/issues/996")]
-        public override Task String_contains_on_argument_with_wildcard_column_negated(bool isAsync)
-            => base.String_contains_on_argument_with_wildcard_column_negated(isAsync);
-
-        [ConditionalTheory(Skip = "https://github.com/npgsql/Npgsql.EntityFrameworkCore.PostgreSQL/issues/996")]
-        public override Task String_contains_on_argument_with_wildcard_constant(bool isAsync)
-            => base.String_contains_on_argument_with_wildcard_constant(isAsync);
-
-        [ConditionalTheory(Skip = "https://github.com/npgsql/Npgsql.EntityFrameworkCore.PostgreSQL/issues/996")]
-        public override Task String_contains_on_argument_with_wildcard_parameter(bool isAsync)
-            => base.String_contains_on_argument_with_wildcard_parameter(isAsync);
-
         public class FunkyDataQueryNpgsqlFixture : FunkyDataQueryFixtureBase
         {
             public TestSqlLoggerFactory TestSqlLoggerFactory => (TestSqlLoggerFactory)ListLoggerFactory;