Skip to content

Commit

Permalink
Query: Assign proper type to collection result expression (part 2)
Browse files Browse the repository at this point in the history
This improves the fix made in #27134

In earlier PR we converted `IQueryable<T>` to `IEnumerable<T>` becaused we converted enumerable to queryable during preprocessing phase and types aligned in later phase since we create a list (which does implement `IEnumerable<T>`). 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<T>` 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
  • Loading branch information
smitpatel committed Mar 17, 2022
1 parent 324eb94 commit 0db9e7a
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 5 deletions.
2 changes: 1 addition & 1 deletion src/EFCore.Analyzers/InternalUsageDiagnosticAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Customer>()
.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<Customer>()
.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<Customer>()
.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)));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down

0 comments on commit 0db9e7a

Please sign in to comment.