From 813dafdb7e568c6c98c177a8d3797ec18e15b112 Mon Sep 17 00:00:00 2001 From: hfpt Date: Sun, 22 Aug 2021 22:53:46 -0500 Subject: [PATCH] Query: SqlServer: Add translation for string.IndexOf(string, int) Resolves #25396 --- .../CosmosMethodCallTranslatorProvider.cs | 2 +- ...tor.cs => CosmosStringMethodTranslator.cs} | 4 +- .../SqlServerStringMethodTranslator.cs | 104 +++++++++++------- .../Query/NorthwindFunctionsQueryTestBase.cs | 9 ++ .../NorthwindFunctionsQuerySqlServerTest.cs | 19 +++- 5 files changed, 94 insertions(+), 44 deletions(-) rename src/EFCore.Cosmos/Query/Internal/{StringMethodTranslator.cs => CosmosStringMethodTranslator.cs} (98%) diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosMethodCallTranslatorProvider.cs b/src/EFCore.Cosmos/Query/Internal/CosmosMethodCallTranslatorProvider.cs index c86d1a0b181..d58b9b03785 100644 --- a/src/EFCore.Cosmos/Query/Internal/CosmosMethodCallTranslatorProvider.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosMethodCallTranslatorProvider.cs @@ -37,7 +37,7 @@ public CosmosMethodCallTranslatorProvider( new IMethodCallTranslator[] { new EqualsTranslator(sqlExpressionFactory), - new StringMethodTranslator(sqlExpressionFactory), + new CosmosStringMethodTranslator(sqlExpressionFactory), new ContainsTranslator(sqlExpressionFactory), new RandomTranslator(sqlExpressionFactory), new MathTranslator(sqlExpressionFactory) diff --git a/src/EFCore.Cosmos/Query/Internal/StringMethodTranslator.cs b/src/EFCore.Cosmos/Query/Internal/CosmosStringMethodTranslator.cs similarity index 98% rename from src/EFCore.Cosmos/Query/Internal/StringMethodTranslator.cs rename to src/EFCore.Cosmos/Query/Internal/CosmosStringMethodTranslator.cs index 2a55e57bbbb..6bda6daa35d 100644 --- a/src/EFCore.Cosmos/Query/Internal/StringMethodTranslator.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosStringMethodTranslator.cs @@ -15,7 +15,7 @@ namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public class StringMethodTranslator : IMethodCallTranslator + public class CosmosStringMethodTranslator : IMethodCallTranslator { private static readonly MethodInfo _indexOfMethodInfo = typeof(string).GetRequiredRuntimeMethod(nameof(string.IndexOf), typeof(string)); @@ -98,7 +98,7 @@ private static readonly MethodInfo _stringComparisonWithComparisonTypeArgumentSt /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public StringMethodTranslator(ISqlExpressionFactory sqlExpressionFactory) + public CosmosStringMethodTranslator(ISqlExpressionFactory sqlExpressionFactory) { _sqlExpressionFactory = sqlExpressionFactory; } diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerStringMethodTranslator.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerStringMethodTranslator.cs index ef3a986f2e5..a31535e9333 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerStringMethodTranslator.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerStringMethodTranslator.cs @@ -23,6 +23,12 @@ public class SqlServerStringMethodTranslator : IMethodCallTranslator private static readonly MethodInfo _indexOfMethodInfo = typeof(string).GetRequiredRuntimeMethod(nameof(string.IndexOf), typeof(string)); + private static readonly MethodInfo _indexOfMethodInfoWithStartingPosition + = typeof(string).GetRequiredRuntimeMethod(nameof(string.IndexOf), new[] { typeof(string), typeof(int) }); + + private static readonly MethodInfo _indexOfMethodInfoWithStartingPosition + = typeof(string).GetRequiredRuntimeMethod(nameof(string.IndexOf), new[] { typeof(string), typeof(int) }); + private static readonly MethodInfo _replaceMethodInfo = typeof(string).GetRequiredRuntimeMethod(nameof(string.Replace), typeof(string), typeof(string)); @@ -115,46 +121,12 @@ public SqlServerStringMethodTranslator(ISqlExpressionFactory sqlExpressionFactor { if (_indexOfMethodInfo.Equals(method)) { - var argument = arguments[0]; - var stringTypeMapping = ExpressionExtensions.InferTypeMapping(instance, argument)!; - argument = _sqlExpressionFactory.ApplyTypeMapping(argument, stringTypeMapping); - - SqlExpression charIndexExpression; - var storeType = stringTypeMapping.StoreType; - if (string.Equals(storeType, "nvarchar(max)", StringComparison.OrdinalIgnoreCase) - || string.Equals(storeType, "varchar(max)", StringComparison.OrdinalIgnoreCase)) - { - charIndexExpression = _sqlExpressionFactory.Function( - "CHARINDEX", - new[] { argument, _sqlExpressionFactory.ApplyTypeMapping(instance, stringTypeMapping) }, - nullable: true, - argumentsPropagateNullability: new[] { true, true }, - typeof(long)); - - charIndexExpression = _sqlExpressionFactory.Convert(charIndexExpression, typeof(int)); - } - else - { - charIndexExpression = _sqlExpressionFactory.Function( - "CHARINDEX", - new[] { argument, _sqlExpressionFactory.ApplyTypeMapping(instance, stringTypeMapping) }, - nullable: true, - argumentsPropagateNullability: new[] { true, true }, - method.ReturnType); - } - - charIndexExpression = _sqlExpressionFactory.Subtract(charIndexExpression, _sqlExpressionFactory.Constant(1)); + return TranslateIndexOf(instance, method, arguments[0], null); + } - return _sqlExpressionFactory.Case( - new[] - { - new CaseWhenClause( - _sqlExpressionFactory.Equal( - argument, - _sqlExpressionFactory.Constant(string.Empty, stringTypeMapping)), - _sqlExpressionFactory.Constant(0)) - }, - charIndexExpression); + if (_indexOfMethodInfoWithStartingPosition.Equals(method)) + { + return TranslateIndexOf(instance, method, arguments[0], arguments[1]); } if (_replaceMethodInfo.Equals(method)) @@ -465,6 +437,60 @@ private SqlExpression TranslateStartsEndsWith(SqlExpression instance, SqlExpress pattern); } + private SqlExpression TranslateIndexOf(SqlExpression instance, MethodInfo method, SqlExpression searchExpression, SqlExpression? startIndex) + { + var stringTypeMapping = ExpressionExtensions.InferTypeMapping(instance, searchExpression)!; + searchExpression = _sqlExpressionFactory.ApplyTypeMapping(searchExpression, stringTypeMapping); + instance = _sqlExpressionFactory.ApplyTypeMapping(instance, stringTypeMapping); + + var charIndexArguments = new List { searchExpression, instance }; + + if (startIndex is not null) + { + charIndexArguments.Add(_sqlExpressionFactory.Add(startIndex, _sqlExpressionFactory.Constant(1))); + } + + var argumentsPropagateNullability = Enumerable.Repeat(true, charIndexArguments.Count); + + SqlExpression charIndexExpression; + var storeType = stringTypeMapping.StoreType; + if (string.Equals(storeType, "nvarchar(max)", StringComparison.OrdinalIgnoreCase) + || string.Equals(storeType, "varchar(max)", StringComparison.OrdinalIgnoreCase)) + { + charIndexExpression = _sqlExpressionFactory.Function( + "CHARINDEX", + charIndexArguments, + nullable: true, + argumentsPropagateNullability, + typeof(long)); + + charIndexExpression = _sqlExpressionFactory.Convert(charIndexExpression, typeof(int)); + } + else + { + charIndexExpression = _sqlExpressionFactory.Function( + "CHARINDEX", + charIndexArguments, + nullable: true, + argumentsPropagateNullability, + method.ReturnType); + } + + charIndexExpression = _sqlExpressionFactory.Subtract(charIndexExpression, _sqlExpressionFactory.Constant(1)); + + return _sqlExpressionFactory.Case( + new[] + { + new CaseWhenClause( + _sqlExpressionFactory.Equal( + searchExpression, + _sqlExpressionFactory.Constant(string.Empty, stringTypeMapping)), + _sqlExpressionFactory.Constant(0)) + }, + charIndexExpression); + } + + // See https://docs.microsoft.com/en-us/sql/t-sql/language-elements/like-transact-sql private bool IsLikeWildChar(char c) => c == '%' || c == '_' || c == '['; diff --git a/test/EFCore.Specification.Tests/Query/NorthwindFunctionsQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NorthwindFunctionsQueryTestBase.cs index 3b52258933b..15c9d276893 100644 --- a/test/EFCore.Specification.Tests/Query/NorthwindFunctionsQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/NorthwindFunctionsQueryTestBase.cs @@ -1561,6 +1561,15 @@ public virtual Task Indexof_with_emptystring(bool async) ss => ss.Set().Where(c => c.CustomerID == "ALFKI").Select(c => c.ContactName.IndexOf(string.Empty))); } + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Indexof_with_one_arg(bool async) + { + return AssertQueryScalar( + async, + ss => ss.Set().Where(c => c.CustomerID == "ALFKI").Select(c => c.ContactName.IndexOf("a"))); + } + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task Indexof_with_starting_position(bool async) diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs index ab3fd6d779f..b47dc97c0e4 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs @@ -1528,13 +1528,28 @@ FROM [Customers] AS [c] WHERE [c].[CustomerID] = N'ALFKI'"); } - [ConditionalTheory(Skip = "issue #25396")] + public override async Task Indexof_with_one_arg(bool async) + { + await base.Indexof_with_one_arg(async); + + AssertSql( + @"SELECT CASE + WHEN N'a' = N'' THEN 0 + ELSE CAST(CHARINDEX(N'a', [c].[ContactName]) AS int) - 1 +END +FROM [Customers] AS [c] +WHERE [c].[CustomerID] = N'ALFKI'"); + } + public override async Task Indexof_with_starting_position(bool async) { await base.Indexof_with_starting_position(async); AssertSql( - @"SELECT [c].[ContactName] + @"SELECT CASE + WHEN N'a' = N'' THEN 0 + ELSE CAST(CHARINDEX(N'a', [c].[ContactName], 3 + 1) AS int) - 1 +END FROM [Customers] AS [c] WHERE [c].[CustomerID] = N'ALFKI'"); }