diff --git a/src/EFCore.Relational/Query/ISqlExpressionFactory.cs b/src/EFCore.Relational/Query/ISqlExpressionFactory.cs index ccfc99a3c8e..191e869380e 100644 --- a/src/EFCore.Relational/Query/ISqlExpressionFactory.cs +++ b/src/EFCore.Relational/Query/ISqlExpressionFactory.cs @@ -217,14 +217,15 @@ SqlBinaryExpression Or( RelationalTypeMapping? typeMapping = null); // Other + /// - /// Creates a which represents a COALESCE operation. + /// Creates an expression which represents a SQL COALESCE operation. /// /// The left operand. /// The right operand. /// A type mapping to be assigned to the created expression. /// An expression representing a SQL COALESCE operation. - SqlFunctionExpression Coalesce( + SqlExpression Coalesce( SqlExpression left, SqlExpression right, RelationalTypeMapping? typeMapping = null); diff --git a/src/EFCore.Relational/Query/SqlExpressionFactory.cs b/src/EFCore.Relational/Query/SqlExpressionFactory.cs index 0a7ae6228eb..390ef33b530 100644 --- a/src/EFCore.Relational/Query/SqlExpressionFactory.cs +++ b/src/EFCore.Relational/Query/SqlExpressionFactory.cs @@ -481,8 +481,19 @@ public virtual SqlBinaryExpression Or(SqlExpression left, SqlExpression right, R => MakeBinary(ExpressionType.Or, left, right, typeMapping)!; /// - public virtual SqlFunctionExpression Coalesce(SqlExpression left, SqlExpression right, RelationalTypeMapping? typeMapping = null) + public virtual SqlExpression Coalesce(SqlExpression left, SqlExpression right, RelationalTypeMapping? typeMapping = null) { + // TODO: generalize this if we ever introduce nullability on SqlExpression, #33889 + switch (left) + { + case SqlConstantExpression { Value: not null }: + case ColumnExpression { IsNullable: false }: + return left; + + case SqlConstantExpression { Value: null }: + return right; + } + var resultType = right.Type; var inferredTypeMapping = typeMapping ?? ExpressionExtensions.InferTypeMapping(left, right) diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerSqlTranslatingExpressionVisitor.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerSqlTranslatingExpressionVisitor.cs index 438a00a3d86..0546ba3f6c0 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerSqlTranslatingExpressionVisitor.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerSqlTranslatingExpressionVisitor.cs @@ -5,6 +5,7 @@ using System.Globalization; using System.Text; using Microsoft.EntityFrameworkCore.Query.SqlExpressions; +using Microsoft.EntityFrameworkCore.SqlServer.Infrastructure.Internal; using ExpressionExtensions = Microsoft.EntityFrameworkCore.Query.ExpressionExtensions; namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal; @@ -19,6 +20,8 @@ public class SqlServerSqlTranslatingExpressionVisitor : RelationalSqlTranslating { private readonly SqlServerQueryCompilationContext _queryCompilationContext; private readonly ISqlExpressionFactory _sqlExpressionFactory; + private readonly IRelationalTypeMappingSource _typeMappingSource; + private readonly int _sqlServerCompatibilityLevel; private static readonly HashSet DateTimeDataTypes = @@ -59,6 +62,9 @@ private static readonly MethodInfo StringEndsWithMethodInfo private static readonly MethodInfo StringContainsMethodInfo = typeof(string).GetRuntimeMethod(nameof(string.Contains), [typeof(string)])!; + private static readonly MethodInfo StringJoinMethodInfo + = typeof(string).GetRuntimeMethod(nameof(string.Join), [typeof(string), typeof(string[])])!; + private static readonly MethodInfo EscapeLikePatternParameterMethod = typeof(SqlServerSqlTranslatingExpressionVisitor).GetTypeInfo().GetDeclaredMethod(nameof(ConstructLikePatternParameter))!; @@ -74,11 +80,14 @@ private static readonly MethodInfo StringContainsMethodInfo public SqlServerSqlTranslatingExpressionVisitor( RelationalSqlTranslatingExpressionVisitorDependencies dependencies, SqlServerQueryCompilationContext queryCompilationContext, - QueryableMethodTranslatingExpressionVisitor queryableMethodTranslatingExpressionVisitor) + QueryableMethodTranslatingExpressionVisitor queryableMethodTranslatingExpressionVisitor, + ISqlServerSingletonOptions sqlServerSingletonOptions) : base(dependencies, queryCompilationContext, queryableMethodTranslatingExpressionVisitor) { _queryCompilationContext = queryCompilationContext; _sqlExpressionFactory = dependencies.SqlExpressionFactory; + _typeMappingSource = dependencies.TypeMappingSource; + _sqlServerCompatibilityLevel = sqlServerSingletonOptions.CompatibilityLevel; } /// @@ -199,6 +208,56 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp return translation3; } + // Translate non-aggregate string.Join to CONCAT_WS (for aggregate string.Join, see SqlServerStringAggregateMethodTranslator) + if (method == StringJoinMethodInfo + && methodCallExpression.Arguments[1] is NewArrayExpression newArrayExpression + && _sqlServerCompatibilityLevel >= 140) + { + if (TranslationFailed(methodCallExpression.Arguments[0], Visit(methodCallExpression.Arguments[0]), out var delimiter)) + { + return QueryCompilationContext.NotTranslatedExpression; + } + + var arguments = new SqlExpression[newArrayExpression.Expressions.Count + 1]; + arguments[0] = delimiter!; + + var isUnicode = delimiter!.TypeMapping?.IsUnicode == true; + var stringTypeMapping = _typeMappingSource.FindMapping(typeof(string)); + + for (var i = 0; i < newArrayExpression.Expressions.Count; i++) + { + var argument = newArrayExpression.Expressions[i]; + if (TranslationFailed(argument, Visit(argument), out var sqlArgument)) + { + return QueryCompilationContext.NotTranslatedExpression; + } + + // CONCAT_WS returns a type with a length that varies based on actual inputs (i.e. the sum of all argument lengths, plus + // the length needed for the delimiters). We don't know column values (or even parameter values, so we always return max. + // We do vary return varchar(max) or nvarchar(max) based on whether we saw any nvarchar mapping. + if (sqlArgument!.TypeMapping?.IsUnicode == true) + { + isUnicode = true; + } + + // CONCAT_WS filters out nulls, but string.Join treats them as empty strings; so coalesce (which is a no-op for non-nullable + // arguments). + arguments[i + 1] = _sqlExpressionFactory.Coalesce( + sqlArgument, + _sqlExpressionFactory.Constant(string.Empty, typeof(string))); + } + + // CONCAT_WS never returns null; a null delimiter is interpreted as an empty string, and null arguments are skipped + // (but we coalesce them above in any case). + return Dependencies.SqlExpressionFactory.Function( + "CONCAT_WS", + arguments, + nullable: false, + argumentsPropagateNullability: new bool[arguments.Length], + typeof(string), + _typeMappingSource.FindMapping(isUnicode ? "nvarchar(max)" : "varchar(max)")); + } + return base.VisitMethodCall(methodCallExpression); bool TryTranslateStartsEndsWithContains( @@ -515,6 +574,20 @@ private Expression TranslateByteArrayElementAccess(Expression array, Expression : QueryCompilationContext.NotTranslatedExpression; } + [DebuggerStepThrough] + private static bool TranslationFailed(Expression? original, Expression? translation, out SqlExpression? castTranslation) + { + if (original != null + && translation is not SqlExpression) + { + castTranslation = null; + return true; + } + + castTranslation = translation as SqlExpression; + return false; + } + private static string? GetProviderType(SqlExpression expression) => expression.TypeMapping?.StoreType; } diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerSqlTranslatingExpressionVisitorFactory.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerSqlTranslatingExpressionVisitorFactory.cs index e038210d2cd..b2c39840b7e 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerSqlTranslatingExpressionVisitorFactory.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerSqlTranslatingExpressionVisitorFactory.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; /// @@ -11,6 +13,8 @@ namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal; /// public class SqlServerSqlTranslatingExpressionVisitorFactory : IRelationalSqlTranslatingExpressionVisitorFactory { + 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 @@ -18,9 +22,11 @@ public class SqlServerSqlTranslatingExpressionVisitorFactory : IRelationalSqlTra /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public SqlServerSqlTranslatingExpressionVisitorFactory( - RelationalSqlTranslatingExpressionVisitorDependencies dependencies) + RelationalSqlTranslatingExpressionVisitorDependencies dependencies, + ISqlServerSingletonOptions sqlServerSingletonOptions) { Dependencies = dependencies; + _sqlServerSingletonOptions = sqlServerSingletonOptions; } /// @@ -40,5 +46,6 @@ public virtual RelationalSqlTranslatingExpressionVisitor Create( => new SqlServerSqlTranslatingExpressionVisitor( Dependencies, (SqlServerQueryCompilationContext)queryCompilationContext, - queryableMethodTranslatingExpressionVisitor); + queryableMethodTranslatingExpressionVisitor, + _sqlServerSingletonOptions); } diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindFunctionsQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindFunctionsQueryCosmosTest.cs index 424a74feccb..989b27154c2 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindFunctionsQueryCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindFunctionsQueryCosmosTest.cs @@ -1940,6 +1940,9 @@ public override Task String_Join_with_ordering(bool async) public override Task String_Join_over_nullable_column(bool async) => AssertTranslationFailed(() => base.String_Join_over_nullable_column(async)); + public override Task String_Join_non_aggregate(bool async) + => AssertTranslationFailed(() => base.String_Join_non_aggregate(async)); + public override Task String_Concat(bool async) => AssertTranslationFailed(() => base.String_Concat(async)); diff --git a/test/EFCore.Specification.Tests/Query/NorthwindFunctionsQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NorthwindFunctionsQueryTestBase.cs index 40791fdab34..d533bad4764 100644 --- a/test/EFCore.Specification.Tests/Query/NorthwindFunctionsQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/NorthwindFunctionsQueryTestBase.cs @@ -271,6 +271,17 @@ public virtual Task String_Join_over_nullable_column(bool async) a.Regions.Split("|").OrderBy(id => id).ToArray()); }); + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task String_Join_non_aggregate(bool async) + { + var foo = "foo"; + + return AssertQuery( + async, + ss => ss.Set().Where(c => string.Join("|", new[] { c.CompanyName, foo, null, "bar" }) == "Around the Horn|foo||bar")); + } + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task String_Concat(bool async) diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs index ec711fabc35..534c9bad624 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs @@ -359,6 +359,21 @@ GROUP BY [c].[City] """); } + [SqlServerCondition(SqlServerCondition.SupportsFunctions2017)] + public override async Task String_Join_non_aggregate(bool async) + { + await base.String_Join_non_aggregate(async); + + AssertSql( + """ +@__foo_0='foo' (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 CONCAT_WS(N'|', [c].[CompanyName], COALESCE(@__foo_0, N''), N'', N'bar') = N'Around the Horn|foo||bar' +"""); + } + [SqlServerCondition(SqlServerCondition.SupportsFunctions2017)] public override async Task String_Concat(bool async) { diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindFunctionsQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindFunctionsQuerySqliteTest.cs index 704235085bd..6ff8a38721c 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindFunctionsQuerySqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindFunctionsQuerySqliteTest.cs @@ -989,6 +989,9 @@ GROUP BY "c"."City" """); } + public override Task String_Join_non_aggregate(bool async) + => AssertTranslationFailed(() => base.String_Join_non_aggregate(async)); + public override async Task String_Concat(bool async) { await base.String_Concat(async);