Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/3.1] Query: Map projection properly when joining 2 tables #20207

Merged
merged 1 commit into from
Mar 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 23 additions & 6 deletions src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -914,9 +914,11 @@ public Expression ApplyCollectionJoin(
}

var indexOffset = _projection.Count;
foreach (var projection in innerSelectExpression.Projection)
var innerProjectionCount = innerSelectExpression.Projection.Count;
var indexMap = new int[innerProjectionCount];
for (var i = 0; i < innerProjectionCount; i++)
{
AddToProjection(MakeNullable(projection.Expression));
indexMap[i] = AddToProjection(MakeNullable(innerSelectExpression.Projection[i].Expression));
}

foreach (var identifier in innerSelectExpression._identifier.Concat(innerSelectExpression._childIdentifiers))
Expand All @@ -926,7 +928,10 @@ public Expression ApplyCollectionJoin(
AppendOrdering(new OrderingExpression(updatedColumn, ascending: true));
}

var shaperRemapper = new ShaperRemappingExpressionVisitor(this, innerSelectExpression, indexOffset);
var shaperRemapper = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue19616", out var isEnabled)
ajcvickers marked this conversation as resolved.
Show resolved Hide resolved
&& isEnabled
? new ShaperRemappingExpressionVisitor(this, innerSelectExpression, indexOffset)
: new ShaperRemappingExpressionVisitor(this, innerSelectExpression, indexMap);
innerShaper = shaperRemapper.Visit(innerShaper);
selfIdentifier = shaperRemapper.Visit(selfIdentifier);

Expand Down Expand Up @@ -961,21 +966,32 @@ private class ShaperRemappingExpressionVisitor : ExpressionVisitor
private readonly SelectExpression _queryExpression;
private readonly SelectExpression _innerSelectExpression;
private readonly int _offset;
private readonly int[] _indexMap;

public ShaperRemappingExpressionVisitor(SelectExpression queryExpression, SelectExpression innerSelectExpression, int offset)
public ShaperRemappingExpressionVisitor(
SelectExpression queryExpression, SelectExpression innerSelectExpression, int offset)
{
_queryExpression = queryExpression;
_innerSelectExpression = innerSelectExpression;
_offset = offset;
}

public ShaperRemappingExpressionVisitor(
SelectExpression queryExpression, SelectExpression innerSelectExpression, int[] indexMap)
{
_queryExpression = queryExpression;
_innerSelectExpression = innerSelectExpression;
_indexMap = indexMap;
}

protected override Expression VisitExtension(Expression extensionExpression)
{
if (extensionExpression is ProjectionBindingExpression projectionBindingExpression)
{
var oldIndex = (int)GetProjectionIndex(projectionBindingExpression);
var newIndex = _indexMap?[oldIndex] ?? oldIndex + _offset;

return new ProjectionBindingExpression(_queryExpression, oldIndex + _offset, projectionBindingExpression.Type);
return new ProjectionBindingExpression(_queryExpression, newIndex, projectionBindingExpression.Type);
}

if (extensionExpression is EntityShaperExpression entityShaper)
Expand All @@ -985,7 +1001,8 @@ protected override Expression VisitExtension(Expression extensionExpression)
var indexMap = new Dictionary<IProperty, int>();
foreach (var keyValuePair in oldIndexMap)
{
indexMap[keyValuePair.Key] = keyValuePair.Value + _offset;
var oldIndex = keyValuePair.Value;
indexMap[keyValuePair.Key] = _indexMap?[oldIndex] ?? oldIndex + _offset;
}

return new EntityShaperExpression(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -921,5 +921,11 @@ public override Task LastOrDefault_member_access_in_projection_translates_to_ser
{
return base.LastOrDefault_member_access_in_projection_translates_to_server(isAsync);
}

[ConditionalTheory(Skip = "Issue#17246")]
public override Task Projecting_multiple_collection_with_same_constant_works(bool async)
{
return base.Projecting_multiple_collection_with_same_constant_works(async);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1492,5 +1492,25 @@ public CustomerWrapper2(Customer customer)
public string City { get; set; }
public Customer Customer { get; }
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Projecting_multiple_collection_with_same_constant_works(bool async)
{
return AssertQuery(
async,
ss => ss.Set<Customer>().Where(c => c.CustomerID == "ALFKI")
.Select(c => new
{
O1 = c.Orders.Select(e => new { Value = 1 }),
O2 = c.Orders.Select(e => new { AnotherValue = 1 })
}),
assertOrder: true, //single element
elementAsserter: (e, a) =>
{
AssertCollection(e.O1, a.O1, ordered: true);
AssertCollection(e.O2, a.O2, ordered: true);
});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1227,5 +1227,18 @@ FROM [Orders] AS [o]
FROM [Customers] AS [c]
WHERE [c].[CustomerID] LIKE N'A%'");
}

public override async Task Projecting_multiple_collection_with_same_constant_works(bool async)
{
await base.Projecting_multiple_collection_with_same_constant_works(async);

AssertSql(
@"SELECT [c].[CustomerID], 1, [o].[OrderID], [o0].[OrderID]
FROM [Customers] AS [c]
LEFT JOIN [Orders] AS [o] ON [c].[CustomerID] = [o].[CustomerID]
LEFT JOIN [Orders] AS [o0] ON [c].[CustomerID] = [o0].[CustomerID]
WHERE [c].[CustomerID] = N'ALFKI'
ORDER BY [c].[CustomerID], [o].[OrderID], [o0].[OrderID]");
}
}
}