From 4448f3588413f74ebc729795d3910f85ca157c5f Mon Sep 17 00:00:00 2001 From: acarrazana Date: Tue, 14 May 2024 09:33:37 -0400 Subject: [PATCH 1/7] TrimStart and TrimEnd with option char array implementation for SQL Server. Fixes #22924 --- .../SqlServerMethodCallTranslatorProvider.cs | 6 +- .../SqlServerStringMethodTranslator.cs | 70 ++++++++++++++++++- .../NorthwindFunctionsQuerySqlServerTest.cs | 40 +++++++---- 3 files changed, 99 insertions(+), 17 deletions(-) diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerMethodCallTranslatorProvider.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerMethodCallTranslatorProvider.cs index f717eff25d8..ac7c9711894 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerMethodCallTranslatorProvider.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerMethodCallTranslatorProvider.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.EntityFrameworkCore.SqlServer.Infrastructure.Internal; + namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal; /// @@ -17,7 +19,7 @@ public class SqlServerMethodCallTranslatorProvider : RelationalMethodCallTransla /// 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 SqlServerMethodCallTranslatorProvider(RelationalMethodCallTranslatorProviderDependencies dependencies) + public SqlServerMethodCallTranslatorProvider(RelationalMethodCallTranslatorProviderDependencies dependencies, ISqlServerSingletonOptions sqlServerSingletonOptions) : base(dependencies) { var sqlExpressionFactory = dependencies.SqlExpressionFactory; @@ -38,7 +40,7 @@ public SqlServerMethodCallTranslatorProvider(RelationalMethodCallTranslatorProvi new SqlServerMathTranslator(sqlExpressionFactory), new SqlServerNewGuidTranslator(sqlExpressionFactory), new SqlServerObjectToStringTranslator(sqlExpressionFactory, typeMappingSource), - new SqlServerStringMethodTranslator(sqlExpressionFactory), + new SqlServerStringMethodTranslator(sqlExpressionFactory, sqlServerSingletonOptions), new SqlServerTimeOnlyMethodTranslator(sqlExpressionFactory) }); } diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerStringMethodTranslator.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerStringMethodTranslator.cs index 24cf719c2f2..cb5fd2bfd18 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerStringMethodTranslator.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerStringMethodTranslator.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using Microsoft.EntityFrameworkCore.Query.SqlExpressions; +using Microsoft.EntityFrameworkCore.SqlServer.Infrastructure.Internal; using ExpressionExtensions = Microsoft.EntityFrameworkCore.Query.ExpressionExtensions; namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal; @@ -61,6 +62,12 @@ private static readonly MethodInfo TrimEndMethodInfoWithCharArrayArg private static readonly MethodInfo TrimMethodInfoWithCharArrayArg = typeof(string).GetRuntimeMethod(nameof(string.Trim), [typeof(char[])])!; + private static readonly MethodInfo TrimStartMethodInfoWithCharArg + = typeof(string).GetRuntimeMethod(nameof(string.TrimStart), [typeof(char)])!; + + private static readonly MethodInfo TrimEndMethodInfoWithCharArg + = typeof(string).GetRuntimeMethod(nameof(string.TrimEnd), [typeof(char)])!; + private static readonly MethodInfo FirstOrDefaultMethodInfoWithoutArgs = typeof(Enumerable).GetRuntimeMethods().Single( m => m.Name == nameof(Enumerable.FirstOrDefault) @@ -73,15 +80,19 @@ private static readonly MethodInfo LastOrDefaultMethodInfoWithoutArgs private readonly ISqlExpressionFactory _sqlExpressionFactory; + private readonly ISqlServerSingletonOptions _sqlServerSingletonOptions; + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in /// 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 SqlServerStringMethodTranslator(ISqlExpressionFactory sqlExpressionFactory) + public SqlServerStringMethodTranslator(ISqlExpressionFactory sqlExpressionFactory, ISqlServerSingletonOptions sqlServerSingletonOptions) { _sqlExpressionFactory = sqlExpressionFactory; + + _sqlServerSingletonOptions = sqlServerSingletonOptions; } /// @@ -182,7 +193,6 @@ public SqlServerStringMethodTranslator(ISqlExpressionFactory sqlExpressionFactor if (TrimStartMethodInfoWithoutArgs.Equals(method) || (TrimStartMethodInfoWithCharArrayArg.Equals(method) - // SqlServer LTRIM does not take arguments && ((arguments[0] as SqlConstantExpression)?.Value as Array)?.Length == 0)) { return _sqlExpressionFactory.Function( @@ -194,9 +204,35 @@ public SqlServerStringMethodTranslator(ISqlExpressionFactory sqlExpressionFactor instance.TypeMapping); } + if (TrimStartMethodInfoWithCharArg.Equals(method) + && _sqlServerSingletonOptions.CompatibilityLevel >= 160) + { + return _sqlExpressionFactory.Function( + "LTRIM", + new[] { instance, arguments[0] }, + nullable: true, + argumentsPropagateNullability: new[] { true, true }, + instance.Type, + instance.TypeMapping); + } + + if (TrimStartMethodInfoWithCharArrayArg.Equals(method) + && _sqlServerSingletonOptions.CompatibilityLevel >= 160) + { + var firstArgumentValue = (arguments[0] as SqlConstantExpression)?.Value as char[]; + var firstArgument = _sqlExpressionFactory.Constant(new string(firstArgumentValue), instance.TypeMapping); + + return _sqlExpressionFactory.Function( + "LTRIM", + new[] { instance, firstArgument }, + nullable: true, + argumentsPropagateNullability: new[] { true, true }, + instance.Type, + instance.TypeMapping); + } + if (TrimEndMethodInfoWithoutArgs.Equals(method) || (TrimEndMethodInfoWithCharArrayArg.Equals(method) - // SqlServer RTRIM does not take arguments && ((arguments[0] as SqlConstantExpression)?.Value as Array)?.Length == 0)) { return _sqlExpressionFactory.Function( @@ -208,6 +244,34 @@ public SqlServerStringMethodTranslator(ISqlExpressionFactory sqlExpressionFactor instance.TypeMapping); } + + if (TrimEndMethodInfoWithCharArg.Equals(method) + && _sqlServerSingletonOptions.CompatibilityLevel >= 160) + { + return _sqlExpressionFactory.Function( + "RTRIM", + new[] { instance, arguments[0] }, + nullable: true, + argumentsPropagateNullability: new[] { true, true }, + instance.Type, + instance.TypeMapping); + } + + if (TrimEndMethodInfoWithCharArrayArg.Equals(method) + && _sqlServerSingletonOptions.CompatibilityLevel >= 160) + { + var firstArgumentValue = (arguments[0] as SqlConstantExpression)?.Value as char[]; + var firstArgument = _sqlExpressionFactory.Constant(new string(firstArgumentValue), instance.TypeMapping); + + return _sqlExpressionFactory.Function( + "RTRIM", + new[] { instance, firstArgument }, + nullable: true, + argumentsPropagateNullability: new[] { true, true }, + instance.Type, + instance.TypeMapping); + } + if (TrimMethodInfoWithoutArgs.Equals(method) || (TrimMethodInfoWithCharArrayArg.Equals(method) // SqlServer LTRIM/RTRIM does not take arguments diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs index ae4e0c9cbdd..c02a3032581 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs @@ -2542,18 +2542,26 @@ WHERE LTRIM([c].[ContactTitle]) = N'Owner' public override async Task TrimStart_with_char_argument_in_predicate(bool async) { - // String.Trim with parameters. Issue #22927. - await AssertTranslationFailed(() => base.TrimStart_with_char_argument_in_predicate(async)); + await base.TrimStart_with_char_argument_in_predicate(async); - AssertSql(); + 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 LTRIM([c].[ContactTitle], N'O') = N'wner' +"""); } public override async Task TrimStart_with_char_array_argument_in_predicate(bool async) { - // String.Trim with parameters. Issue #22927. - await AssertTranslationFailed(() => base.TrimStart_with_char_array_argument_in_predicate(async)); + await base.TrimStart_with_char_array_argument_in_predicate(async); - AssertSql(); + 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 LTRIM([c].[ContactTitle], N'Ow') = N'ner' +"""); } public override async Task TrimEnd_without_arguments_in_predicate(bool async) @@ -2570,18 +2578,26 @@ WHERE RTRIM([c].[ContactTitle]) = N'Owner' public override async Task TrimEnd_with_char_argument_in_predicate(bool async) { - // String.Trim with parameters. Issue #22927. - await AssertTranslationFailed(() => base.TrimEnd_with_char_argument_in_predicate(async)); + await base.TrimEnd_with_char_argument_in_predicate(async); - AssertSql(); + 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 RTRIM([c].[ContactTitle], N'r') = N'Owne' +"""); } public override async Task TrimEnd_with_char_array_argument_in_predicate(bool async) { - // String.Trim with parameters. Issue #22927. - await AssertTranslationFailed(() => base.TrimEnd_with_char_array_argument_in_predicate(async)); + await base.TrimEnd_with_char_array_argument_in_predicate(async); - AssertSql(); + 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 RTRIM([c].[ContactTitle], N'er') = N'Own' +"""); } public override async Task Trim_without_argument_in_predicate(bool async) From e1a73e70a2aec23a37f9971ad73da786d526811f Mon Sep 17 00:00:00 2001 From: Abdiel Carrazana Date: Thu, 16 May 2024 13:43:04 -0400 Subject: [PATCH 2/7] Using SqlServerCondition.SupportsFunctions2022 for LTRIM and RTRIM. Handling compatibility level to low. --- .../Properties/SqlServerStrings.Designer.cs | 8 +++++++ .../Properties/SqlServerStrings.resx | 3 +++ .../SqlServerStringMethodTranslator.cs | 21 +++++++++++++++---- .../NorthwindFunctionsQuerySqlServerTest.cs | 4 ++++ 4 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs index 0401a37e299..9995586cd7f 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs @@ -407,6 +407,14 @@ public static string TemporalSetOperationOnMismatchedSources(object? entityType) public static string TransientExceptionDetected => GetString("TransientExceptionDetected"); + /// + /// This usage of string.TrimStart or string.TrimEnd with args requires SQL Server functions LTRIM and RTRIM, which require compatibility level 160. + /// + public static string TrimStartTrimEndWithArgsCompatibilityLevelTooLow(object? compatibilityLevel) + => string.Format( + GetString("TrimStartTrimEndWithArgsCompatibilityLevelTooLow", nameof(compatibilityLevel)), + compatibilityLevel); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name)!; diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx index 8a229138734..befd75740f4 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx @@ -362,4 +362,7 @@ An exception has been raised that is likely due to a transient failure. Consider enabling transient error resiliency by adding 'EnableRetryOnFailure' to the 'UseSqlServer' call. + + EF Core's SQL Server compatibility level is set to {compatibilityLevel}; compatibility level 160 (SQL Server 2022) is the minimum for functions LTRIM and RTRIM with the optional characters positional argument. + \ No newline at end of file diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerStringMethodTranslator.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerStringMethodTranslator.cs index cb5fd2bfd18..749b4e737e6 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerStringMethodTranslator.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerStringMethodTranslator.cs @@ -3,6 +3,7 @@ using Microsoft.EntityFrameworkCore.Query.SqlExpressions; using Microsoft.EntityFrameworkCore.SqlServer.Infrastructure.Internal; +using Microsoft.EntityFrameworkCore.SqlServer.Internal; using ExpressionExtensions = Microsoft.EntityFrameworkCore.Query.ExpressionExtensions; namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal; @@ -205,7 +206,7 @@ public SqlServerStringMethodTranslator(ISqlExpressionFactory sqlExpressionFactor } if (TrimStartMethodInfoWithCharArg.Equals(method) - && _sqlServerSingletonOptions.CompatibilityLevel >= 160) + && CheckTrimStartTrimEndWithArgsCompatibilityLevel()) { return _sqlExpressionFactory.Function( "LTRIM", @@ -217,7 +218,7 @@ public SqlServerStringMethodTranslator(ISqlExpressionFactory sqlExpressionFactor } if (TrimStartMethodInfoWithCharArrayArg.Equals(method) - && _sqlServerSingletonOptions.CompatibilityLevel >= 160) + && CheckTrimStartTrimEndWithArgsCompatibilityLevel()) { var firstArgumentValue = (arguments[0] as SqlConstantExpression)?.Value as char[]; var firstArgument = _sqlExpressionFactory.Constant(new string(firstArgumentValue), instance.TypeMapping); @@ -246,7 +247,7 @@ public SqlServerStringMethodTranslator(ISqlExpressionFactory sqlExpressionFactor if (TrimEndMethodInfoWithCharArg.Equals(method) - && _sqlServerSingletonOptions.CompatibilityLevel >= 160) + && CheckTrimStartTrimEndWithArgsCompatibilityLevel()) { return _sqlExpressionFactory.Function( "RTRIM", @@ -258,7 +259,7 @@ public SqlServerStringMethodTranslator(ISqlExpressionFactory sqlExpressionFactor } if (TrimEndMethodInfoWithCharArrayArg.Equals(method) - && _sqlServerSingletonOptions.CompatibilityLevel >= 160) + && CheckTrimStartTrimEndWithArgsCompatibilityLevel()) { var firstArgumentValue = (arguments[0] as SqlConstantExpression)?.Value as char[]; var firstArgument = _sqlExpressionFactory.Constant(new string(firstArgumentValue), instance.TypeMapping); @@ -421,4 +422,16 @@ private SqlExpression TranslateIndexOf( }, charIndexExpression); } + + private bool CheckTrimStartTrimEndWithArgsCompatibilityLevel() + => CheckCompatibilityLevel(160, SqlServerStrings.TrimStartTrimEndWithArgsCompatibilityLevelTooLow); + + private bool CheckCompatibilityLevel(int compatibilityLevel, Func compatibilityTooLowFunction) + { + if (_sqlServerSingletonOptions.CompatibilityLevel < compatibilityLevel) + { + throw new InvalidOperationException(compatibilityTooLowFunction(compatibilityLevel)); + } + return true; + } } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs index c02a3032581..64f3451cd3d 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs @@ -2540,6 +2540,7 @@ WHERE LTRIM([c].[ContactTitle]) = N'Owner' """); } + [SqlServerCondition(SqlServerCondition.SupportsFunctions2022)] public override async Task TrimStart_with_char_argument_in_predicate(bool async) { await base.TrimStart_with_char_argument_in_predicate(async); @@ -2552,6 +2553,7 @@ WHERE LTRIM([c].[ContactTitle], N'O') = N'wner' """); } + [SqlServerCondition(SqlServerCondition.SupportsFunctions2022)] public override async Task TrimStart_with_char_array_argument_in_predicate(bool async) { await base.TrimStart_with_char_array_argument_in_predicate(async); @@ -2576,6 +2578,7 @@ WHERE RTRIM([c].[ContactTitle]) = N'Owner' """); } + [SqlServerCondition(SqlServerCondition.SupportsFunctions2022)] public override async Task TrimEnd_with_char_argument_in_predicate(bool async) { await base.TrimEnd_with_char_argument_in_predicate(async); @@ -2588,6 +2591,7 @@ WHERE RTRIM([c].[ContactTitle], N'r') = N'Owne' """); } + [SqlServerCondition(SqlServerCondition.SupportsFunctions2022)] public override async Task TrimEnd_with_char_array_argument_in_predicate(bool async) { await base.TrimEnd_with_char_array_argument_in_predicate(async); From 3761affca6c9f0bbcc3bd6ce2117cd6b478604d9 Mon Sep 17 00:00:00 2001 From: Abdiel Carrazana Date: Tue, 28 May 2024 12:11:57 -0400 Subject: [PATCH 3/7] Revert "Using SqlServerCondition.SupportsFunctions2022 for LTRIM and RTRIM. Handling compatibility level to low." This reverts commit e1a73e70a2aec23a37f9971ad73da786d526811f. --- .../Properties/SqlServerStrings.Designer.cs | 8 ------- .../Properties/SqlServerStrings.resx | 3 --- .../SqlServerStringMethodTranslator.cs | 21 ++++--------------- .../NorthwindFunctionsQuerySqlServerTest.cs | 4 ---- 4 files changed, 4 insertions(+), 32 deletions(-) diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs index 9995586cd7f..0401a37e299 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs @@ -407,14 +407,6 @@ public static string TemporalSetOperationOnMismatchedSources(object? entityType) public static string TransientExceptionDetected => GetString("TransientExceptionDetected"); - /// - /// This usage of string.TrimStart or string.TrimEnd with args requires SQL Server functions LTRIM and RTRIM, which require compatibility level 160. - /// - public static string TrimStartTrimEndWithArgsCompatibilityLevelTooLow(object? compatibilityLevel) - => string.Format( - GetString("TrimStartTrimEndWithArgsCompatibilityLevelTooLow", nameof(compatibilityLevel)), - compatibilityLevel); - private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name)!; diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx index befd75740f4..8a229138734 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx @@ -362,7 +362,4 @@ An exception has been raised that is likely due to a transient failure. Consider enabling transient error resiliency by adding 'EnableRetryOnFailure' to the 'UseSqlServer' call. - - EF Core's SQL Server compatibility level is set to {compatibilityLevel}; compatibility level 160 (SQL Server 2022) is the minimum for functions LTRIM and RTRIM with the optional characters positional argument. - \ No newline at end of file diff --git a/src/EFCore.SqlServer/Query/Internal/Translators/SqlServerStringMethodTranslator.cs b/src/EFCore.SqlServer/Query/Internal/Translators/SqlServerStringMethodTranslator.cs index 2a23daab7b4..67c9f378a8d 100644 --- a/src/EFCore.SqlServer/Query/Internal/Translators/SqlServerStringMethodTranslator.cs +++ b/src/EFCore.SqlServer/Query/Internal/Translators/SqlServerStringMethodTranslator.cs @@ -3,7 +3,6 @@ using Microsoft.EntityFrameworkCore.Query.SqlExpressions; using Microsoft.EntityFrameworkCore.SqlServer.Infrastructure.Internal; -using Microsoft.EntityFrameworkCore.SqlServer.Internal; using ExpressionExtensions = Microsoft.EntityFrameworkCore.Query.ExpressionExtensions; namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal.Translators; @@ -206,7 +205,7 @@ public SqlServerStringMethodTranslator(ISqlExpressionFactory sqlExpressionFactor } if (TrimStartMethodInfoWithCharArg.Equals(method) - && CheckTrimStartTrimEndWithArgsCompatibilityLevel()) + && _sqlServerSingletonOptions.CompatibilityLevel >= 160) { return _sqlExpressionFactory.Function( "LTRIM", @@ -218,7 +217,7 @@ public SqlServerStringMethodTranslator(ISqlExpressionFactory sqlExpressionFactor } if (TrimStartMethodInfoWithCharArrayArg.Equals(method) - && CheckTrimStartTrimEndWithArgsCompatibilityLevel()) + && _sqlServerSingletonOptions.CompatibilityLevel >= 160) { var firstArgumentValue = (arguments[0] as SqlConstantExpression)?.Value as char[]; var firstArgument = _sqlExpressionFactory.Constant(new string(firstArgumentValue), instance.TypeMapping); @@ -247,7 +246,7 @@ public SqlServerStringMethodTranslator(ISqlExpressionFactory sqlExpressionFactor if (TrimEndMethodInfoWithCharArg.Equals(method) - && CheckTrimStartTrimEndWithArgsCompatibilityLevel()) + && _sqlServerSingletonOptions.CompatibilityLevel >= 160) { return _sqlExpressionFactory.Function( "RTRIM", @@ -259,7 +258,7 @@ public SqlServerStringMethodTranslator(ISqlExpressionFactory sqlExpressionFactor } if (TrimEndMethodInfoWithCharArrayArg.Equals(method) - && CheckTrimStartTrimEndWithArgsCompatibilityLevel()) + && _sqlServerSingletonOptions.CompatibilityLevel >= 160) { var firstArgumentValue = (arguments[0] as SqlConstantExpression)?.Value as char[]; var firstArgument = _sqlExpressionFactory.Constant(new string(firstArgumentValue), instance.TypeMapping); @@ -422,16 +421,4 @@ private SqlExpression TranslateIndexOf( }, charIndexExpression); } - - private bool CheckTrimStartTrimEndWithArgsCompatibilityLevel() - => CheckCompatibilityLevel(160, SqlServerStrings.TrimStartTrimEndWithArgsCompatibilityLevelTooLow); - - private bool CheckCompatibilityLevel(int compatibilityLevel, Func compatibilityTooLowFunction) - { - if (_sqlServerSingletonOptions.CompatibilityLevel < compatibilityLevel) - { - throw new InvalidOperationException(compatibilityTooLowFunction(compatibilityLevel)); - } - return true; - } } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs index 64f3451cd3d..c02a3032581 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs @@ -2540,7 +2540,6 @@ WHERE LTRIM([c].[ContactTitle]) = N'Owner' """); } - [SqlServerCondition(SqlServerCondition.SupportsFunctions2022)] public override async Task TrimStart_with_char_argument_in_predicate(bool async) { await base.TrimStart_with_char_argument_in_predicate(async); @@ -2553,7 +2552,6 @@ WHERE LTRIM([c].[ContactTitle], N'O') = N'wner' """); } - [SqlServerCondition(SqlServerCondition.SupportsFunctions2022)] public override async Task TrimStart_with_char_array_argument_in_predicate(bool async) { await base.TrimStart_with_char_array_argument_in_predicate(async); @@ -2578,7 +2576,6 @@ WHERE RTRIM([c].[ContactTitle]) = N'Owner' """); } - [SqlServerCondition(SqlServerCondition.SupportsFunctions2022)] public override async Task TrimEnd_with_char_argument_in_predicate(bool async) { await base.TrimEnd_with_char_argument_in_predicate(async); @@ -2591,7 +2588,6 @@ WHERE RTRIM([c].[ContactTitle], N'r') = N'Owne' """); } - [SqlServerCondition(SqlServerCondition.SupportsFunctions2022)] public override async Task TrimEnd_with_char_array_argument_in_predicate(bool async) { await base.TrimEnd_with_char_array_argument_in_predicate(async); From 8df4eec6b0ad69a445c52fee140588775689d381 Mon Sep 17 00:00:00 2001 From: Abdiel Carrazana Date: Tue, 28 May 2024 13:32:33 -0400 Subject: [PATCH 4/7] Rolling back throwing exceptions gracefully. Processing trim start and end in a consistent way. --- .../SqlServerStringMethodTranslator.cs | 134 ++++++++---------- .../NorthwindFunctionsQuerySqlServerTest.cs | 4 + 2 files changed, 60 insertions(+), 78 deletions(-) diff --git a/src/EFCore.SqlServer/Query/Internal/Translators/SqlServerStringMethodTranslator.cs b/src/EFCore.SqlServer/Query/Internal/Translators/SqlServerStringMethodTranslator.cs index 67c9f378a8d..b952dfc84bb 100644 --- a/src/EFCore.SqlServer/Query/Internal/Translators/SqlServerStringMethodTranslator.cs +++ b/src/EFCore.SqlServer/Query/Internal/Translators/SqlServerStringMethodTranslator.cs @@ -191,86 +191,22 @@ public SqlServerStringMethodTranslator(ISqlExpressionFactory sqlExpressionFactor instance.TypeMapping); } - if (TrimStartMethodInfoWithoutArgs.Equals(method) - || (TrimStartMethodInfoWithCharArrayArg.Equals(method) - && ((arguments[0] as SqlConstantExpression)?.Value as Array)?.Length == 0)) - { - return _sqlExpressionFactory.Function( - "LTRIM", - new[] { instance }, - nullable: true, - argumentsPropagateNullability: new[] { true }, - instance.Type, - instance.TypeMapping); - } - - if (TrimStartMethodInfoWithCharArg.Equals(method) - && _sqlServerSingletonOptions.CompatibilityLevel >= 160) - { - return _sqlExpressionFactory.Function( - "LTRIM", - new[] { instance, arguments[0] }, - nullable: true, - argumentsPropagateNullability: new[] { true, true }, - instance.Type, - instance.TypeMapping); - } - - if (TrimStartMethodInfoWithCharArrayArg.Equals(method) - && _sqlServerSingletonOptions.CompatibilityLevel >= 160) - { - var firstArgumentValue = (arguments[0] as SqlConstantExpression)?.Value as char[]; - var firstArgument = _sqlExpressionFactory.Constant(new string(firstArgumentValue), instance.TypeMapping); - - return _sqlExpressionFactory.Function( - "LTRIM", - new[] { instance, firstArgument }, - nullable: true, - argumentsPropagateNullability: new[] { true, true }, - instance.Type, - instance.TypeMapping); - } - - if (TrimEndMethodInfoWithoutArgs.Equals(method) - || (TrimEndMethodInfoWithCharArrayArg.Equals(method) - && ((arguments[0] as SqlConstantExpression)?.Value as Array)?.Length == 0)) - { - return _sqlExpressionFactory.Function( - "RTRIM", - new[] { instance }, - nullable: true, - argumentsPropagateNullability: new[] { true }, - instance.Type, - instance.TypeMapping); - } - - - if (TrimEndMethodInfoWithCharArg.Equals(method) - && _sqlServerSingletonOptions.CompatibilityLevel >= 160) - { - return _sqlExpressionFactory.Function( - "RTRIM", - new[] { instance, arguments[0] }, - nullable: true, - argumentsPropagateNullability: new[] { true, true }, - instance.Type, - instance.TypeMapping); - } - - if (TrimEndMethodInfoWithCharArrayArg.Equals(method) - && _sqlServerSingletonOptions.CompatibilityLevel >= 160) + if (_sqlServerSingletonOptions.CompatibilityLevel >= 160) { - var firstArgumentValue = (arguments[0] as SqlConstantExpression)?.Value as char[]; - var firstArgument = _sqlExpressionFactory.Constant(new string(firstArgumentValue), instance.TypeMapping); + if (TrimStartMethodInfoWithoutArgs.Equals(method) + || TrimStartMethodInfoWithCharArg.Equals(method) + || TrimStartMethodInfoWithCharArrayArg.Equals(method)) + { + return ProcessTrimMethod(instance, arguments, "LTRIM"); + } - return _sqlExpressionFactory.Function( - "RTRIM", - new[] { instance, firstArgument }, - nullable: true, - argumentsPropagateNullability: new[] { true, true }, - instance.Type, - instance.TypeMapping); - } + if (TrimEndMethodInfoWithoutArgs.Equals(method) + || TrimEndMethodInfoWithCharArg.Equals(method) + || TrimEndMethodInfoWithCharArrayArg.Equals(method)) + { + return ProcessTrimMethod(instance, arguments, "RTRIM"); + } + } if (TrimMethodInfoWithoutArgs.Equals(method) || (TrimMethodInfoWithCharArrayArg.Equals(method) @@ -421,4 +357,46 @@ private SqlExpression TranslateIndexOf( }, charIndexExpression); } + + private SqlExpression? ProcessTrimMethod(SqlExpression instance, IReadOnlyList arguments, string functionName) + { + var typeMapping = instance.TypeMapping; + if (typeMapping == null) + { + return null; + } + + var sqlArguments = new List { instance }; + if (arguments.Count == 1) + { + var constantValue = (arguments[0] as SqlConstantExpression)?.Value; + var charactersToTrim = new List(); + + if (constantValue is char singleChar) + { + charactersToTrim.Add(singleChar); + } + else if (constantValue is char[] charArray) + { + charactersToTrim.AddRange(charArray); + } + else + { + return null; + } + + if (charactersToTrim.Count > 0) + { + sqlArguments.Add(_sqlExpressionFactory.Constant(new string(charactersToTrim.ToArray()), typeMapping)); + } + } + + return _sqlExpressionFactory.Function( + functionName, + sqlArguments, + nullable: true, + argumentsPropagateNullability: sqlArguments.Select(_ => true).ToList(), + instance.Type, + typeMapping); + } } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs index c02a3032581..64f3451cd3d 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs @@ -2540,6 +2540,7 @@ WHERE LTRIM([c].[ContactTitle]) = N'Owner' """); } + [SqlServerCondition(SqlServerCondition.SupportsFunctions2022)] public override async Task TrimStart_with_char_argument_in_predicate(bool async) { await base.TrimStart_with_char_argument_in_predicate(async); @@ -2552,6 +2553,7 @@ WHERE LTRIM([c].[ContactTitle], N'O') = N'wner' """); } + [SqlServerCondition(SqlServerCondition.SupportsFunctions2022)] public override async Task TrimStart_with_char_array_argument_in_predicate(bool async) { await base.TrimStart_with_char_array_argument_in_predicate(async); @@ -2576,6 +2578,7 @@ WHERE RTRIM([c].[ContactTitle]) = N'Owner' """); } + [SqlServerCondition(SqlServerCondition.SupportsFunctions2022)] public override async Task TrimEnd_with_char_argument_in_predicate(bool async) { await base.TrimEnd_with_char_argument_in_predicate(async); @@ -2588,6 +2591,7 @@ WHERE RTRIM([c].[ContactTitle], N'r') = N'Owne' """); } + [SqlServerCondition(SqlServerCondition.SupportsFunctions2022)] public override async Task TrimEnd_with_char_array_argument_in_predicate(bool async) { await base.TrimEnd_with_char_array_argument_in_predicate(async); From 86d51737836258d693dac636fcd3a74851208a4e Mon Sep 17 00:00:00 2001 From: abdielcs Date: Fri, 14 Jun 2024 11:01:04 -0400 Subject: [PATCH 5/7] Using StringBuilder instead of List. Other minor fixes. --- .../SqlServerMethodCallTranslatorProvider.cs | 1 + .../SqlServerStringMethodTranslator.cs | 15 ++++++++------- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerMethodCallTranslatorProvider.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerMethodCallTranslatorProvider.cs index 2498b23653b..9c960d29c1c 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerMethodCallTranslatorProvider.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerMethodCallTranslatorProvider.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using Microsoft.EntityFrameworkCore.SqlServer.Infrastructure.Internal; + namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal; /// diff --git a/src/EFCore.SqlServer/Query/Internal/Translators/SqlServerStringMethodTranslator.cs b/src/EFCore.SqlServer/Query/Internal/Translators/SqlServerStringMethodTranslator.cs index 234f5002eb4..5fb4590c6e7 100644 --- a/src/EFCore.SqlServer/Query/Internal/Translators/SqlServerStringMethodTranslator.cs +++ b/src/EFCore.SqlServer/Query/Internal/Translators/SqlServerStringMethodTranslator.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Text; using Microsoft.EntityFrameworkCore.Query.SqlExpressions; using Microsoft.EntityFrameworkCore.SqlServer.Infrastructure.Internal; using ExpressionExtensions = Microsoft.EntityFrameworkCore.Query.ExpressionExtensions; @@ -195,8 +196,8 @@ public SqlServerStringMethodTranslator(ISqlExpressionFactory sqlExpressionFactor if (_sqlServerSingletonOptions.CompatibilityLevel >= 160) { if (TrimStartMethodInfoWithoutArgs.Equals(method) - || TrimStartMethodInfoWithCharArg.Equals(method) - || TrimStartMethodInfoWithCharArrayArg.Equals(method)) + || TrimStartMethodInfoWithCharArg.Equals(method) + || TrimStartMethodInfoWithCharArrayArg.Equals(method)) { return ProcessTrimMethod(instance, arguments, "LTRIM"); } @@ -375,24 +376,24 @@ private SqlExpression TranslateIndexOf( if (arguments.Count == 1) { var constantValue = (arguments[0] as SqlConstantExpression)?.Value; - var charactersToTrim = new List(); + var charactersToTrim = new StringBuilder(); if (constantValue is char singleChar) { - charactersToTrim.Add(singleChar); + charactersToTrim.Append(singleChar); } else if (constantValue is char[] charArray) { - charactersToTrim.AddRange(charArray); + charactersToTrim.Append(charArray); } else { return null; } - if (charactersToTrim.Count > 0) + if (charactersToTrim.Length > 0) { - sqlArguments.Add(_sqlExpressionFactory.Constant(new string(charactersToTrim.ToArray()), typeMapping)); + sqlArguments.Add(_sqlExpressionFactory.Constant(new string(charactersToTrim.ToString()), typeMapping)); } } From 46a8f782665bff2bf3169068756f717e301b898b Mon Sep 17 00:00:00 2001 From: abdielcs Date: Wed, 19 Jun 2024 11:07:21 -0400 Subject: [PATCH 6/7] Only TrimStart and TrimEnd with the optional char or array must be validated by compatibility level. --- .../SqlServerStringMethodTranslator.cs | 46 +++++++++---------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/src/EFCore.SqlServer/Query/Internal/Translators/SqlServerStringMethodTranslator.cs b/src/EFCore.SqlServer/Query/Internal/Translators/SqlServerStringMethodTranslator.cs index 3df03ddad1e..494daaa13a3 100644 --- a/src/EFCore.SqlServer/Query/Internal/Translators/SqlServerStringMethodTranslator.cs +++ b/src/EFCore.SqlServer/Query/Internal/Translators/SqlServerStringMethodTranslator.cs @@ -198,22 +198,21 @@ public SqlServerStringMethodTranslator(ISqlExpressionFactory sqlExpressionFactor instance.TypeMapping); } - if (_sqlServerSingletonOptions.CompatibilityLevel >= 160) + if (TrimStartMethodInfoWithoutArgs.Equals(method) + || (_sqlServerSingletonOptions.CompatibilityLevel >= 160 + && (TrimStartMethodInfoWithCharArg.Equals(method) + || TrimStartMethodInfoWithCharArrayArg.Equals(method)))) { - if (TrimStartMethodInfoWithoutArgs.Equals(method) - || TrimStartMethodInfoWithCharArg.Equals(method) - || TrimStartMethodInfoWithCharArrayArg.Equals(method)) - { - return ProcessTrimMethod(instance, arguments, "LTRIM"); - } + return ProcessTrimMethod(instance, arguments, "LTRIM"); + } - if (TrimEndMethodInfoWithoutArgs.Equals(method) - || TrimEndMethodInfoWithCharArg.Equals(method) - || TrimEndMethodInfoWithCharArrayArg.Equals(method)) - { - return ProcessTrimMethod(instance, arguments, "RTRIM"); - } - } + if (TrimEndMethodInfoWithoutArgs.Equals(method) + || (_sqlServerSingletonOptions.CompatibilityLevel >= 160 + && (TrimEndMethodInfoWithCharArg.Equals(method) + || TrimEndMethodInfoWithCharArrayArg.Equals(method)))) + { + return ProcessTrimMethod(instance, arguments, "RTRIM"); + } if (TrimMethodInfoWithoutArgs.Equals(method) || (TrimMethodInfoWithCharArrayArg.Equals(method) @@ -397,17 +396,16 @@ private SqlExpression TranslateIndexOf( var constantValue = (arguments[0] as SqlConstantExpression)?.Value; var charactersToTrim = new StringBuilder(); - if (constantValue is char singleChar) - { - charactersToTrim.Append(singleChar); - } - else if (constantValue is char[] charArray) - { - charactersToTrim.Append(charArray); - } - else + switch(constantValue) { - return null; + case char singleChar: + charactersToTrim.Append(singleChar); + break; + case char[] charArray: + charactersToTrim.Append(charArray); + break; + default: + return null; } if (charactersToTrim.Length > 0) From fcda1f8159055cd2283812d0bb1e59076bf4e9e4 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Thu, 20 Jun 2024 22:28:03 +0200 Subject: [PATCH 7/7] Fix method matching and cleanup --- .../SqlServerStringMethodTranslator.cs | 65 +++++++------------ 1 file changed, 23 insertions(+), 42 deletions(-) diff --git a/src/EFCore.SqlServer/Query/Internal/Translators/SqlServerStringMethodTranslator.cs b/src/EFCore.SqlServer/Query/Internal/Translators/SqlServerStringMethodTranslator.cs index 494daaa13a3..f1663ec86cb 100644 --- a/src/EFCore.SqlServer/Query/Internal/Translators/SqlServerStringMethodTranslator.cs +++ b/src/EFCore.SqlServer/Query/Internal/Translators/SqlServerStringMethodTranslator.cs @@ -198,26 +198,26 @@ public SqlServerStringMethodTranslator(ISqlExpressionFactory sqlExpressionFactor instance.TypeMapping); } - if (TrimStartMethodInfoWithoutArgs.Equals(method) + // There's single-parameter LTRIM/RTRIM for all versions (trims whitespace), but startin with SQL Server 2022 there's also + // an overload that accepts the characters to trim. + if (method == TrimStartMethodInfoWithoutArgs + || (method == TrimStartMethodInfoWithCharArrayArg && arguments[0] is SqlConstantExpression { Value: char[] { Length: 0 } }) || (_sqlServerSingletonOptions.CompatibilityLevel >= 160 - && (TrimStartMethodInfoWithCharArg.Equals(method) - || TrimStartMethodInfoWithCharArrayArg.Equals(method)))) + && (method == TrimStartMethodInfoWithCharArg || method == TrimStartMethodInfoWithCharArrayArg))) { - return ProcessTrimMethod(instance, arguments, "LTRIM"); + return ProcessTrimStartEnd(instance, arguments, "LTRIM"); } - if (TrimEndMethodInfoWithoutArgs.Equals(method) + if (method == TrimEndMethodInfoWithoutArgs + || (method == TrimEndMethodInfoWithCharArrayArg && arguments[0] is SqlConstantExpression { Value: char[] { Length: 0 } }) || (_sqlServerSingletonOptions.CompatibilityLevel >= 160 - && (TrimEndMethodInfoWithCharArg.Equals(method) - || TrimEndMethodInfoWithCharArrayArg.Equals(method)))) + && (method == TrimEndMethodInfoWithCharArg || method == TrimEndMethodInfoWithCharArrayArg))) { - return ProcessTrimMethod(instance, arguments, "RTRIM"); + return ProcessTrimStartEnd(instance, arguments, "RTRIM"); } - if (TrimMethodInfoWithoutArgs.Equals(method) - || (TrimMethodInfoWithCharArrayArg.Equals(method) - // SqlServer LTRIM/RTRIM does not take arguments - && ((arguments[0] as SqlConstantExpression)?.Value as Array)?.Length == 0)) + if (method == TrimMethodInfoWithoutArgs + || (method == TrimMethodInfoWithCharArrayArg && arguments[0] is SqlConstantExpression { Value: char[] { Length: 0 } })) { return _sqlExpressionFactory.Function( "LTRIM", @@ -382,44 +382,25 @@ private SqlExpression TranslateIndexOf( return _sqlExpressionFactory.Subtract(charIndexExpression, offsetExpression); } - private SqlExpression? ProcessTrimMethod(SqlExpression instance, IReadOnlyList arguments, string functionName) + private SqlExpression? ProcessTrimStartEnd(SqlExpression instance, IReadOnlyList arguments, string functionName) { - var typeMapping = instance.TypeMapping; - if (typeMapping == null) + SqlConstantExpression? charactersToTrim = null; + if (arguments.Count > 0 && arguments[0] is SqlConstantExpression { Value: var charactersToTrimValue }) { - return null; - } - - var sqlArguments = new List { instance }; - if (arguments.Count == 1) - { - var constantValue = (arguments[0] as SqlConstantExpression)?.Value; - var charactersToTrim = new StringBuilder(); - - switch(constantValue) + charactersToTrim = charactersToTrimValue switch { - case char singleChar: - charactersToTrim.Append(singleChar); - break; - case char[] charArray: - charactersToTrim.Append(charArray); - break; - default: - return null; - } - - if (charactersToTrim.Length > 0) - { - sqlArguments.Add(_sqlExpressionFactory.Constant(new string(charactersToTrim.ToString()), typeMapping)); - } + char singleChar => _sqlExpressionFactory.Constant(singleChar.ToString(), instance.TypeMapping), + char[] charArray => _sqlExpressionFactory.Constant(new string(charArray), instance.TypeMapping), + _ => throw new UnreachableException("Invalid parameter type for string.TrimStart/TrimEnd") + }; } return _sqlExpressionFactory.Function( functionName, - sqlArguments, + arguments: charactersToTrim is null ? [instance] : [instance, charactersToTrim], nullable: true, - argumentsPropagateNullability: sqlArguments.Select(_ => true).ToList(), + argumentsPropagateNullability: charactersToTrim is null ? [true] : [true, true], instance.Type, - typeMapping); + instance.TypeMapping); } }