Skip to content

Commit

Permalink
Query: Generate single SQL for queries projecting out non scalar sing…
Browse files Browse the repository at this point in the history 
…le result

When projection contains single result coming out of collection which is not a mappable scalar then we cause client eval and add outer apply to generate the result.
Also tracking for such elements is fixed in new pipeline
Resolves #10001
Resolves #11762
  • Loading branch information
@smitpatel
smitpatel committed Aug 20, 2019
1 parent ba5c3f7 commit 3554db6
Show file tree
Hide file tree
Showing 14 changed files with 311 additions and 230 deletions.
23 changes: 23 additions & 0 deletions src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,29 @@ public override Expression Visit(Expression expression)
{
return _selectExpression.AddCollectionProjection(subquery, null, subquery.ShaperExpression.Type);
}

static bool IsAggregateResultWithCustomShaper(MethodInfo method)
{
if (method.IsGenericMethod)
{
method = method.GetGenericMethodDefinition();
}

return QueryableMethods.IsAverageWithoutSelector(method)
|| QueryableMethods.IsAverageWithSelector(method)
|| method == QueryableMethods.MaxWithoutSelector
|| method == QueryableMethods.MaxWithSelector
|| method == QueryableMethods.MinWithoutSelector
|| method == QueryableMethods.MinWithSelector
|| QueryableMethods.IsSumWithoutSelector(method)
|| QueryableMethods.IsSumWithSelector(method);
}

if (!(subquery.ShaperExpression is ProjectionBindingExpression
|| IsAggregateResultWithCustomShaper(methodCallExpression.Method)))
{
return _selectExpression.AddSingleProjection(subquery);
}
}

break;
Expand Down
39 changes: 15 additions & 24 deletions src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1172,43 +1172,34 @@ private ShapedQueryExpression AggregateResultShaper(

Expression shaper = new ProjectionBindingExpression(source.QueryExpression, new ProjectionMember(), projection.Type);

if (throwOnNullResult
&& resultType.IsNullableType())
if (throwOnNullResult)
{
var resultVariable = Expression.Variable(projection.Type, "result");
var returnValueForNull = resultType.IsNullableType()
? (Expression)Expression.Constant(null, resultType)
: Expression.Throw(
Expression.New(
typeof(InvalidOperationException).GetConstructors()
.Single(ci => ci.GetParameters().Length == 1),
Expression.Constant(CoreStrings.NoElements)),
resultType);

shaper = Expression.Block(
new[] { resultVariable },
Expression.Assign(resultVariable, shaper),
Expression.Condition(
Expression.Equal(resultVariable, Expression.Default(projection.Type)),
Expression.Constant(null, resultType),
returnValueForNull,
resultType != resultVariable.Type
? Expression.Convert(resultVariable, resultType)
: (Expression)resultVariable));
}
else if (throwOnNullResult)
{
var resultVariable = Expression.Variable(projection.Type, "result");

shaper = Expression.Block(
new[] { resultVariable },
Expression.Assign(resultVariable, shaper),
Expression.Condition(
Expression.Equal(resultVariable, Expression.Default(projection.Type)),
Expression.Throw(
Expression.New(
typeof(InvalidOperationException).GetConstructors()
.Single(ci => ci.GetParameters().Length == 1),
Expression.Constant(CoreStrings.NoElements)),
resultType),
resultType != resultVariable.Type
? Expression.Convert(resultVariable, resultType)
: (Expression)resultVariable));
}
else if (resultType.IsNullableType())
else
{
shaper = Expression.Convert(shaper, resultType);
if (resultType.IsNullableType())
{
shaper = Expression.Convert(shaper, resultType);
}
}

source.ShaperExpression = shaper;
Expand Down
20 changes: 19 additions & 1 deletion src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,23 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
var subqueryTranslation = _queryableMethodTranslatingExpressionVisitor.TranslateSubquery(methodCallExpression);
if (subqueryTranslation != null)
{
static bool IsAggregateResultWithCustomShaper(MethodInfo method)
{
if (method.IsGenericMethod)
{
method = method.GetGenericMethodDefinition();
}

return QueryableMethods.IsAverageWithoutSelector(method)
|| QueryableMethods.IsAverageWithSelector(method)
|| method == QueryableMethods.MaxWithoutSelector
|| method == QueryableMethods.MaxWithSelector
|| method == QueryableMethods.MinWithoutSelector
|| method == QueryableMethods.MinWithSelector
|| QueryableMethods.IsSumWithoutSelector(method)
|| QueryableMethods.IsSumWithSelector(method);
}

if (subqueryTranslation.ResultCardinality == ResultCardinality.Enumerable)
{
return null;
Expand All @@ -324,7 +341,8 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
var subquery = (SelectExpression)subqueryTranslation.QueryExpression;
subquery.ApplyProjection();

if (subquery.Projection.Count != 1)
if (!(subqueryTranslation.ShaperExpression is ProjectionBindingExpression
|| IsAggregateResultWithCustomShaper(methodCallExpression.Method)))
{
return null;
}
Expand Down
53 changes: 38 additions & 15 deletions src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,12 @@ ColumnExpression addSetOperationColumnProjections(
}
}

private ColumnExpression GenerateOuterColumn(SqlExpression projection)
{
var index = AddToProjection(projection);
return new ColumnExpression(_projection[index], this);
}

public IDictionary<SqlExpression, ColumnExpression> PushdownIntoSubquery()
{
var subquery = new SelectExpression(
Expand All @@ -574,20 +580,13 @@ public IDictionary<SqlExpression, ColumnExpression> PushdownIntoSubquery()

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

ColumnExpression liftProjectionFromSubquery(SqlExpression projection)
{
var index = subquery.AddToProjection(projection);
var projectionExpression = subquery._projection[index];
return new ColumnExpression(projectionExpression, subquery);
}

EntityProjectionExpression liftEntityProjectionFromSubquery(EntityProjectionExpression entityProjection)
{
var propertyExpressions = new Dictionary<IProperty, ColumnExpression>();
foreach (var property in GetAllPropertiesInHierarchy(entityProjection.EntityType))
{
var innerColumn = entityProjection.BindProperty(property);
var outerColumn = liftProjectionFromSubquery(innerColumn);
var outerColumn = subquery.GenerateOuterColumn(innerColumn);
projectionMap[innerColumn] = outerColumn;
propertyExpressions[property] = outerColumn;
}
Expand Down Expand Up @@ -616,7 +615,7 @@ EntityProjectionExpression liftEntityProjectionFromSubquery(EntityProjectionExpr
_projection.Clear();
foreach (var projection in projections)
{
var outerColumn = liftProjectionFromSubquery(projection);
var outerColumn = subquery.GenerateOuterColumn(projection);
AddToProjection(outerColumn);
projectionMap[projection] = outerColumn;
}
Expand All @@ -632,7 +631,7 @@ EntityProjectionExpression liftEntityProjectionFromSubquery(EntityProjectionExpr
else
{
var innerColumn = (SqlExpression)mapping.Value;
var outerColumn = liftProjectionFromSubquery(innerColumn);
var outerColumn = subquery.GenerateOuterColumn(innerColumn);
projectionMap[innerColumn] = outerColumn;
_projectionMapping[mapping.Key] = outerColumn;
}
Expand All @@ -650,7 +649,7 @@ EntityProjectionExpression liftEntityProjectionFromSubquery(EntityProjectionExpr
}
else if (!IsDistinct && GroupBy.Count == 0)
{
outerColumn = liftProjectionFromSubquery(identifier);
outerColumn = subquery.GenerateOuterColumn(identifier);
_identifier.Add(outerColumn);
}
}
Expand All @@ -666,7 +665,7 @@ EntityProjectionExpression liftEntityProjectionFromSubquery(EntityProjectionExpr
}
else if (!IsDistinct && GroupBy.Count == 0)
{
outerColumn = liftProjectionFromSubquery(identifier);
outerColumn = subquery.GenerateOuterColumn(identifier);
_childIdentifiers.Add(outerColumn);
}
}
Expand All @@ -684,7 +683,7 @@ EntityProjectionExpression liftEntityProjectionFromSubquery(EntityProjectionExpr
var orderingExpression = ordering.Expression;
if (!projectionMap.TryGetValue(orderingExpression, out var outerColumn))
{
outerColumn = liftProjectionFromSubquery(orderingExpression);
outerColumn = subquery.GenerateOuterColumn(orderingExpression);
}

_orderings.Add(ordering.Update(outerColumn));
Expand All @@ -709,6 +708,29 @@ EntityProjectionExpression liftEntityProjectionFromSubquery(EntityProjectionExpr
return projectionMap;
}

public Expression AddSingleProjection(ShapedQueryExpression shapedQueryExpression)
{
var innerSelectExpression = (SelectExpression)shapedQueryExpression.QueryExpression;
innerSelectExpression.ApplyProjection();
var projectionCount = innerSelectExpression.Projection.Count;
AddLeftJoinLateral(innerSelectExpression, null);

// Joined SelectExpression may different based on left join or outer apply
// And it will always be SelectExpression because of presence of Take(1)
// So we need to remap projections from that SelectExpression to outer SelectExpression
var addedSelectExperssion = (SelectExpression)((JoinExpressionBase)_tables[_tables.Count - 1]).Table;
var indexOffset = _projection.Count;
// We only take projectionCount since the subquery can have additional projections for identifiers
// Which are not relevant for this translation
foreach (var projection in addedSelectExperssion.Projection.Take(projectionCount))
{
AddToProjection(MakeNullable(addedSelectExperssion.GenerateOuterColumn(projection.Expression)));
}

return new ShaperRemappingExpressionVisitor(this, innerSelectExpression, indexOffset)
.Visit(shapedQueryExpression.ShaperExpression);
}

public CollectionShaperExpression AddCollectionProjection(
ShapedQueryExpression shapedQueryExpression, INavigation navigation, Type elementType)
{
Expand Down Expand Up @@ -1026,8 +1048,9 @@ private void AddJoin(
// Verify what are the cases of pushdown for inner & outer both sides
if (Limit != null || Offset != null || IsDistinct || IsSetOperation || GroupBy.Count > 1)
{
joinPredicate = new SqlRemappingVisitor(PushdownIntoSubquery(), (SelectExpression)Tables[0])
.Remap(joinPredicate);
var sqlRemappingVisitor = new SqlRemappingVisitor(PushdownIntoSubquery(), (SelectExpression)Tables[0]);
innerSelectExpression = sqlRemappingVisitor.Remap(innerSelectExpression);
joinPredicate = sqlRemappingVisitor.Remap(joinPredicate);
}

if (innerSelectExpression.Orderings.Any()
Expand Down
1 change: 1 addition & 0 deletions test/EFCore.Cosmos.FunctionalTests/Query/SimpleQueryCosmosTest.Select.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,7 @@ FROM root c
WHERE ((c[""Discriminator""] = ""Order"") AND (c[""OrderID""] < 10250))");
}

[ConditionalTheory(Skip = "Issue #17246")]
public override async Task Project_single_element_from_collection_with_OrderBy_over_navigation_Take_and_FirstOrDefault_2(
bool isAsync)
{
Expand Down
42 changes: 42 additions & 0 deletions test/EFCore.InMemory.FunctionalTests/Query/QueryNavigationsInMemoryTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,47 @@ public override void Collection_where_nav_prop_all_client()

[ConditionalTheory(Skip = "Issue #16963")]
public override Task Select_collection_navigation_multi_part2(bool isAsync) => null;

[ConditionalTheory(Skip = "Issue #16963")]
public override Task Collection_select_nav_prop_first_or_default(bool isAsync)
{
return base.Collection_select_nav_prop_first_or_default(isAsync);
}

[ConditionalTheory(Skip = "Issue #16963")]
public override Task Collection_select_nav_prop_first_or_default_then_nav_prop(bool isAsync)
{
return base.Collection_select_nav_prop_first_or_default_then_nav_prop(isAsync);
}

[ConditionalTheory(Skip = "Issue #16963")]
public override Task Project_single_entity_value_subquery_works(bool isAsync)
{
return base.Project_single_entity_value_subquery_works(isAsync);
}

[ConditionalTheory(Skip = "Issue #16963")]
public override Task Select_collection_FirstOrDefault_project_anonymous_type(bool isAsync)
{
return base.Select_collection_FirstOrDefault_project_anonymous_type(isAsync);
}

[ConditionalTheory(Skip = "Issue #16963")]
public override Task Select_collection_FirstOrDefault_project_entity(bool isAsync)
{
return base.Select_collection_FirstOrDefault_project_entity(isAsync);
}

[ConditionalTheory(Skip = "Issue #16963")]
public override Task Skip_Select_Navigation(bool isAsync)
{
return base.Skip_Select_Navigation(isAsync);
}

[ConditionalTheory(Skip = "Issue #16963")]
public override Task Take_Select_Navigation(bool isAsync)
{
return base.Take_Select_Navigation(isAsync);
}
}
}
6 changes: 6 additions & 0 deletions test/EFCore.InMemory.FunctionalTests/Query/SimpleQueryInMemoryTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,5 +381,11 @@ public override Task Projection_when_client_evald_subquery(bool isAsync)

[ConditionalTheory(Skip = "Issue #16963")]
public override Task Select_bool_closure_with_order_by_property_with_cast_to_nullable(bool isAsync) => null;

[ConditionalTheory(Skip = "Issue #16963")]
public override Task Project_single_element_from_collection_with_multiple_OrderBys_Take_and_FirstOrDefault_2(bool isAsync)
{
return base.Project_single_element_from_collection_with_multiple_OrderBys_Take_and_FirstOrDefault_2(isAsync);
}
}
}
4 changes: 2 additions & 2 deletions test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6324,7 +6324,7 @@ public virtual Task Select_subquery_projecting_single_constant_bool(bool isAsync
}));
}

[ConditionalTheory(Skip = "Issue#10001")]
[ConditionalTheory(Skip = "Issue#17287")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Select_subquery_projecting_single_constant_inside_anonymous(bool isAsync)
{
Expand All @@ -6342,7 +6342,7 @@ public virtual Task Select_subquery_projecting_single_constant_inside_anonymous(
}));
}

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

0 comments on commit 3554db6

Please sign in to comment.