Skip to content

Commit

Permalink
Query: Lift projections from single result query rather than applying…
Browse files Browse the repository at this point in the history 
… nested

This fixes slight regression which was introduced in #24675
We need to lift projections for single result since if there is a split query on it then we need it joined to top level to get proper parent query to clone
  • Loading branch information
@smitpatel
smitpatel committed Apr 21, 2021
1 parent 9fe0d4b commit 571f73f
Show file tree
Hide file tree
Showing 13 changed files with 120 additions and 166 deletions.
19 changes: 11 additions & 8 deletions src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,26 +128,29 @@ public ProjectionMemberToIndexConvertingExpressionVisitor(

private sealed class ProjectionIndexRemappingExpressionVisitor : ExpressionVisitor
{
private readonly SelectExpression _queryExpression;
private readonly SelectExpression _oldSelectExpression;
private readonly SelectExpression _newSelectExpression;
private readonly int[] _indexMap;

public ProjectionIndexRemappingExpressionVisitor(
SelectExpression queryExpression, int[] indexMap)
SelectExpression oldSelectExpression, SelectExpression newSelectExpression, int[] indexMap)
{
_queryExpression = queryExpression;
_oldSelectExpression = oldSelectExpression;
_newSelectExpression = newSelectExpression;
_indexMap = indexMap;
}

[return: NotNullIfNotNull("expression")]
public override Expression? Visit(Expression? expression)
{
if (expression is ProjectionBindingExpression projectionBindingExpression)
if (expression is ProjectionBindingExpression projectionBindingExpression
&& ReferenceEquals(projectionBindingExpression.QueryExpression, _oldSelectExpression))
{
Check.DebugAssert(projectionBindingExpression.Index != null,
"ProjectionBindingExpression must have index.");

return new ProjectionBindingExpression(
_queryExpression,
_newSelectExpression,
_indexMap[projectionBindingExpression.Index.Value],
projectionBindingExpression.Type);
}
Expand Down Expand Up @@ -541,9 +544,9 @@ public SplitCollectionInfo(

private sealed class ClientProjectionRemappingExpressionVisitor : ExpressionVisitor
{
private readonly object[] _clientProjectionIndexMap;
private readonly List<object> _clientProjectionIndexMap;

public ClientProjectionRemappingExpressionVisitor(object[] clientProjectionIndexMap)
public ClientProjectionRemappingExpressionVisitor(List<object> clientProjectionIndexMap)
{
_clientProjectionIndexMap = clientProjectionIndexMap;
}
Expand All @@ -562,7 +565,7 @@ public ClientProjectionRemappingExpressionVisitor(object[] clientProjectionIndex

if (value is Expression innerShaper)
{
return innerShaper;
return Visit(innerShaper);
}

throw new InvalidCastException();
Expand Down
51 changes: 30 additions & 21 deletions src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -403,20 +403,23 @@ public Expression ApplyProjection(

if (_clientProjections.Count > 0)
{
if (_clientProjections.Any(e => e is ShapedQueryExpression)
&& (Limit != null
EntityShaperNullableMarkingExpressionVisitor? entityShaperNullableMarkingExpressionVisitor = null;
if (_clientProjections.Any(e => e is ShapedQueryExpression))
{
if (Limit != null
|| Offset != null
|| IsDistinct
|| GroupBy.Count > 0))
{
PushdownIntoSubqueryInternal();
|| GroupBy.Count > 0)
{
PushdownIntoSubqueryInternal();
}
entityShaperNullableMarkingExpressionVisitor = new EntityShaperNullableMarkingExpressionVisitor();
}

var projectionCount = _clientProjections.Count;
var newClientProjections = new List<Expression>();
var clientProjectionIndexMap = new object[projectionCount];
var clientProjectionIndexMap = new List<object>();
var remappingRequired = false;
for (var i = 0; i < projectionCount; i++)
for (var i = 0; i < _clientProjections.Count; i++)
{
var value = _clientProjections[i];
switch (value)
Expand All @@ -425,7 +428,7 @@ public Expression ApplyProjection(
{
var result = AddEntityProjection(entityProjection);
newClientProjections.Add(result);
clientProjectionIndexMap[i] = newClientProjections.Count - 1;
clientProjectionIndexMap.Add(newClientProjections.Count - 1);

break;
}
Expand All @@ -434,7 +437,7 @@ public Expression ApplyProjection(
{
var result = Constant(AddToProjection(sqlExpression, _aliasForClientProjections[i]));
newClientProjections.Add(result);
clientProjectionIndexMap[i] = newClientProjections.Count - 1;
clientProjectionIndexMap.Add(newClientProjections.Count - 1);

break;
}
Expand Down Expand Up @@ -485,12 +488,18 @@ public Expression ApplyProjection(
innerShaperExpression);
}

innerShaperExpression = innerSelectExpression.ApplyProjection(
innerShaperExpression, shapedQueryExpression.ResultCardinality, querySplittingBehavior);
AddJoin(JoinType.OuterApply, ref innerSelectExpression);

innerShaperExpression = CopyProjectionToOuter(innerSelectExpression, innerShaperExpression);
clientProjectionIndexMap[i] = innerShaperExpression;
var offset = _clientProjections.Count;
var count = innerSelectExpression._clientProjections.Count;
_clientProjections.AddRange(innerSelectExpression._clientProjections.Select(e => MakeNullable(e, nullable: true)));
_aliasForClientProjections.AddRange(innerSelectExpression._aliasForClientProjections);
innerShaperExpression = new ProjectionIndexRemappingExpressionVisitor(
innerSelectExpression,
this,
Enumerable.Range(offset, count).ToArray())
.Visit(innerShaperExpression);
innerShaperExpression = entityShaperNullableMarkingExpressionVisitor!.Visit(innerShaperExpression);
clientProjectionIndexMap.Add(innerShaperExpression);
remappingRequired = true;
break;

Expand Down Expand Up @@ -651,7 +660,7 @@ static Expression RemoveConvert(Expression expression)
var result = new SplitCollectionInfo(
parentIdentifier, childIdentifier, childIdentifierValueComparers,
innerSelectExpression, innerShaperExpression);
clientProjectionIndexMap[i] = result;
clientProjectionIndexMap.Add(result);
}
else
{
Expand Down Expand Up @@ -746,7 +755,7 @@ static Expression RemoveConvert(Expression expression)
parentIdentifier, outerIdentifier, selfIdentifier,
parentIdentifierValueComparers, outerIdentifierValueComparers, selfIdentifierValueComparers,
innerShaperExpression);
clientProjectionIndexMap[i] = result;
clientProjectionIndexMap.Add(result);
}
remappingRequired = true;

Expand Down Expand Up @@ -835,8 +844,8 @@ Expression CopyProjectionToOuter(SelectExpression innerSelectExpression, Express

innerSelectExpression._clientProjections.Clear();
innerSelectExpression._aliasForClientProjections.Clear();
innerShaperExpression = new ProjectionIndexRemappingExpressionVisitor(this, indexMap).Visit(innerShaperExpression);
innerShaperExpression = new EntityShaperNullableMarkingExpressionVisitor().Visit(innerShaperExpression);
innerShaperExpression = new ProjectionIndexRemappingExpressionVisitor(innerSelectExpression, this, indexMap).Visit(innerShaperExpression);
innerShaperExpression = entityShaperNullableMarkingExpressionVisitor!.Visit(innerShaperExpression);

return innerShaperExpression;
}
Expand Down Expand Up @@ -1784,7 +1793,7 @@ private Expression AddJoin(
innerSelectExpression._clientProjections.Clear();
innerSelectExpression._aliasForClientProjections.Clear();

innerShaper = new ProjectionIndexRemappingExpressionVisitor(this, indexMap).Visit(innerShaper);
innerShaper = new ProjectionIndexRemappingExpressionVisitor(innerSelectExpression, this, indexMap).Visit(innerShaper);
}
else
{
Expand Down Expand Up @@ -1814,7 +1823,7 @@ private Expression AddJoin(
innerSelectExpression._clientProjections.Clear();
innerSelectExpression._aliasForClientProjections.Clear();

innerShaper = new ProjectionIndexRemappingExpressionVisitor(this, indexMap).Visit(innerShaper);
innerShaper = new ProjectionIndexRemappingExpressionVisitor(innerSelectExpression, this, indexMap).Visit(innerShaper);
}
else
{
Expand Down
79 changes: 39 additions & 40 deletions ...EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsCollectionsQuerySqlServerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1091,7 +1091,7 @@ public override async Task Lift_projection_mapping_when_pushing_down_subquery(bo
AssertSql(
@"@__p_0='25'
SELECT [t].[Id], [t0].[Id], [t0].[c], [l1].[Id]
SELECT [t].[Id], [t0].[Id], [l1].[Id], [t0].[c]
FROM (
SELECT TOP(@__p_0) [l].[Id]
FROM [LevelOne] AS [l]
Expand Down Expand Up @@ -1142,49 +1142,47 @@ public override async Task Select_subquery_single_nested_subquery(bool async)
await base.Select_subquery_single_nested_subquery(async);

AssertSql(
@"SELECT [t1].[Id], [t1].[Id0], [t1].[c]
@"SELECT [l].[Id], [t0].[Id], [t1].[Id], [t0].[c]
FROM [LevelOne] AS [l]
OUTER APPLY (
SELECT [t].[Id], [t0].[Id] AS [Id0], [t].[c]
LEFT JOIN (
SELECT [t].[c], [t].[Id], [t].[OneToMany_Optional_Inverse2Id]
FROM (
SELECT TOP(1) 1 AS [c], [l0].[Id]
SELECT 1 AS [c], [l0].[Id], [l0].[OneToMany_Optional_Inverse2Id], ROW_NUMBER() OVER(PARTITION BY [l0].[OneToMany_Optional_Inverse2Id] ORDER BY [l0].[Id]) AS [row]
FROM [LevelTwo] AS [l0]
WHERE [l].[Id] = [l0].[OneToMany_Optional_Inverse2Id]
ORDER BY [l0].[Id]
) AS [t]
LEFT JOIN (
SELECT [l1].[Id], [l1].[OneToMany_Optional_Inverse3Id]
FROM [LevelThree] AS [l1]
) AS [t0] ON [t].[Id] = [t0].[OneToMany_Optional_Inverse3Id]
) AS [t1]
ORDER BY [l].[Id]");
WHERE [t].[row] <= 1
) AS [t0] ON [l].[Id] = [t0].[OneToMany_Optional_Inverse2Id]
LEFT JOIN (
SELECT [l1].[Id], [l1].[OneToMany_Optional_Inverse3Id]
FROM [LevelThree] AS [l1]
) AS [t1] ON [t0].[Id] = [t1].[OneToMany_Optional_Inverse3Id]
ORDER BY [l].[Id], [t0].[Id], [t1].[Id]");
}

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

AssertSql(
@"SELECT [l].[Id], [t2].[Id], [t2].[Id0], [t2].[c], [t2].[Id1]
@"SELECT [l].[Id], [t2].[Id], [t2].[Id0], [t2].[Id1], [t2].[c]
FROM [LevelOne] AS [l]
LEFT JOIN (
SELECT [t1].[Id], [t1].[Id0], [t1].[c], [l0].[Id] AS [Id1], [l0].[OneToMany_Optional_Inverse2Id]
SELECT [l0].[Id], [t0].[Id] AS [Id0], [t1].[Id] AS [Id1], [t0].[c], [l0].[OneToMany_Optional_Inverse2Id]
FROM [LevelTwo] AS [l0]
OUTER APPLY (
SELECT [t].[Id], [t0].[Id] AS [Id0], [t].[c]
LEFT JOIN (
SELECT [t].[c], [t].[Id], [t].[OneToMany_Optional_Inverse3Id]
FROM (
SELECT TOP(1) 1 AS [c], [l1].[Id]
SELECT 1 AS [c], [l1].[Id], [l1].[OneToMany_Optional_Inverse3Id], ROW_NUMBER() OVER(PARTITION BY [l1].[OneToMany_Optional_Inverse3Id] ORDER BY [l1].[Id]) AS [row]
FROM [LevelThree] AS [l1]
WHERE [l0].[Id] = [l1].[OneToMany_Optional_Inverse3Id]
ORDER BY [l1].[Id]
) AS [t]
LEFT JOIN (
SELECT [l2].[Id], [l2].[OneToMany_Optional_Inverse4Id]
FROM [LevelFour] AS [l2]
) AS [t0] ON [t].[Id] = [t0].[OneToMany_Optional_Inverse4Id]
) AS [t1]
WHERE [t].[row] <= 1
) AS [t0] ON [l0].[Id] = [t0].[OneToMany_Optional_Inverse3Id]
LEFT JOIN (
SELECT [l2].[Id], [l2].[OneToMany_Optional_Inverse4Id]
FROM [LevelFour] AS [l2]
) AS [t1] ON [t0].[Id] = [t1].[OneToMany_Optional_Inverse4Id]
) AS [t2] ON [l].[Id] = [t2].[OneToMany_Optional_Inverse2Id]
ORDER BY [l].[Id], [t2].[Id1], [t2].[Id], [t2].[Id0]");
ORDER BY [l].[Id], [t2].[Id], [t2].[Id0], [t2].[Id1]");
}

public override async Task Filtered_include_basic_Where(bool async)
Expand Down Expand Up @@ -1710,25 +1708,26 @@ public override async Task Complex_query_with_let_collection_projection_FirstOrD
await base.Complex_query_with_let_collection_projection_FirstOrDefault(async);

AssertSql(
@"SELECT [t1].[Id], [t1].[Name], [t1].[Id0], [t1].[c]
@"SELECT [l].[Id], [t0].[Id], [t1].[Name], [t1].[Id], [t0].[c]
FROM [LevelOne] AS [l]
OUTER APPLY (
SELECT [t].[Id], [t0].[Name], [t0].[Id] AS [Id0], [t].[c]
LEFT JOIN (
SELECT [t].[c], [t].[Id], [t].[OneToMany_Optional_Inverse2Id]
FROM (
SELECT TOP(1) 1 AS [c], [l0].[Id]
SELECT 1 AS [c], [l0].[Id], [l0].[OneToMany_Optional_Inverse2Id], ROW_NUMBER() OVER(PARTITION BY [l0].[OneToMany_Optional_Inverse2Id] ORDER BY [l0].[Id]) AS [row]
FROM [LevelTwo] AS [l0]
WHERE ([l].[Id] = [l0].[OneToMany_Optional_Inverse2Id]) AND (([l0].[Name] <> N'Foo') OR [l0].[Name] IS NULL)
WHERE ([l0].[Name] <> N'Foo') OR [l0].[Name] IS NULL
) AS [t]
OUTER APPLY (
SELECT [l1].[Name], [l1].[Id]
FROM [LevelOne] AS [l1]
WHERE EXISTS (
SELECT 1
FROM [LevelTwo] AS [l2]
WHERE ([l1].[Id] = [l2].[OneToMany_Optional_Inverse2Id]) AND ([l2].[Id] = [t].[Id]))
) AS [t0]
WHERE [t].[row] <= 1
) AS [t0] ON [l].[Id] = [t0].[OneToMany_Optional_Inverse2Id]
OUTER APPLY (
SELECT [l1].[Name], [l1].[Id]
FROM [LevelOne] AS [l1]
WHERE EXISTS (
SELECT 1
FROM [LevelTwo] AS [l2]
WHERE ([l1].[Id] = [l2].[OneToMany_Optional_Inverse2Id]) AND ([l2].[Id] = [t0].[Id]))
) AS [t1]
ORDER BY [l].[Id]");
ORDER BY [l].[Id], [t0].[Id], [t1].[Id]");
}

public override async Task SelectMany_DefaultIfEmpty_multiple_times_with_joins_projecting_a_collection(bool async)
Expand Down
44 changes: 21 additions & 23 deletions test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsQuerySqlServerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3108,34 +3108,32 @@ public override void Member_pushdown_chain_3_levels_deep_entity()
base.Member_pushdown_chain_3_levels_deep_entity();

AssertSql(
@"SELECT [t4].[Id], [t4].[Level3_Optional_Id], [t4].[Level3_Required_Id], [t4].[Name], [t4].[OneToMany_Optional_Inverse4Id], [t4].[OneToMany_Optional_Self_Inverse4Id], [t4].[OneToMany_Required_Inverse4Id], [t4].[OneToMany_Required_Self_Inverse4Id], [t4].[OneToOne_Optional_PK_Inverse4Id], [t4].[OneToOne_Optional_Self4Id], [t4].[c], [t4].[c0]
@"SELECT [t0].[c], [t1].[c], [t3].[Id], [t3].[Level3_Optional_Id], [t3].[Level3_Required_Id], [t3].[Name], [t3].[OneToMany_Optional_Inverse4Id], [t3].[OneToMany_Optional_Self_Inverse4Id], [t3].[OneToMany_Required_Inverse4Id], [t3].[OneToMany_Required_Self_Inverse4Id], [t3].[OneToOne_Optional_PK_Inverse4Id], [t3].[OneToOne_Optional_Self4Id]
FROM [LevelOne] AS [l]
OUTER APPLY (
SELECT [t2].[Id], [t2].[Level3_Optional_Id], [t2].[Level3_Required_Id], [t2].[Name], [t2].[OneToMany_Optional_Inverse4Id], [t2].[OneToMany_Optional_Self_Inverse4Id], [t2].[OneToMany_Required_Inverse4Id], [t2].[OneToMany_Required_Self_Inverse4Id], [t2].[OneToOne_Optional_PK_Inverse4Id], [t2].[OneToOne_Optional_Self4Id], [t2].[c], [t].[c] AS [c0]
LEFT JOIN (
SELECT [t].[c], [t].[Id], [t].[Level1_Optional_Id]
FROM (
SELECT TOP(1) 1 AS [c], [l0].[Id]
SELECT 1 AS [c], [l0].[Id], [l0].[Level1_Optional_Id], ROW_NUMBER() OVER(PARTITION BY [l0].[Level1_Optional_Id] ORDER BY [l0].[Id]) AS [row]
FROM [LevelTwo] AS [l0]
WHERE [l0].[Level1_Optional_Id] = [l].[Id]
ORDER BY [l0].[Id]
) AS [t]
OUTER APPLY (
SELECT [t1].[Id], [t1].[Level3_Optional_Id], [t1].[Level3_Required_Id], [t1].[Name], [t1].[OneToMany_Optional_Inverse4Id], [t1].[OneToMany_Optional_Self_Inverse4Id], [t1].[OneToMany_Required_Inverse4Id], [t1].[OneToMany_Required_Self_Inverse4Id], [t1].[OneToOne_Optional_PK_Inverse4Id], [t1].[OneToOne_Optional_Self4Id], [t0].[c]
FROM (
SELECT TOP(1) 1 AS [c], [l1].[Id]
FROM [LevelThree] AS [l1]
WHERE [l1].[Level2_Required_Id] = [t].[Id]
ORDER BY [l1].[Id]
) AS [t0]
LEFT JOIN (
SELECT [t3].[Id], [t3].[Level3_Optional_Id], [t3].[Level3_Required_Id], [t3].[Name], [t3].[OneToMany_Optional_Inverse4Id], [t3].[OneToMany_Optional_Self_Inverse4Id], [t3].[OneToMany_Required_Inverse4Id], [t3].[OneToMany_Required_Self_Inverse4Id], [t3].[OneToOne_Optional_PK_Inverse4Id], [t3].[OneToOne_Optional_Self4Id]
FROM (
SELECT [l2].[Id], [l2].[Level3_Optional_Id], [l2].[Level3_Required_Id], [l2].[Name], [l2].[OneToMany_Optional_Inverse4Id], [l2].[OneToMany_Optional_Self_Inverse4Id], [l2].[OneToMany_Required_Inverse4Id], [l2].[OneToMany_Required_Self_Inverse4Id], [l2].[OneToOne_Optional_PK_Inverse4Id], [l2].[OneToOne_Optional_Self4Id], ROW_NUMBER() OVER(PARTITION BY [l2].[Level3_Required_Id] ORDER BY [l2].[Id]) AS [row]
FROM [LevelFour] AS [l2]
) AS [t3]
WHERE [t3].[row] <= 1
) AS [t1] ON [t0].[Id] = [t1].[Level3_Required_Id]
WHERE [t].[row] <= 1
) AS [t0] ON [l].[Id] = [t0].[Level1_Optional_Id]
LEFT JOIN (
SELECT [t2].[c], [t2].[Id], [t2].[Level2_Required_Id]
FROM (
SELECT 1 AS [c], [l1].[Id], [l1].[Level2_Required_Id], ROW_NUMBER() OVER(PARTITION BY [l1].[Level2_Required_Id] ORDER BY [l1].[Id]) AS [row]
FROM [LevelThree] AS [l1]
) AS [t2]
) AS [t4]
WHERE [t2].[row] <= 1
) AS [t1] ON [t0].[Id] = [t1].[Level2_Required_Id]
LEFT JOIN (
SELECT [t4].[Id], [t4].[Level3_Optional_Id], [t4].[Level3_Required_Id], [t4].[Name], [t4].[OneToMany_Optional_Inverse4Id], [t4].[OneToMany_Optional_Self_Inverse4Id], [t4].[OneToMany_Required_Inverse4Id], [t4].[OneToMany_Required_Self_Inverse4Id], [t4].[OneToOne_Optional_PK_Inverse4Id], [t4].[OneToOne_Optional_Self4Id]
FROM (
SELECT [l2].[Id], [l2].[Level3_Optional_Id], [l2].[Level3_Required_Id], [l2].[Name], [l2].[OneToMany_Optional_Inverse4Id], [l2].[OneToMany_Optional_Self_Inverse4Id], [l2].[OneToMany_Required_Inverse4Id], [l2].[OneToMany_Required_Self_Inverse4Id], [l2].[OneToOne_Optional_PK_Inverse4Id], [l2].[OneToOne_Optional_Self4Id], ROW_NUMBER() OVER(PARTITION BY [l2].[Level3_Required_Id] ORDER BY [l2].[Id]) AS [row]
FROM [LevelFour] AS [l2]
) AS [t4]
WHERE [t4].[row] <= 1
) AS [t3] ON [t1].[Id] = [t3].[Level3_Required_Id]
ORDER BY [l].[Id]");
}

Expand Down
Loading

0 comments on commit 571f73f

Please sign in to comment.