Improve SQL Server translation of string.IndexOf with constants

dotnet · May 16, 2022 · 9de225e · 9de225e
1 parent 57f7b5c
commit 9de225e
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 16 deletions.
diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerStringMethodTranslator.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerStringMethodTranslator.cs
@@ -443,7 +443,10 @@ private SqlExpression TranslateIndexOf(
 
         if (startIndex is not null)
         {
-            charIndexArguments.Add(_sqlExpressionFactory.Add(startIndex, _sqlExpressionFactory.Constant(1)));
+            charIndexArguments.Add(
+                startIndex is SqlConstantExpression { Value : int constantStartIndex }
+                    ? _sqlExpressionFactory.Constant(constantStartIndex + 1, typeof(int))
+                    : _sqlExpressionFactory.Add(startIndex, _sqlExpressionFactory.Constant(1)));
         }
 
         var argumentsPropagateNullability = Enumerable.Repeat(true, charIndexArguments.Count);
@@ -474,6 +477,15 @@ private SqlExpression TranslateIndexOf(
 
         charIndexExpression = _sqlExpressionFactory.Subtract(charIndexExpression, _sqlExpressionFactory.Constant(1));
 
+        // If the pattern is an empty string, we need to special case to always return 0 (since CHARINDEX return 0, which we'd subtract to
+        // -1). Handle separately for constant and non-constant patterns.
+        if (searchExpression is SqlConstantExpression { Value : string constantSearchPattern })
+        {
+            return constantSearchPattern == string.Empty
+                ? _sqlExpressionFactory.Constant(0, typeof(int))
+                : charIndexExpression;
+        }
+
         return _sqlExpressionFactory.Case(
             new[]
             {

diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindFunctionsQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindFunctionsQueryCosmosTest.cs
@@ -822,16 +822,28 @@ FROM root c
 WHERE ((c[""Discriminator""] = ""Customer"") AND (INDEX_OF(c[""ContactName""], ""a"") = 1))");
     }
 
-    public override async Task Indexof_with_starting_position(bool async)
+    public override async Task Indexof_with_constant_starting_position(bool async)
     {
-        await base.Indexof_with_starting_position(async);
+        await base.Indexof_with_constant_starting_position(async);
 
         AssertSql(
             @"SELECT c
 FROM root c
 WHERE ((c[""Discriminator""] = ""Customer"") AND (INDEX_OF(c[""ContactName""], ""a"", 2) = 4))");
     }
 
+    public override async Task Indexof_with_parameter_starting_position(bool async)
+    {
+        await base.Indexof_with_parameter_starting_position(async);
+
+        AssertSql(
+            @"@__start_0='2'
+
+SELECT c
+FROM root c
+WHERE ((c[""Discriminator""] = ""Customer"") AND (INDEX_OF(c[""ContactName""], ""a"", @__start_0) = 4))");
+    }
+
     public override async Task Replace_with_emptystring(bool async)
     {
         await base.Replace_with_emptystring(async);

diff --git a/test/EFCore.Specification.Tests/Query/NorthwindFunctionsQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NorthwindFunctionsQueryTestBase.cs
@@ -1421,12 +1421,24 @@ public virtual Task Indexof_with_one_arg(bool async)
 
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
-    public virtual Task Indexof_with_starting_position(bool async)
+    public virtual Task Indexof_with_constant_starting_position(bool async)
         => AssertQuery(
             async,
             ss => ss.Set<Customer>().Where(c => c.ContactName.IndexOf("a", 2) == 4),
             entryCount: 15);
 
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Indexof_with_parameter_starting_position(bool async)
+    {
+        var start = 2;
+
+        return AssertQuery(
+            async,
+            ss => ss.Set<Customer>().Where(c => c.ContactName.IndexOf("a", start) == 4),
+            entryCount: 15);
+    }
+
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
     public virtual Task Replace_with_emptystring(bool async)

diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs
@@ -1582,23 +1582,29 @@ public override async Task Indexof_with_one_arg(bool async)
         AssertSql(
             @"SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region]
 FROM [Customers] AS [c]
-WHERE CASE
-    WHEN N'a' = N'' THEN 0
-    ELSE CAST(CHARINDEX(N'a', [c].[ContactName]) AS int) - 1
-END = 1");
+WHERE (CAST(CHARINDEX(N'a', [c].[ContactName]) AS int) - 1) = 1");
     }
 
-    public override async Task Indexof_with_starting_position(bool async)
+    public override async Task Indexof_with_constant_starting_position(bool async)
     {
-        await base.Indexof_with_starting_position(async);
+        await base.Indexof_with_constant_starting_position(async);
 
         AssertSql(
             @"SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region]
 FROM [Customers] AS [c]
-WHERE CASE
-    WHEN N'a' = N'' THEN 0
-    ELSE CAST(CHARINDEX(N'a', [c].[ContactName], 2 + 1) AS int) - 1
-END = 4");
+WHERE (CAST(CHARINDEX(N'a', [c].[ContactName], 3) AS int) - 1) = 4");
+    }
+
+    public override async Task Indexof_with_parameter_starting_position(bool async)
+    {
+        await base.Indexof_with_parameter_starting_position(async);
+
+        AssertSql(
+            @"@__start_0='2'
+
+SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region]
+FROM [Customers] AS [c]
+WHERE (CAST(CHARINDEX(N'a', [c].[ContactName], @__start_0 + 1) AS int) - 1) = 4");
     }
 
     public override async Task Replace_with_emptystring(bool async)

diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindFunctionsQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindFunctionsQuerySqliteTest.cs
@@ -342,8 +342,11 @@ public override async Task Indexof_with_one_arg(bool async)
 WHERE (instr(""c"".""ContactName"", 'a') - 1) = 1");
     }
 
-    public override Task Indexof_with_starting_position(bool async)
-        => AssertTranslationFailed(() => base.Indexof_with_starting_position(async));
+    public override Task Indexof_with_constant_starting_position(bool async)
+        => AssertTranslationFailed(() => base.Indexof_with_constant_starting_position(async));
+
+    public override Task Indexof_with_parameter_starting_position(bool async)
+        => AssertTranslationFailed(() => base.Indexof_with_parameter_starting_position(async));
 
     public override async Task Replace_with_emptystring(bool async)
     {