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 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.
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
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
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
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.