Skip to content

Commit

Permalink
Query: Lift order by collection queries
Browse files Browse the repository at this point in the history
  • Loading branch information
@smitpatel
smitpatel committed Jul 9, 2019
1 parent b5cee2d commit fbf575a
Show file tree
Hide file tree
Showing 5 changed files with 222 additions and 333 deletions.
14 changes: 12 additions & 2 deletions src/EFCore.Relational/Query/Pipeline/SqlExpressions/ColumnExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,21 @@ internal ColumnExpression(IProperty property, TableExpressionBase table, bool nu
{
}

internal ColumnExpression(ProjectionExpression subqueryProjection, TableExpressionBase table, bool nullable)
: this(subqueryProjection.Alias, table, subqueryProjection.Type, subqueryProjection.Expression.TypeMapping, nullable)
internal ColumnExpression(ProjectionExpression subqueryProjection, TableExpressionBase table)
: this(subqueryProjection.Alias, table,
subqueryProjection.Type, subqueryProjection.Expression.TypeMapping,
IsNullableProjection(subqueryProjection))
{
}

private static bool IsNullableProjection(ProjectionExpression projectionExpression)
=> projectionExpression.Expression switch
{
ColumnExpression columnExpression => columnExpression.Nullable,
SqlConstantExpression sqlConstantExpression => sqlConstantExpression.Value == null,
_ => true,
};

private ColumnExpression(string name, TableExpressionBase table, Type type, RelationalTypeMapping typeMapping, bool nullable)
: base(type, typeMapping)
{
Expand Down
83 changes: 40 additions & 43 deletions src/EFCore.Relational/Query/Pipeline/SqlExpressions/SelectExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ public Expression ApplyGrouping(Expression keySelector)
var subquery = (SelectExpression)_tables[0];
var projectionIndex = subquery.AddToProjection((SqlExpression)keySelector, nameof(IGrouping<int, int>.Key));

keySelector = new ColumnExpression(subquery.Projection[projectionIndex], subquery, false);
keySelector = new ColumnExpression(subquery.Projection[projectionIndex], subquery);
}

AppendGroupBy(keySelector);
Expand Down Expand Up @@ -391,6 +391,7 @@ public void ApplyDistinct()
}

IsDistinct = true;

ClearOrdering();
}

Expand Down Expand Up @@ -456,14 +457,14 @@ public Expression ApplySetOperation(
if (joinedMapping.Value1 is EntityProjectionExpression entityProjection1
&& joinedMapping.Value2 is EntityProjectionExpression entityProjection2)
{
HandleEntityMapping(joinedMapping.Key, select1, entityProjection1, select2, entityProjection2);
handleEntityMapping(joinedMapping.Key, select1, entityProjection1, select2, entityProjection2);
continue;
}

if (joinedMapping.Value1 is ColumnExpression && joinedMapping.Value2 is ColumnExpression
|| joinedMapping.Value1 is SubSelectExpression && joinedMapping.Value2 is SubSelectExpression)
{
HandleColumnMapping(
handleColumnMapping(
joinedMapping.Key,
select1, (SqlExpression)joinedMapping.Value1,
select2, (SqlExpression)joinedMapping.Value2);
Expand All @@ -487,7 +488,7 @@ public Expression ApplySetOperation(

return shaperExpression;

void HandleEntityMapping(
void handleEntityMapping(
ProjectionMember projectionMember,
SelectExpression select1, EntityProjectionExpression projection1,
SelectExpression select2, EntityProjectionExpression projection2)
Expand All @@ -498,7 +499,7 @@ void HandleEntityMapping(
{
foreach (var property in GetAllPropertiesInHierarchy(projection1.EntityType))
{
propertyExpressions[property] = AddSetOperationColumnProjections(
propertyExpressions[property] = addSetOperationColumnProjections(
property,
select1, projection1.BindProperty(property),
select2, projection2.BindProperty(property));
Expand All @@ -511,7 +512,7 @@ void HandleEntityMapping(
throw new InvalidOperationException("Set operations over different entity types are currently unsupported (see #16298)");
}

ColumnExpression AddSetOperationColumnProjections(
ColumnExpression addSetOperationColumnProjections(
IProperty property,
SelectExpression select1, ColumnExpression column1,
SelectExpression select2, ColumnExpression column2)
Expand All @@ -529,7 +530,7 @@ ColumnExpression AddSetOperationColumnProjections(
return column1;
}

void HandleColumnMapping(
void handleColumnMapping(
ProjectionMember projectionMember,
SelectExpression select1, SqlExpression innerColumn1,
SelectExpression select2, SqlExpression innerColumn2)
Expand All @@ -545,7 +546,8 @@ void HandleColumnMapping(

public IDictionary<SqlExpression, ColumnExpression> PushdownIntoSubquery()
{
var subquery = new SelectExpression("t", new List<ProjectionExpression>(), _tables.ToList(), _groupBy.ToList(), _orderings.ToList())
var subquery = new SelectExpression(
"t", new List<ProjectionExpression>(), _tables.ToList(), _groupBy.ToList(), _orderings.ToList())
{
IsDistinct = IsDistinct,
Predicate = Predicate,
Expand All @@ -555,9 +557,11 @@ public IDictionary<SqlExpression, ColumnExpression> PushdownIntoSubquery()
SetOperationType = SetOperationType
};

if (subquery.Limit == null && subquery.Offset == null)
ColumnExpression liftProjectionFromSubquery(SqlExpression projection)
{
subquery.ClearOrdering();
var index = subquery.AddToProjection(projection);
var projectionExpression = subquery._projection[index];
return new ColumnExpression(projectionExpression, subquery);
}

var projectionMap = new Dictionary<SqlExpression, ColumnExpression>();
Expand All @@ -567,9 +571,7 @@ public IDictionary<SqlExpression, ColumnExpression> PushdownIntoSubquery()
_projection.Clear();
foreach (var projection in projections)
{
var index = subquery.AddToProjection(projection);
var projectionExpression = subquery._projection[index];
var outerColumn = new ColumnExpression(projectionExpression, subquery, IsNullableProjection(projectionExpression));
var outerColumn = liftProjectionFromSubquery(projection);
AddToProjection(outerColumn);
projectionMap[projection] = outerColumn;
}
Expand All @@ -584,9 +586,7 @@ public IDictionary<SqlExpression, ColumnExpression> PushdownIntoSubquery()
foreach (var property in GetAllPropertiesInHierarchy(entityProjection.EntityType))
{
var innerColumn = entityProjection.BindProperty(property);
var index = subquery.AddToProjection(innerColumn);
var projectionExpression = subquery._projection[index];
var outerColumn = new ColumnExpression(projectionExpression, subquery, IsNullableProjection(projectionExpression));
var outerColumn = liftProjectionFromSubquery(innerColumn);
projectionMap[innerColumn] = outerColumn;
propertyExpressions[property] = outerColumn;
}
Expand All @@ -596,9 +596,7 @@ public IDictionary<SqlExpression, ColumnExpression> PushdownIntoSubquery()
else
{
var innerColumn = (SqlExpression)mapping.Value;
var index = subquery.AddToProjection(innerColumn);
var projectionExpression = subquery._projection[index];
var outerColumn = new ColumnExpression(projectionExpression, subquery, IsNullableProjection(projectionExpression));
var outerColumn = liftProjectionFromSubquery(innerColumn);
projectionMap[innerColumn] = outerColumn;
_projectionMapping[mapping.Key] = outerColumn;
}
Expand All @@ -616,9 +614,7 @@ public IDictionary<SqlExpression, ColumnExpression> PushdownIntoSubquery()
}
else if (!IsDistinct && GroupBy.Count == 0)
{
var index = subquery.AddToProjection(identifier);
var projectionExpression = subquery._projection[index];
outerColumn = new ColumnExpression(projectionExpression, subquery, IsNullableProjection(projectionExpression));
outerColumn = liftProjectionFromSubquery(identifier);
_identifier.Add(outerColumn);
}
}
Expand All @@ -634,9 +630,7 @@ public IDictionary<SqlExpression, ColumnExpression> PushdownIntoSubquery()
}
else if (!IsDistinct && GroupBy.Count == 0)
{
var index = subquery.AddToProjection(identifier);
var projectionExpression = subquery._projection[index];
outerColumn = new ColumnExpression(projectionExpression, subquery, IsNullableProjection(projectionExpression));
outerColumn = liftProjectionFromSubquery(identifier);
_childIdentifiers.Add(outerColumn);
}
}
Expand All @@ -646,16 +640,24 @@ public IDictionary<SqlExpression, ColumnExpression> PushdownIntoSubquery()
_pendingCollections.AddRange(pendingCollections.Select(new SqlRemappingVisitor(projectionMap).Remap));

_orderings.Clear();
foreach (var ordering in subquery._orderings)
// Only lift order by to outer if subquery does not have distinct
if (!subquery.IsDistinct)
{
var orderingExpression = ordering.Expression;
if (!projectionMap.TryGetValue(orderingExpression, out var outerColumn))
foreach (var ordering in subquery._orderings)
{
var index = subquery.AddToProjection(orderingExpression);
var projectionExpression = subquery._projection[index];
outerColumn = new ColumnExpression(projectionExpression, subquery, IsNullableProjection(projectionExpression));
var orderingExpression = ordering.Expression;
if (!projectionMap.TryGetValue(orderingExpression, out var outerColumn))
{
outerColumn = liftProjectionFromSubquery(orderingExpression);
}

_orderings.Add(ordering.Update(outerColumn));
}
_orderings.Add(new OrderingExpression(outerColumn, ordering.Ascending));
}

if (subquery.Offset == null && subquery.Limit == null)
{
subquery.ClearOrdering();
}

Offset = null;
Expand All @@ -671,11 +673,6 @@ public IDictionary<SqlExpression, ColumnExpression> PushdownIntoSubquery()
return projectionMap;
}

private static bool IsNullableProjection(ProjectionExpression projection)
{
return projection.Expression is ColumnExpression column ? column.Nullable : true;
}

public CollectionShaperExpression AddCollectionProjection(
ShapedQueryExpression shapedQueryExpression, INavigation navigation, Type elementType)
{
Expand Down Expand Up @@ -717,18 +714,18 @@ public Expression ApplyCollectionJoin(
|| innerSelectExpression.Tables.Count > 1
|| innerSelectExpression.GroupBy.Count > 1)
{
var orderings = innerSelectExpression.Orderings.ToList();
var sqlRemappingVisitor = new SqlRemappingVisitor(innerSelectExpression.PushdownIntoSubquery());
joinPredicate = sqlRemappingVisitor.Remap(joinPredicate);

foreach (var ordering in orderings)
{
AppendOrdering(ordering.Update(MakeNullable(sqlRemappingVisitor.Remap(ordering.Expression))));
}
}

var leftJoinExpression = new LeftJoinExpression(innerSelectExpression.Tables.Single(), joinPredicate);
_tables.Add(leftJoinExpression);

foreach (var ordering in innerSelectExpression.Orderings)
{
AppendOrdering(ordering.Update(MakeNullable(ordering.Expression)));
}

var indexOffset = _projection.Count;
foreach (var projection in innerSelectExpression.Projection)
{
Expand Down
16 changes: 8 additions & 8 deletions test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4661,7 +4661,7 @@ where ps.SquadId < 7
});
}

[ConditionalTheory(Skip = "Issue#16226")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Correlated_collections_nested_with_custom_ordering(bool isAsync)
{
Expand Down Expand Up @@ -4749,7 +4749,7 @@ orderby g.Rank
});
}

[ConditionalTheory(Skip = "Issue#16226")]
[ConditionalTheory(Skip = "Issue#16318")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Correlated_collections_different_collections_projected(bool isAsync)
{
Expand Down Expand Up @@ -4797,7 +4797,7 @@ where o.Reports.Any()
assertOrder: true);
}

[ConditionalTheory(Skip = "Issue#16226")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Multiple_orderby_with_navigation_expansion_on_one_of_the_order_bys_inside_subquery(bool isAsync)
{
Expand Down Expand Up @@ -4825,7 +4825,7 @@ where o.Reports.Any()
});
}

[ConditionalTheory(Skip = "Issue#16226")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Multiple_orderby_with_navigation_expansion_on_one_of_the_order_bys_inside_subquery_duplicated_orderings(bool isAsync)
{
Expand Down Expand Up @@ -4855,7 +4855,7 @@ where o.Reports.Any()
}


[ConditionalTheory(Skip = "Issue#16226")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Multiple_orderby_with_navigation_expansion_on_one_of_the_order_bys_inside_subquery_complex_orderings(bool isAsync)
{
Expand Down Expand Up @@ -4883,7 +4883,7 @@ where o.Reports.Any()
});
}

[ConditionalTheory(Skip = "Issue#16226")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Correlated_collections_multiple_nested_complex_collections(bool isAsync)
{
Expand Down Expand Up @@ -5284,7 +5284,7 @@ from g in grouping.DefaultIfEmpty()
eee => eee.Id, (eee, aaa) => Assert.Equal(eee.Id, aaa.Id))));
}

[ConditionalTheory(Skip = "Issue#16226")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Correlated_collections_from_left_join_with_additional_elements_projected_of_that_join(bool isAsync)
{
Expand Down Expand Up @@ -5442,7 +5442,7 @@ from r in gs
});
}

[ConditionalTheory(Skip = "Issue#16226")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Correlated_collections_with_funky_orderby_complex_scenario2(bool isAsync)
{
Expand Down
Loading

0 comments on commit fbf575a

Please sign in to comment.