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.

Sign up for GitHub

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

Query: Lift projectionMapping always when pushing down SelectExpressi… #18520

Merged
merged 1 commit into from
Oct 23, 2019
Merged

Query: Lift projectionMapping always when pushing down SelectExpressi… #18520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
99 changes: 52 additions & 47 deletions src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,19 @@ namespace Microsoft.EntityFrameworkCore.Query.SqlExpressions
{
public class SelectExpression : TableExpressionBase
{
private IDictionary<ProjectionMember, Expression> _projectionMapping = new Dictionary<ProjectionMember, Expression>();
private readonly List<ProjectionExpression> _projection = new List<ProjectionExpression>();

private readonly IDictionary<EntityProjectionExpression, IDictionary<IProperty, int>> _entityProjectionCache
= new Dictionary<EntityProjectionExpression, IDictionary<IProperty, int>>();

private readonly List<ProjectionExpression> _projection = new List<ProjectionExpression>();
private readonly List<TableExpressionBase> _tables = new List<TableExpressionBase>();
private readonly List<SqlExpression> _groupBy = new List<SqlExpression>();
private readonly List<OrderingExpression> _orderings = new List<OrderingExpression>();

private readonly List<SqlExpression> _identifier = new List<SqlExpression>();
private readonly List<SqlExpression> _childIdentifiers = new List<SqlExpression>();
private readonly List<SelectExpression> _pendingCollections = new List<SelectExpression>();

private IDictionary<ProjectionMember, Expression> _projectionMapping = new Dictionary<ProjectionMember, Expression>();

public IReadOnlyList<ProjectionExpression> Projection => _projection;
public IReadOnlyList<TableExpressionBase> Tables => _tables;
public IReadOnlyList<SqlExpression> GroupBy => _groupBy;
Expand Down Expand Up @@ -137,7 +136,7 @@ public void ApplyProjection()
_projectionMapping = result;
}

private IEnumerable<IProperty> GetAllPropertiesInHierarchy(IEntityType entityType)
private static IEnumerable<IProperty> GetAllPropertiesInHierarchy(IEntityType entityType)
=> entityType.GetTypesInHierarchy().SelectMany(EntityTypeExtensions.GetDeclaredProperties);

public void ReplaceProjectionMapping(IDictionary<ProjectionMember, Expression> projectionMapping)
Expand Down Expand Up @@ -660,35 +659,7 @@ public IDictionary<SqlExpression, ColumnExpression> PushdownIntoSubquery()

var projectionMap = new Dictionary<SqlExpression, ColumnExpression>();

EntityProjectionExpression liftEntityProjectionFromSubquery(EntityProjectionExpression entityProjection)
{
var propertyExpressions = new Dictionary<IProperty, ColumnExpression>();
foreach (var property in GetAllPropertiesInHierarchy(entityProjection.EntityType))
{
var innerColumn = entityProjection.BindProperty(property);
var outerColumn = subquery.GenerateOuterColumn(innerColumn);
projectionMap[innerColumn] = outerColumn;
propertyExpressions[property] = outerColumn;
}

var newEntityProjection = new EntityProjectionExpression(entityProjection.EntityType, propertyExpressions);
// Also lift nested entity projections
foreach (var navigation in entityProjection.EntityType.GetTypesInHierarchy()
.SelectMany(EntityTypeExtensions.GetDeclaredNavigations))
{
var boundEntityShaperExpression = entityProjection.BindNavigation(navigation);
if (boundEntityShaperExpression != null)
{
var innerEntityProjection = (EntityProjectionExpression)boundEntityShaperExpression.ValueBufferExpression;
var newInnerEntityProjection = liftEntityProjectionFromSubquery(innerEntityProjection);
boundEntityShaperExpression = boundEntityShaperExpression.Update(newInnerEntityProjection);
newEntityProjection.AddNavigationBinding(navigation, boundEntityShaperExpression);
}
}

return newEntityProjection;
}

// Projections may be present if added by lifting SingleResult/Enumerable in projection through join
if (_projection.Any())
{
var projections = _projection.Select(pe => pe.Expression).ToList();
Expand All @@ -700,21 +671,26 @@ EntityProjectionExpression liftEntityProjectionFromSubquery(EntityProjectionExpr
projectionMap[projection] = outerColumn;
}
}
else

foreach (var mapping in _projectionMapping.ToList())
{
foreach (var mapping in _projectionMapping.ToList())
// If projectionMapping's value is ConstantExpression then projection has already been applied
// And captured in _projections above so we don't need to process this.
if (mapping.Value is ConstantExpression)
{
if (mapping.Value is EntityProjectionExpression entityProjection)
{
_projectionMapping[mapping.Key] = liftEntityProjectionFromSubquery(entityProjection);
}
else
{
var innerColumn = (SqlExpression)mapping.Value;
var outerColumn = subquery.GenerateOuterColumn(innerColumn);
projectionMap[innerColumn] = outerColumn;
_projectionMapping[mapping.Key] = outerColumn;
}
break;
}

if (mapping.Value is EntityProjectionExpression entityProjection)
{
_projectionMapping[mapping.Key] = LiftEntityProjectionFromSubquery(entityProjection);
}
else
{
var innerColumn = (SqlExpression)mapping.Value;
var outerColumn = subquery.GenerateOuterColumn(innerColumn);
projectionMap[innerColumn] = outerColumn;
_projectionMapping[mapping.Key] = outerColumn;
}
}

Expand Down Expand Up @@ -788,6 +764,35 @@ EntityProjectionExpression liftEntityProjectionFromSubquery(EntityProjectionExpr
_groupBy.Clear();

return projectionMap;

EntityProjectionExpression LiftEntityProjectionFromSubquery(EntityProjectionExpression entityProjection)
{
var propertyExpressions = new Dictionary<IProperty, ColumnExpression>();
foreach (var property in GetAllPropertiesInHierarchy(entityProjection.EntityType))
{
var innerColumn = entityProjection.BindProperty(property);
var outerColumn = subquery.GenerateOuterColumn(innerColumn);
projectionMap[innerColumn] = outerColumn;
propertyExpressions[property] = outerColumn;
}

var newEntityProjection = new EntityProjectionExpression(entityProjection.EntityType, propertyExpressions);
// Also lift nested entity projections
foreach (var navigation in entityProjection.EntityType.GetTypesInHierarchy()
.SelectMany(EntityTypeExtensions.GetDeclaredNavigations))
{
var boundEntityShaperExpression = entityProjection.BindNavigation(navigation);
if (boundEntityShaperExpression != null)
{
var innerEntityProjection = (EntityProjectionExpression)boundEntityShaperExpression.ValueBufferExpression;
var newInnerEntityProjection = LiftEntityProjectionFromSubquery(innerEntityProjection);
boundEntityShaperExpression = boundEntityShaperExpression.Update(newInnerEntityProjection);
newEntityProjection.AddNavigationBinding(navigation, boundEntityShaperExpression);
}
}

return newEntityProjection;
}
}

public Expression AddSingleProjection(ShapedQueryExpression shapedQueryExpression)
Expand Down
6 changes: 6 additions & 0 deletions test/EFCore.InMemory.FunctionalTests/Query/ComplexNavigationsQueryInMemoryTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,11 @@ public override void Member_pushdown_chain_3_levels_deep_entity()
{
base.Member_pushdown_chain_3_levels_deep_entity();
}

[ConditionalTheory(Skip = "issue #17620")]
public override Task Lift_projection_mapping_when_pushing_down_subquery(bool isAsync)
{
return base.Lift_projection_mapping_when_pushing_down_subquery(isAsync);
}
}
}
48 changes: 36 additions & 12 deletions test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,18 @@
using Microsoft.EntityFrameworkCore.TestUtilities;
using Xunit;

// ReSharper disable ConvertClosureToMethodGroup
// ReSharper disable PossibleUnintendedReferenceComparison
// ReSharper disable ArgumentsStyleLiteral
// ReSharper disable PossibleMultipleEnumeration
// ReSharper disable UnusedVariable
// ReSharper disable EqualExpressionComparison
// ReSharper disable AccessToDisposedClosure
// ReSharper disable StringStartsWithIsCultureSpecific
// ReSharper disable InconsistentNaming
// ReSharper disable MergeConditionalExpression
// ReSharper disable ReplaceWithSingleCallToSingle
// ReSharper disable ReturnValueOfPureMethodIsNotUsed
// ReSharper disable ConvertClosureToMethodGroup
// ReSharper disable PossibleUnintendedReferenceComparison
// ReSharper disable ArgumentsStyleLiteral
// ReSharper disable PossibleMultipleEnumeration
// ReSharper disable UnusedVariable
// ReSharper disable EqualExpressionComparison
// ReSharper disable AccessToDisposedClosure
// ReSharper disable StringStartsWithIsCultureSpecific
// ReSharper disable InconsistentNaming
// ReSharper disable MergeConditionalExpression
// ReSharper disable ReplaceWithSingleCallToSingle
// ReSharper disable ReturnValueOfPureMethodIsNotUsed
// ReSharper disable ConvertToExpressionBodyWhenPossible
#pragma warning disable RCS1155 // Use StringComparison when comparing strings.
namespace Microsoft.EntityFrameworkCore.Query
Expand Down Expand Up @@ -5725,5 +5725,29 @@ public virtual void Union_over_entities_with_different_nullability()

var result = query.ToList();
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Lift_projection_mapping_when_pushing_down_subquery(bool isAsync)
{
return AssertQuery(
isAsync,
ss => ss.Set<Level1>()
.Take(25)
.Select(l1 => new
{
l1.Id,
c1 = l1.OneToMany_Required1.Select(l2 => new { l2.Id }).FirstOrDefault(),
c2 = l1.OneToMany_Required1.Select(l2 => new { l2.Id })
}),
elementSorter: t => t.Id,
elementAsserter: (e, a) =>
{
Assert.Equal(e.Id, a.Id);
// See issue#17287
Assert.Equal(e.c1?.Id ?? 0, a.c1?.Id);
AssertCollection(e.c2, a.c2, elementSorter: i => i.Id, elementAsserter: (ie, ia) => Assert.Equal(ie.Id, ia.Id));
});
}
}
}
6 changes: 6 additions & 0 deletions test/EFCore.Specification.Tests/Query/ComplexNavigationsWeakQueryTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,5 +178,11 @@ public override Task Include_inside_subquery(bool isAsync)
{
return base.Include_inside_subquery(isAsync);
}

[ConditionalTheory(Skip = "Issue#18519")]
public override Task Lift_projection_mapping_when_pushing_down_subquery(bool isAsync)
{
return base.Lift_projection_mapping_when_pushing_down_subquery(isAsync);
}
}
}
24 changes: 24 additions & 0 deletions test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsQuerySqlServerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4375,6 +4375,30 @@ WHERE [l2].[Id] IS NULL
) AS [t]");
}

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

AssertSql(
@"@__p_0='25'

SELECT [t].[Id], [t1].[Id], [l1].[Id]
FROM (
SELECT TOP(@__p_0) [l].[Id], [l].[Date], [l].[Name], [l].[OneToMany_Optional_Self_Inverse1Id], [l].[OneToMany_Required_Self_Inverse1Id], [l].[OneToOne_Optional_Self1Id]
FROM [LevelOne] AS [l]
) AS [t]
LEFT JOIN (
SELECT [t0].[Id], [t0].[OneToMany_Required_Inverse2Id]
FROM (
SELECT [l0].[Id], [l0].[OneToMany_Required_Inverse2Id], ROW_NUMBER() OVER(PARTITION BY [l0].[OneToMany_Required_Inverse2Id] ORDER BY [l0].[Id]) AS [row]
FROM [LevelTwo] AS [l0]
) AS [t0]
WHERE [t0].[row] <= 1
) AS [t1] ON [t].[Id] = [t1].[OneToMany_Required_Inverse2Id]
LEFT JOIN [LevelTwo] AS [l1] ON [t].[Id] = [l1].[OneToMany_Required_Inverse2Id]
ORDER BY [t].[Id], [l1].[Id]");
}

private void AssertSql(params string[] expected)
{
Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
Expand Down