Improve SQL Server translation of string.IndexOf with constants

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[]
  {

test/EFCore.Cosmos.FunctionalTests/Query/NorthwindFunctionsQueryCosmosTest.cs

@@ -812,26 +812,50 @@ FROM root c
 WHERE ((c[""Discriminator""] = ""Customer"") AND (INDEX_OF(c[""ContactName""], """") = 0))");
  }
 
- public override async Task Indexof_with_one_arg(bool async)
+ public override async Task Indexof_with_one_constant_arg(bool async)
  {
- await base.Indexof_with_one_arg(async);
+ await base.Indexof_with_one_constant_arg(async);
 
  AssertSql(
  @"SELECT c
 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_one_parameter_arg(bool async)
  {
- await base.Indexof_with_starting_position(async);
+ await base.Indexof_with_one_parameter_arg(async);
+
+ AssertSql(
+ @"@__pattern_0='a'
+
+SELECT c
+FROM root c
+WHERE ((c[""Discriminator""] = ""Customer"") AND (INDEX_OF(c[""ContactName""], @__pattern_0) = 1))");
+ }
+
+ public override async Task Indexof_with_constant_starting_position(bool 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);

test/EFCore.Specification.Tests/Query/NorthwindFunctionsQueryTestBase.cs

@@ -1413,20 +1413,44 @@ public virtual Task Indexof_with_emptystring(bool async)
 
  [ConditionalTheory]
  [MemberData(nameof(IsAsyncData))]
- public virtual Task Indexof_with_one_arg(bool async)
+ public virtual Task Indexof_with_one_constant_arg(bool async)
  => AssertQuery(
  async,
  ss => ss.Set<Customer>().Where(c => c.ContactName.IndexOf("a") == 1),
  entryCount: 32);
 
  [ConditionalTheory]
  [MemberData(nameof(IsAsyncData))]
- public virtual Task Indexof_with_starting_position(bool async)
+ public virtual Task Indexof_with_one_parameter_arg(bool async)
+ {
+ var pattern = "a";
+
+ return AssertQuery(
+ async,
+ ss => ss.Set<Customer>().Where(c => c.ContactName.IndexOf(pattern) == 1),
+ entryCount: 32);
+ }
+
+ [ConditionalTheory]
+ [MemberData(nameof(IsAsyncData))]
+ 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)

test/EFCore.SqlServer.FunctionalTests/BuiltInDataTypesSqlServerTest.cs

@@ -83,10 +83,7 @@ public void String_indexOf_over_varchar_max()
 
  Assert.Equal(-1, Assert.Single(results));
  AssertSql(
- @"SELECT CASE
- WHEN 'a' = '' THEN 0
- ELSE CAST(CHARINDEX('a', [m].[StringAsVarcharMax]) AS int) - 1
-END
+ @"SELECT CAST(CHARINDEX('a', [m].[StringAsVarcharMax]) AS int) - 1
 FROM [MappedNullableDataTypes] AS [m]
 WHERE [m].[Int] = 81");
  }

test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs

@@ -1575,30 +1575,51 @@ public override async Task Indexof_with_emptystring(bool async)
 FROM [Customers] AS [c]");
  }
 
- public override async Task Indexof_with_one_arg(bool async)
+ public override async Task Indexof_with_one_constant_arg(bool async)
  {
- await base.Indexof_with_one_arg(async);
+ await base.Indexof_with_one_constant_arg(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 (CAST(CHARINDEX(N'a', [c].[ContactName]) AS int) - 1) = 1");
+ }
+
+ public override async Task Indexof_with_one_parameter_arg(bool async)
+ {
+ await base.Indexof_with_one_parameter_arg(async);
+
+ AssertSql(
+ @"@__pattern_0='a' (Size = 4000)
+
+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
+ WHEN @__pattern_0 = N'' THEN 0
+ ELSE CAST(CHARINDEX(@__pattern_0, [c].[ContactName]) AS int) - 1
 END = 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)
@@ -1700,10 +1721,7 @@ public override async Task Substring_with_two_args_with_Index_of(bool async)
  await base.Substring_with_two_args_with_Index_of(async);
 
  AssertSql(
- @"SELECT SUBSTRING([c].[ContactName], CASE
- WHEN N'a' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'a', [c].[ContactName]) AS int) - 1
-END + 1, 3)
+ @"SELECT SUBSTRING([c].[ContactName], (CAST(CHARINDEX(N'a', [c].[ContactName]) AS int) - 1) + 1, 3)
 FROM [Customers] AS [c]
 WHERE [c].[CustomerID] = N'ALFKI'");
  }

test/EFCore.SqlServer.FunctionalTests/Query/NorthwindWhereQuerySqlServerTest.cs

@@ -674,13 +674,7 @@ public override async Task Where_string_indexof(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'Sea' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'Sea', [c].[City]) AS int) - 1
-END <> -1 OR CASE
- WHEN N'Sea' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'Sea', [c].[City]) AS int) - 1
-END IS NULL");
+WHERE (CAST(CHARINDEX(N'Sea', [c].[City]) AS int) - 1) <> -1 OR [c].[City] IS NULL");
  }
 
  public override async Task Where_string_replace(bool async)

test/EFCore.SqlServer.FunctionalTests/Query/NullSemanticsQuerySqlServerTest.cs

@@ -1248,36 +1248,15 @@ public override async Task Null_semantics_applied_when_comparing_function_with_n
  AssertSql(
  @"SELECT [e].[Id]
 FROM [Entities1] AS [e]
-WHERE CASE
- WHEN N'oo' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'oo', [e].[NullableStringA]) AS int) - 1
-END = [e].[NullableIntA] OR (CASE
- WHEN N'oo' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'oo', [e].[NullableStringA]) AS int) - 1
-END IS NULL AND [e].[NullableIntA] IS NULL)",
+WHERE (CAST(CHARINDEX(N'oo', [e].[NullableStringA]) AS int) - 1) = [e].[NullableIntA] OR ([e].[NullableStringA] IS NULL AND [e].[NullableIntA] IS NULL)",
  //
  @"SELECT [e].[Id]
 FROM [Entities1] AS [e]
-WHERE CASE
- WHEN N'ar' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'ar', [e].[NullableStringA]) AS int) - 1
-END = [e].[NullableIntA] OR (CASE
- WHEN N'ar' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'ar', [e].[NullableStringA]) AS int) - 1
-END IS NULL AND [e].[NullableIntA] IS NULL)",
+WHERE (CAST(CHARINDEX(N'ar', [e].[NullableStringA]) AS int) - 1) = [e].[NullableIntA] OR ([e].[NullableStringA] IS NULL AND [e].[NullableIntA] IS NULL)",
  //
  @"SELECT [e].[Id]
 FROM [Entities1] AS [e]
-WHERE (CASE
- WHEN N'oo' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'oo', [e].[NullableStringA]) AS int) - 1
-END <> [e].[NullableIntB] OR CASE
- WHEN N'oo' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'oo', [e].[NullableStringA]) AS int) - 1
-END IS NULL OR [e].[NullableIntB] IS NULL) AND (CASE
- WHEN N'oo' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'oo', [e].[NullableStringA]) AS int) - 1
-END IS NOT NULL OR [e].[NullableIntB] IS NOT NULL)");
+WHERE ((CAST(CHARINDEX(N'oo', [e].[NullableStringA]) AS int) - 1) <> [e].[NullableIntB] OR [e].[NullableStringA] IS NULL OR [e].[NullableIntB] IS NULL) AND ([e].[NullableStringA] IS NOT NULL OR [e].[NullableIntB] IS NOT NULL)");
  }
 
  public override async Task Where_IndexOf_empty(bool async)
@@ -1293,10 +1272,7 @@ public override async Task Select_IndexOf(bool async)
  await base.Select_IndexOf(async);
 
  AssertSql(
- @"SELECT CASE
- WHEN N'oo' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'oo', [e].[NullableStringA]) AS int) - 1
-END
+ @"SELECT CAST(CHARINDEX(N'oo', [e].[NullableStringA]) AS int) - 1
 FROM [Entities1] AS [e]
 ORDER BY [e].[Id]");
  }
@@ -1308,63 +1284,15 @@ public override async Task Null_semantics_applied_when_comparing_two_functions_w
  AssertSql(
  @"SELECT [e].[Id]
 FROM [Entities1] AS [e]
-WHERE CASE
- WHEN N'oo' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'oo', [e].[NullableStringA]) AS int) - 1
-END = CASE
- WHEN N'ar' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'ar', [e].[NullableStringB]) AS int) - 1
-END OR (CASE
- WHEN N'oo' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'oo', [e].[NullableStringA]) AS int) - 1
-END IS NULL AND CASE
- WHEN N'ar' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'ar', [e].[NullableStringB]) AS int) - 1
-END IS NULL)",
+WHERE (CAST(CHARINDEX(N'oo', [e].[NullableStringA]) AS int) - 1) = (CAST(CHARINDEX(N'ar', [e].[NullableStringB]) AS int) - 1) OR ([e].[NullableStringA] IS NULL AND [e].[NullableStringB] IS NULL)",
  //
  @"SELECT [e].[Id]
 FROM [Entities1] AS [e]
-WHERE (CASE
- WHEN N'oo' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'oo', [e].[NullableStringA]) AS int) - 1
-END <> CASE
- WHEN N'ar' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'ar', [e].[NullableStringB]) AS int) - 1
-END OR CASE
- WHEN N'oo' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'oo', [e].[NullableStringA]) AS int) - 1
-END IS NULL OR CASE
- WHEN N'ar' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'ar', [e].[NullableStringB]) AS int) - 1
-END IS NULL) AND (CASE
- WHEN N'oo' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'oo', [e].[NullableStringA]) AS int) - 1
-END IS NOT NULL OR CASE
- WHEN N'ar' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'ar', [e].[NullableStringB]) AS int) - 1
-END IS NOT NULL)",
- //
- @"SELECT [e].[Id]
-FROM [Entities1] AS [e]
-WHERE (CASE
- WHEN N'oo' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'oo', [e].[NullableStringA]) AS int) - 1
-END <> CASE
- WHEN N'ar' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'ar', [e].[NullableStringA]) AS int) - 1
-END OR CASE
- WHEN N'oo' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'oo', [e].[NullableStringA]) AS int) - 1
-END IS NULL OR CASE
- WHEN N'ar' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'ar', [e].[NullableStringA]) AS int) - 1
-END IS NULL) AND (CASE
- WHEN N'oo' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'oo', [e].[NullableStringA]) AS int) - 1
-END IS NOT NULL OR CASE
- WHEN N'ar' = N'' THEN 0
- ELSE CAST(CHARINDEX(N'ar', [e].[NullableStringA]) AS int) - 1
-END IS NOT NULL)");
+WHERE ((CAST(CHARINDEX(N'oo', [e].[NullableStringA]) AS int) - 1) <> (CAST(CHARINDEX(N'ar', [e].[NullableStringB]) AS int) - 1) OR [e].[NullableStringA] IS NULL OR [e].[NullableStringB] IS NULL) AND ([e].[NullableStringA] IS NOT NULL OR [e].[NullableStringB] IS NOT NULL)",
+ //
+ @"SELECT [e].[Id]
+FROM [Entities1] AS [e]
+WHERE ((CAST(CHARINDEX(N'oo', [e].[NullableStringA]) AS int) - 1) <> (CAST(CHARINDEX(N'ar', [e].[NullableStringA]) AS int) - 1) OR [e].[NullableStringA] IS NULL) AND [e].[NullableStringA] IS NOT NULL");
  }
 
  public override async Task Null_semantics_applied_when_comparing_two_functions_with_multiple_nullable_arguments(bool async)

test/EFCore.Sqlite.FunctionalTests/Query/NorthwindFunctionsQuerySqliteTest.cs

@@ -332,18 +332,33 @@ public override async Task Indexof_with_emptystring(bool async)
 WHERE (instr(""c"".""ContactName"", '') - 1) = 0");
  }
 
- public override async Task Indexof_with_one_arg(bool async)
+ public override async Task Indexof_with_one_constant_arg(bool async)
  {
- await base.Indexof_with_one_arg(async);
+ await base.Indexof_with_one_constant_arg(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 (instr(""c"".""ContactName"", 'a') - 1) = 1");
  }
 
- public override Task Indexof_with_starting_position(bool async)
- => AssertTranslationFailed(() => base.Indexof_with_starting_position(async));
+ public override async Task Indexof_with_one_parameter_arg(bool async)
+ {
+ await base.Indexof_with_one_parameter_arg(async);
+
+ AssertSql(
+ @"@__pattern_0='a' (Size = 1)
+
+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 (instr(""c"".""ContactName"", @__pattern_0) - 1) = 1");
+ }
+
+ 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)
  {