From 0db9e7ae060afd67306fd3a540f939edac75ae76 Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Thu, 17 Mar 2022 15:06:02 -0700 Subject: [PATCH] Query: Assign proper type to collection result expression (part 2) This improves the fix made in #27134 In earlier PR we converted `IQueryable` to `IEnumerable` becaused we converted enumerable to queryable during preprocessing phase and types aligned in later phase since we create a list (which does implement `IEnumerable`). Though this implementing behavior doesn't work when returning a single result of enumerable during async which wraps collection type inside Task. The better fix is to assign `List` as type since we are eventually creating a list. In case of single result, the single result operator has generic type which will introduce convert node into it automatically which will match types for async tasks. Resolves #27600 --- .../InternalUsageDiagnosticAnalyzer.cs | 2 +- ...ionalProjectionBindingExpressionVisitor.cs | 21 ++++++-- .../Query/NorthwindSelectQueryCosmosTest.cs | 18 +++++++ .../Query/NorthwindSelectQueryTestBase.cs | 42 +++++++++++++++ .../NorthwindSelectQuerySqlServerTest.cs | 53 +++++++++++++++++++ 5 files changed, 131 insertions(+), 5 deletions(-) diff --git a/src/EFCore.Analyzers/InternalUsageDiagnosticAnalyzer.cs b/src/EFCore.Analyzers/InternalUsageDiagnosticAnalyzer.cs index 58929a77b80..6f21d101110 100644 --- a/src/EFCore.Analyzers/InternalUsageDiagnosticAnalyzer.cs +++ b/src/EFCore.Analyzers/InternalUsageDiagnosticAnalyzer.cs @@ -27,7 +27,7 @@ public const string MessageFormat private static readonly int EFLen = "EntityFrameworkCore".Length; private static readonly DiagnosticDescriptor _descriptor - = new( + = new DiagnosticDescriptor( Id, title: DefaultTitle, messageFormat: MessageFormat, diff --git a/src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs b/src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs index b94535ec835..ab68d52d25d 100644 --- a/src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs @@ -198,14 +198,27 @@ public virtual Expression Translate(SelectExpression selectExpression, Expressio _clientProjections!.Add(subquery); var type = expression.Type; - if (!(AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue27105", out var enabled) && enabled)) + + if (!(AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue27105", out var enabled27105) && enabled27105)) { - if (type.IsGenericType - && type.GetGenericTypeDefinition() == typeof(IQueryable<>)) + if (!(AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue27600", out var enabled27600) && enabled27600)) + { + if (type.IsGenericType + && type.GetGenericTypeDefinition() == typeof(IQueryable<>)) + { + type = typeof(List<>).MakeGenericType(type.GetSequenceType()); + } + } + else { - type = typeof(IEnumerable<>).MakeGenericType(type.GetSequenceType()); + if (type.IsGenericType + && type.GetGenericTypeDefinition() == typeof(IQueryable<>)) + { + type = typeof(IEnumerable<>).MakeGenericType(type.GetSequenceType()); + } } } + var projectionBindingExpression = new ProjectionBindingExpression( _selectExpression, _clientProjections.Count - 1, type); return subquery.ResultCardinality == ResultCardinality.Enumerable diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindSelectQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindSelectQueryCosmosTest.cs index 1a731c66ce9..1b611b755d0 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindSelectQueryCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindSelectQueryCosmosTest.cs @@ -1332,6 +1332,24 @@ public override Task List_of_list_of_anonymous_type(bool async) return base.List_of_list_of_anonymous_type(async); } + [ConditionalTheory(Skip = "Cross collection join Issue#17246")] + public override Task List_from_result_of_single_result(bool async) + { + return base.List_from_result_of_single_result(async); + } + + [ConditionalTheory(Skip = "Cross collection join Issue#17246")] + public override Task List_from_result_of_single_result_2(bool async) + { + return base.List_from_result_of_single_result_2(async); + } + + [ConditionalTheory(Skip = "Cross collection join Issue#17246")] + public override Task List_from_result_of_single_result_3(bool async) + { + return base.List_from_result_of_single_result_3(async); + } + private void AssertSql(params string[] expected) => Fixture.TestSqlLoggerFactory.AssertBaseline(expected); diff --git a/test/EFCore.Specification.Tests/Query/NorthwindSelectQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NorthwindSelectQueryTestBase.cs index 1b0be4d80b0..b46661fd38c 100644 --- a/test/EFCore.Specification.Tests/Query/NorthwindSelectQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/NorthwindSelectQueryTestBase.cs @@ -2472,5 +2472,47 @@ public virtual Task List_of_list_of_anonymous_type(bool async) elementAsserter: (ee, aa) => AssertCollection(ee, aa, elementSorter: i => (i.OrderID, i.ProductID))); }); } + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task List_from_result_of_single_result(bool async) + { + return AssertFirstOrDefault( + async, + ss => ss.Set() + .OrderBy(c => c.CustomerID) + .Select(c => c.Orders.Select(e => e.OrderID)), + asserter: (e, a) => AssertCollection(e, a, elementSorter: e => e, elementAsserter: (ee, aa) => AssertEqual(ee, aa))); + } + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task List_from_result_of_single_result_2(bool async) + { + return AssertFirstOrDefault( + async, + ss => ss.Set() + .OrderBy(c => c.CustomerID) + .Select(c => c.Orders.Select(e => new { e.OrderID, e.OrderDate })), + asserter: (e, a) => AssertCollection(e, a, elementSorter: e => e.OrderID, + elementAsserter: (ee, aa) => + { + AssertEqual(ee.OrderID, aa.OrderID); + AssertEqual(ee.OrderDate, aa.OrderDate); + })); + } + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task List_from_result_of_single_result_3(bool async) + { + return AssertFirstOrDefault( + async, + ss => ss.Set() + .OrderBy(c => c.CustomerID) + .Select(c => c.Orders.OrderBy(o => o.OrderDate) + .Select(e => e.OrderDetails.Select(od => od.ProductID)).FirstOrDefault()), + asserter: (e, a) => AssertCollection(e, a, elementSorter: e => e, elementAsserter: (ee, aa) => AssertEqual(ee, aa))); + } } } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSelectQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSelectQuerySqlServerTest.cs index 2c690b9064e..b492b5565e2 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSelectQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSelectQuerySqlServerTest.cs @@ -1891,6 +1891,59 @@ WHERE [c].[CustomerID] LIKE N'F%' ORDER BY [c].[CustomerID], [t].[OrderID], [t].[OrderID0]"); } + public override async Task List_from_result_of_single_result(bool async) + { + await base.List_from_result_of_single_result(async); + + AssertSql( + @"SELECT [t].[CustomerID], [o].[OrderID] +FROM ( + SELECT TOP(1) [c].[CustomerID] + FROM [Customers] AS [c] + ORDER BY [c].[CustomerID] +) AS [t] +LEFT JOIN [Orders] AS [o] ON [t].[CustomerID] = [o].[CustomerID] +ORDER BY [t].[CustomerID]"); + } + + public override async Task List_from_result_of_single_result_2(bool async) + { + await base.List_from_result_of_single_result_2(async); + + AssertSql( + @"SELECT [t].[CustomerID], [o].[OrderID], [o].[OrderDate] +FROM ( + SELECT TOP(1) [c].[CustomerID] + FROM [Customers] AS [c] + ORDER BY [c].[CustomerID] +) AS [t] +LEFT JOIN [Orders] AS [o] ON [t].[CustomerID] = [o].[CustomerID] +ORDER BY [t].[CustomerID]"); + } + + public override async Task List_from_result_of_single_result_3(bool async) + { + await base.List_from_result_of_single_result_3(async); + + AssertSql( + @"SELECT [t].[CustomerID], [t0].[OrderID], [o0].[ProductID], [o0].[OrderID], [t0].[c] +FROM ( + SELECT TOP(1) [c].[CustomerID] + FROM [Customers] AS [c] + ORDER BY [c].[CustomerID] +) AS [t] +LEFT JOIN ( + SELECT [t1].[c], [t1].[OrderID], [t1].[CustomerID] + FROM ( + SELECT 1 AS [c], [o].[OrderID], [o].[CustomerID], ROW_NUMBER() OVER(PARTITION BY [o].[CustomerID] ORDER BY [o].[OrderDate]) AS [row] + FROM [Orders] AS [o] + ) AS [t1] + WHERE [t1].[row] <= 1 +) AS [t0] ON [t].[CustomerID] = [t0].[CustomerID] +LEFT JOIN [Order Details] AS [o0] ON [t0].[OrderID] = [o0].[OrderID] +ORDER BY [t].[CustomerID], [t0].[OrderID], [o0].[OrderID]"); + } + private void AssertSql(params string[] expected) => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);