diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index 9128e7cdeba..0099c83db15 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -580,6 +580,12 @@ public static string PendingAmbientTransaction public static string SetOperationNotWithinEntityTypeHierarchy => GetString("SetOperationNotWithinEntityTypeHierarchy"); + /// + /// FromSqlRaw/FromSqlInterpolated API is being used with non-composable SQL but query is composing over it. Considering using `AsEnumerable` after FromSqlRaw/FromSqlInterpolated method to avoid composing on server side. + /// + public static string FromSqlNonComposable + => GetString("FromSqlNonComposable"); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index 7df7e5b049d..e47c5c13126 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -494,4 +494,7 @@ Set operations (Union, Concat, Intersect, Except) are only supported over entity types within the same type hierarchy. - + + FromSqlRaw/FromSqlInterpolated API is being used with non-composable SQL but query is composing over it. Considering using `AsEnumerable` after FromSqlRaw/FromSqlInterpolated method to avoid composing on server side. + + \ No newline at end of file diff --git a/src/EFCore.Relational/Query/QuerySqlGenerator.cs b/src/EFCore.Relational/Query/QuerySqlGenerator.cs index fff7701a3a5..d38e80dc755 100644 --- a/src/EFCore.Relational/Query/QuerySqlGenerator.cs +++ b/src/EFCore.Relational/Query/QuerySqlGenerator.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Linq; using System.Linq.Expressions; +using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Query.SqlExpressions; using Microsoft.EntityFrameworkCore.Storage; using Microsoft.EntityFrameworkCore.Storage.Internal; @@ -312,6 +313,11 @@ protected override Expression VisitFromSql(FromSqlExpression fromSqlExpression) { _relationalCommandBuilder.AppendLine("("); + if (!IsComposableSql(fromSqlExpression.Sql)) + { + throw new InvalidOperationException(RelationalStrings.FromSqlNonComposable); + } + using (_relationalCommandBuilder.Indent()) { GenerateFromSql(fromSqlExpression); @@ -324,6 +330,15 @@ protected override Expression VisitFromSql(FromSqlExpression fromSqlExpression) return fromSqlExpression; } + private bool IsComposableSql(string sql) + { + var trimmedSql = sql.TrimStart('\r', '\n', '\t', ' '); + + return trimmedSql.StartsWith("SELECT ", StringComparison.OrdinalIgnoreCase) + || trimmedSql.StartsWith("SELECT" + Environment.NewLine, StringComparison.OrdinalIgnoreCase) + || trimmedSql.StartsWith("SELECT\t", StringComparison.OrdinalIgnoreCase); + } + protected override Expression VisitSqlBinary(SqlBinaryExpression sqlBinaryExpression) { if (sqlBinaryExpression.OperatorType == ExpressionType.Coalesce) diff --git a/test/EFCore.Relational.Specification.Tests/Query/FromSqlSprocQueryTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/FromSqlSprocQueryTestBase.cs index f507373922d..29be08bcf90 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/FromSqlSprocQueryTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/FromSqlSprocQueryTestBase.cs @@ -2,9 +2,9 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Data.Common; using System.Linq; using System.Threading.Tasks; +using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.TestModels.Northwind; using Microsoft.EntityFrameworkCore.TestUtilities; using Xunit; @@ -62,7 +62,7 @@ public virtual async Task From_sql_queryable_stored_procedure_projection(bool as [ConditionalTheory] [InlineData(false)] [InlineData(true)] - public virtual async Task From_sql_queryable_stored_procedure_re_projection(bool async) + public virtual async Task From_sql_queryable_stored_procedure_re_projection(bool async) { using var context = CreateContext(); var query = context @@ -71,20 +71,12 @@ public virtual async Task From_sql_queryable_stored_procedure_re_pr .Select( mep => new MostExpensiveProduct { TenMostExpensiveProducts = "Foo", UnitPrice = mep.UnitPrice }); - try - { - var _ = async - ? await query.ToArrayAsync() - : query.ToArray(); - - Assert.True(false); - return null; - } - catch (Exception e) - { - Assert.IsAssignableFrom(e); - return (DbException)e; - } + + Assert.Equal( + RelationalStrings.FromSqlNonComposable, + (async + ? await Assert.ThrowsAsync(() => query.ToArrayAsync()) + : Assert.Throws(() => query.ToArray())).Message); } [ConditionalTheory] @@ -130,29 +122,20 @@ public virtual async Task From_sql_queryable_stored_procedure_with_parameter(boo [ConditionalTheory] [InlineData(false)] [InlineData(true)] - public virtual async Task From_sql_queryable_stored_procedure_composed(bool async) + public virtual async Task From_sql_queryable_stored_procedure_composed(bool async) { using var context = CreateContext(); - try - { - var query = context - .Set() - .FromSqlRaw(TenMostExpensiveProductsSproc, GetTenMostExpensiveProductsParameters()) - .Where(mep => mep.TenMostExpensiveProducts.Contains("C")) - .OrderBy(mep => mep.UnitPrice); - - var _ = async - ? await query.ToArrayAsync() - : query.ToArray(); - - Assert.True(false); - return null; - } - catch (Exception e) - { - Assert.IsAssignableFrom(e); - return (DbException)e; - } + var query = context + .Set() + .FromSqlRaw(TenMostExpensiveProductsSproc, GetTenMostExpensiveProductsParameters()) + .Where(mep => mep.TenMostExpensiveProducts.Contains("C")) + .OrderBy(mep => mep.UnitPrice); + + Assert.Equal( + RelationalStrings.FromSqlNonComposable, + (async + ? await Assert.ThrowsAsync(() => query.ToArrayAsync()) + : Assert.Throws(() => query.ToArray())).Message); } [ConditionalTheory] @@ -180,29 +163,21 @@ public virtual async Task From_sql_queryable_stored_procedure_composed_on_client [ConditionalTheory] [InlineData(false)] [InlineData(true)] - public virtual async Task From_sql_queryable_stored_procedure_with_parameter_composed(bool async) + public virtual async Task From_sql_queryable_stored_procedure_with_parameter_composed(bool async) { using var context = CreateContext(); - try - { - var query = context - .Set() - .FromSqlRaw(CustomerOrderHistorySproc, GetCustomerOrderHistorySprocParameters()) - .Where(coh => coh.ProductName.Contains("C")) - .OrderBy(coh => coh.Total); - - var _ = async - ? await query.ToArrayAsync() - : query.ToArray(); - - Assert.True(false); - return null; - } - catch (Exception e) - { - Assert.IsAssignableFrom(e); - return (DbException)e; - } + + var query = context + .Set() + .FromSqlRaw(CustomerOrderHistorySproc, GetCustomerOrderHistorySprocParameters()) + .Where(coh => coh.ProductName.Contains("C")) + .OrderBy(coh => coh.Total); + + Assert.Equal( + RelationalStrings.FromSqlNonComposable, + (async + ? await Assert.ThrowsAsync(() => query.ToArrayAsync()) + : Assert.Throws(() => query.ToArray())).Message); } [ConditionalTheory] @@ -230,29 +205,20 @@ public virtual async Task From_sql_queryable_stored_procedure_with_parameter_com [ConditionalTheory] [InlineData(false)] [InlineData(true)] - public virtual async Task From_sql_queryable_stored_procedure_take(bool async) + public virtual async Task From_sql_queryable_stored_procedure_take(bool async) { using var context = CreateContext(); - try - { - var query = context - .Set() - .FromSqlRaw(TenMostExpensiveProductsSproc, GetTenMostExpensiveProductsParameters()) - .OrderByDescending(mep => mep.UnitPrice) - .Take(2); - - var _ = async - ? await query.ToArrayAsync() - : query.ToArray(); - - Assert.True(false); - return null; - } - catch (Exception e) - { - Assert.IsAssignableFrom(e); - return (DbException)e; - } + var query = context + .Set() + .FromSqlRaw(TenMostExpensiveProductsSproc, GetTenMostExpensiveProductsParameters()) + .OrderByDescending(mep => mep.UnitPrice) + .Take(2); + + Assert.Equal( + RelationalStrings.FromSqlNonComposable, + (async + ? await Assert.ThrowsAsync(() => query.ToArrayAsync()) + : Assert.Throws(() => query.ToArray())).Message); } [ConditionalTheory] @@ -280,26 +246,17 @@ public virtual async Task From_sql_queryable_stored_procedure_take_on_client(boo [ConditionalTheory] [InlineData(false)] [InlineData(true)] - public virtual async Task From_sql_queryable_stored_procedure_min(bool async) + public virtual async Task From_sql_queryable_stored_procedure_min(bool async) { using var context = CreateContext(); - try - { - var query = context.Set() - .FromSqlRaw(TenMostExpensiveProductsSproc, GetTenMostExpensiveProductsParameters()); - - var _ = async - ? await query.MinAsync(mep => mep.UnitPrice) - : query.Min(mep => mep.UnitPrice); - - Assert.True(false); - return null; - } - catch (Exception e) - { - Assert.IsAssignableFrom(e); - return (DbException)e; - } + var query = context.Set() + .FromSqlRaw(TenMostExpensiveProductsSproc, GetTenMostExpensiveProductsParameters()); + + Assert.Equal( + RelationalStrings.FromSqlNonComposable, + (async + ? await Assert.ThrowsAsync(() => query.MinAsync(mep => mep.UnitPrice)) + : Assert.Throws(() => query.Min(mep => mep.UnitPrice))).Message); } [ConditionalTheory] @@ -322,33 +279,24 @@ public virtual async Task From_sql_queryable_stored_procedure_min_on_client(bool [ConditionalTheory] [InlineData(false)] [InlineData(true)] - public virtual async Task From_sql_queryable_stored_procedure_with_include_throws(bool async) + public virtual async Task From_sql_queryable_stored_procedure_with_include_throws(bool async) { using var context = CreateContext(); - try - { - var query = context.Set() - .FromSqlRaw("SelectStoredProcedure") - .Include(p => p.OrderDetails); - - var _ = async - ? await query.ToArrayAsync() - : query.ToArray(); - - Assert.True(false); - return null; - } - catch (Exception e) - { - Assert.IsAssignableFrom(e); - return (DbException)e; - } + var query = context.Set() + .FromSqlRaw("SelectStoredProcedure") + .Include(p => p.OrderDetails); + + Assert.Equal( + RelationalStrings.FromSqlNonComposable, + (async + ? await Assert.ThrowsAsync(() => query.ToArrayAsync()) + : Assert.Throws(() => query.ToArray())).Message); } [ConditionalTheory] [InlineData(false)] [InlineData(true)] - public virtual async Task From_sql_queryable_with_multiple_stored_procedures(bool async) + public virtual async Task From_sql_queryable_with_multiple_stored_procedures(bool async) { using var context = CreateContext(); var query = from a in context.Set() @@ -358,20 +306,11 @@ from b in context.Set() where a.TenMostExpensiveProducts == b.TenMostExpensiveProducts select new { a, b }; - try - { - var _ = async - ? await query.ToArrayAsync() - : query.ToArray(); - - Assert.True(false); - return null; - } - catch (Exception e) - { - Assert.IsAssignableFrom(e); - return (DbException)e; - } + Assert.Equal( + RelationalStrings.FromSqlNonComposable, + (async + ? await Assert.ThrowsAsync(() => query.ToArrayAsync()) + : Assert.Throws(() => query.ToArray())).Message); } [ConditionalTheory] @@ -400,7 +339,7 @@ from b in results2 [ConditionalTheory] [InlineData(false)] [InlineData(true)] - public virtual async Task From_sql_queryable_stored_procedure_and_select(bool async) + public virtual async Task From_sql_queryable_stored_procedure_and_select(bool async) { using var context = CreateContext(); var query = from mep in context.Set() @@ -410,20 +349,11 @@ from p in context.Set() where mep.TenMostExpensiveProducts == p.ProductName select new { mep, p }; - try - { - var _ = async - ? await query.ToArrayAsync() - : query.ToArray(); - - Assert.True(false); - return null; - } - catch (Exception e) - { - Assert.IsAssignableFrom(e); - return (DbException)e; - } + Assert.Equal( + RelationalStrings.FromSqlNonComposable, + (async + ? await Assert.ThrowsAsync(() => query.ToArrayAsync()) + : Assert.Throws(() => query.ToArray())).Message); } [ConditionalTheory] @@ -451,7 +381,7 @@ from p in results2 [ConditionalTheory] [InlineData(false)] [InlineData(true)] - public virtual async Task From_sql_queryable_select_and_stored_procedure(bool async) + public virtual async Task From_sql_queryable_select_and_stored_procedure(bool async) { using var context = CreateContext(); var query = from p in context.Set().FromSqlRaw(NormalizeDelimetersInRawString("SELECT * FROM [Products]")) @@ -460,20 +390,11 @@ from mep in context.Set() where mep.TenMostExpensiveProducts == p.ProductName select new { mep, p }; - try - { - var _ = async - ? await query.ToArrayAsync() - : query.ToArray(); - - Assert.True(false); - return null; - } - catch (Exception e) - { - Assert.IsAssignableFrom(e); - return (DbException)e; - } + Assert.Equal( + RelationalStrings.FromSqlNonComposable, + (async + ? await Assert.ThrowsAsync(() => query.ToArrayAsync()) + : Assert.Throws(() => query.ToArray())).Message); } [ConditionalTheory] diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/FromSqlSprocQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/FromSqlSprocQuerySqlServerTest.cs index 8504dbf4681..efb1dc1358b 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/FromSqlSprocQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/FromSqlSprocQuerySqlServerTest.cs @@ -1,12 +1,8 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; -using System.Data.Common; using System.Threading.Tasks; -using Microsoft.Data.SqlClient; using Microsoft.EntityFrameworkCore.TestUtilities; -using Xunit; namespace Microsoft.EntityFrameworkCore.Query { @@ -18,17 +14,6 @@ public FromSqlSprocQuerySqlServerTest(NorthwindQuerySqlServerFixture From_sql_queryable_stored_procedure_with_include_throws(bool async) - => AssertSqlException(await base.From_sql_queryable_stored_procedure_with_include_throws(async)); - - private static DbException AssertSqlException(DbException exception) - { - Assert.IsType(exception); - Assert.Equal(102, ((SqlException)exception).Number); - - return exception; - } - public override async Task From_sql_queryable_stored_procedure(bool async) { await base.From_sql_queryable_stored_procedure(async); @@ -60,9 +45,6 @@ public override async Task From_sql_queryable_stored_procedure_re_projection_on_ AssertSql("[dbo].[Ten Most Expensive Products]"); } - public override async Task From_sql_queryable_stored_procedure_re_projection(bool async) - => AssertSqlException(await base.From_sql_queryable_stored_procedure_re_projection(async)); - public override async Task From_sql_queryable_stored_procedure_composed_on_client(bool async) { await base.From_sql_queryable_stored_procedure_composed_on_client(async); @@ -70,9 +52,6 @@ public override async Task From_sql_queryable_stored_procedure_composed_on_clien AssertSql("[dbo].[Ten Most Expensive Products]"); } - public override async Task From_sql_queryable_stored_procedure_composed(bool async) - => AssertSqlException(await base.From_sql_queryable_stored_procedure_composed(async)); - public override async Task From_sql_queryable_stored_procedure_with_parameter_composed_on_client(bool async) { await base.From_sql_queryable_stored_procedure_with_parameter_composed_on_client(async); @@ -83,9 +62,6 @@ public override async Task From_sql_queryable_stored_procedure_with_parameter_co [dbo].[CustOrderHist] @CustomerID = @p0"); } - public override async Task From_sql_queryable_stored_procedure_with_parameter_composed(bool async) - => AssertSqlException(await base.From_sql_queryable_stored_procedure_with_parameter_composed(async)); - public override async Task From_sql_queryable_stored_procedure_take_on_client(bool async) { await base.From_sql_queryable_stored_procedure_take_on_client(async); @@ -93,9 +69,6 @@ public override async Task From_sql_queryable_stored_procedure_take_on_client(bo AssertSql("[dbo].[Ten Most Expensive Products]"); } - public override async Task From_sql_queryable_stored_procedure_take(bool async) - => AssertSqlException(await base.From_sql_queryable_stored_procedure_take(async)); - public override async Task From_sql_queryable_stored_procedure_min_on_client(bool async) { await base.From_sql_queryable_stored_procedure_min_on_client(async); @@ -103,18 +76,6 @@ public override async Task From_sql_queryable_stored_procedure_min_on_client(boo AssertSql("[dbo].[Ten Most Expensive Products]"); } - public override async Task From_sql_queryable_stored_procedure_min(bool async) - => AssertSqlException(await base.From_sql_queryable_stored_procedure_min(async)); - - public override async Task From_sql_queryable_with_multiple_stored_procedures(bool async) - => AssertSqlException(await base.From_sql_queryable_with_multiple_stored_procedures(async)); - - public override async Task From_sql_queryable_stored_procedure_and_select(bool async) - => AssertSqlException(await base.From_sql_queryable_stored_procedure_and_select(async)); - - public override async Task From_sql_queryable_select_and_stored_procedure(bool async) - => AssertSqlException(await base.From_sql_queryable_select_and_stored_procedure(async)); - private void AssertSql(params string[] expected) => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);