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

Jump to bottom

Query: Wrap up collection materialization code path for relational #16340

Merged
merged 1 commit into from
Jun 28, 2019
Merged

Query: Wrap up collection materialization code path for relational #16340

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
12 changes: 7 additions & 5 deletions src/EFCore.Relational/Query/Pipeline/IncludeCompilingExpressionVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,16 +81,17 @@ private static readonly MethodInfo _populateCollectionMethodInfo
= typeof(CustomShaperCompilingExpressionVisitor).GetTypeInfo()
.GetDeclaredMethod(nameof(PopulateCollection));

private static void PopulateCollection<TCollection, TIncludedEntity>(
private static void PopulateCollection<TCollection, TElement, TRelatedEntity>(
int collectionId,
QueryContext queryContext,
DbDataReader dbDataReader,
ResultCoordinator resultCoordinator,
Func<QueryContext, DbDataReader, object[]> parentIdentifier,
Func<QueryContext, DbDataReader, object[]> outerIdentifier,
Func<QueryContext, DbDataReader, object[]> selfIdentifier,
Func<QueryContext, DbDataReader, TIncludedEntity, ResultCoordinator, TIncludedEntity> innerShaper)
where TCollection : class, ICollection<TIncludedEntity>
Func<QueryContext, DbDataReader, TRelatedEntity, ResultCoordinator, TRelatedEntity> innerShaper)
where TRelatedEntity : TElement
where TCollection : class, ICollection<TElement>
{
var collectionMaterializationContext = resultCoordinator.Collections[collectionId];

Expand Down Expand Up @@ -127,7 +128,7 @@ private static void PopulateCollection<TCollection, TIncludedEntity>(
innerKey, collectionMaterializationContext.SelfIdentifier))
{
// We don't need to materialize this entity but we may need to populate inner collections if any.
innerShaper(queryContext, dbDataReader, (TIncludedEntity)collectionMaterializationContext.Current, resultCoordinator);
innerShaper(queryContext, dbDataReader, (TRelatedEntity)collectionMaterializationContext.Current, resultCoordinator);

return;
}
Expand Down Expand Up @@ -410,9 +411,10 @@ protected override Expression VisitExtension(Expression extensionExpression)
}

var collectionType = collectionPopulatingExpression.Type;
var elementType = collectionType.TryGetSequenceType();

return Expression.Call(
_populateCollectionMethodInfo.MakeGenericMethod(collectionType, relatedEntityClrType),
_populateCollectionMethodInfo.MakeGenericMethod(collectionType, elementType, relatedEntityClrType),
Expression.Constant(collectionShaper.CollectionId),
QueryCompilationContext.QueryContextParameter,
_dbDataReaderParameter,
Expand Down
9 changes: 6 additions & 3 deletions src/EFCore.Relational/Query/Pipeline/RelationalCollectionShaperExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,16 @@ public RelationalCollectionShaperExpression(
Expression outerIdentifier,
Expression selfIdentifier,
Expression innerShaper,
INavigation navigation)
INavigation navigation,
Type elementType)
{
CollectionId = collectionId;
ParentIdentifier = parentIdentifier;
OuterIdentifier = outerIdentifier;
SelfIdentifier = selfIdentifier;
InnerShaper = innerShaper;
Navigation = navigation;
ElementType = elementType;
}

public int CollectionId { get; }
Expand All @@ -34,8 +36,9 @@ public RelationalCollectionShaperExpression(
public Expression SelfIdentifier { get; }
public Expression InnerShaper { get; }
public INavigation Navigation { get; }
public Type ElementType { get; }

public override Type Type => Navigation?.ClrType ?? typeof(List<>).MakeGenericType(InnerShaper.Type);
public override Type Type => Navigation?.ClrType ?? typeof(List<>).MakeGenericType(ElementType);
public override ExpressionType NodeType => ExpressionType.Extension;

protected override Expression VisitChildren(ExpressionVisitor visitor)
Expand All @@ -56,7 +59,7 @@ public RelationalCollectionShaperExpression Update(
|| selfIdentifier != SelfIdentifier
|| innerShaper != InnerShaper
? new RelationalCollectionShaperExpression(
CollectionId, parentIdentifier, outerIdentifier, selfIdentifier, innerShaper, Navigation)
CollectionId, parentIdentifier, outerIdentifier, selfIdentifier, innerShaper, Navigation, ElementType)
: this;
}

Expand Down
13 changes: 9 additions & 4 deletions src/EFCore.Relational/Query/Pipeline/RelationalProjectionBindingExpressionVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using Microsoft.EntityFrameworkCore.Metadata;
Expand Down Expand Up @@ -101,14 +102,18 @@ public override Expression Visit(Expression expression)
var result = _queryableMethodTranslatingExpressionVisitor.TranslateSubquery(methodCallExpression.Arguments[0]);
var navigation = (INavigation)((ConstantExpression)methodCallExpression.Arguments[1]).Value;

return _selectExpression.AddCollectionProjection(result, navigation);
return _selectExpression.AddCollectionProjection(result, navigation, null);
}

if (methodCallExpression.Method.Name == "ToList")
if (methodCallExpression.Method.IsGenericMethod
&& methodCallExpression.Method.DeclaringType == typeof(Enumerable)
&& methodCallExpression.Method.Name == nameof(Enumerable.ToList))
{
var elementType = methodCallExpression.Method.GetGenericArguments()[0];

var result = _queryableMethodTranslatingExpressionVisitor.TranslateSubquery(methodCallExpression.Arguments[0]);

return _selectExpression.AddCollectionProjection(result, null);
return _selectExpression.AddCollectionProjection(result, null, elementType);
}

var subquery = _queryableMethodTranslatingExpressionVisitor.TranslateSubquery(methodCallExpression);
Expand All @@ -117,7 +122,7 @@ public override Expression Visit(Expression expression)
{
if (subquery.ResultType == ResultType.Enumerable)
{
return _selectExpression.AddCollectionProjection(subquery, null);
return _selectExpression.AddCollectionProjection(subquery, null, subquery.ShaperExpression.Type);
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/EFCore.Relational/Query/Pipeline/RelationalShapedQueryOptimizingExpressionVisitors.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ protected override Expression VisitExtension(Expression extensionExpression)
collectionShaperExpression.Projection.Index.Value,
collectionId,
innerShaper,
collectionShaperExpression.Navigation);
collectionShaperExpression.Navigation,
collectionShaperExpression.ElementType);
}

if (extensionExpression is ShapedQueryExpression shapedQueryExpression)
Expand Down
77 changes: 55 additions & 22 deletions src/EFCore.Relational/Query/Pipeline/SqlExpressions/SelectExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Data.SqlTypes;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
Expand Down Expand Up @@ -541,37 +542,57 @@ public IDictionary<SqlExpression, ColumnExpression> PushdownIntoSubquery()
}
}

var sqlRemappingVisitor = new SqlRemappingVisitor(projectionMap);

var identifier = _identifier.ToList();
var identifiers = _identifier.ToList();
_identifier.Clear();
// TODO: See issue#15873
_identifier.AddRange(identifier.Select(sqlRemappingVisitor.Remap));
foreach (var identifier in identifiers)
{
if (projectionMap.TryGetValue(identifier, out var outerColumn))
{
_identifier.Add(outerColumn);
}
else if (!IsDistinct)
{
var index = subquery.AddToProjection(identifier);
var projectionExpression = subquery._projection[index];
outerColumn = new ColumnExpression(projectionExpression, subquery, IsNullableProjection(projectionExpression));
_identifier.Add(outerColumn);
}
}

var childIdentifiers = _childIdentifiers.ToList();
_childIdentifiers.Clear();
// TODO: See issue#15873
_childIdentifiers.AddRange(childIdentifiers.Select(sqlRemappingVisitor.Remap));
foreach (var identifier in childIdentifiers)
{
if (projectionMap.TryGetValue(identifier, out var outerColumn))
{
_childIdentifiers.Add(outerColumn);
}
else if (!IsDistinct)
{
var index = subquery.AddToProjection(identifier);
var projectionExpression = subquery._projection[index];
outerColumn = new ColumnExpression(projectionExpression, subquery, IsNullableProjection(projectionExpression));
_childIdentifiers.Add(outerColumn);
}
}

var pendingCollections = _pendingCollections.ToList();
_pendingCollections.Clear();
_pendingCollections.AddRange(pendingCollections.Select(sqlRemappingVisitor.Remap));
_pendingCollections.AddRange(pendingCollections.Select(new SqlRemappingVisitor(projectionMap).Remap));

_orderings.Clear();
foreach (var ordering in subquery._orderings)
{
var orderingExpression = ordering.Expression;
if (projectionMap.TryGetValue(orderingExpression, out var outerColumn))
{
_orderings.Add(new OrderingExpression(outerColumn, ordering.Ascending));
}
else
if (!projectionMap.TryGetValue(orderingExpression, out var outerColumn))
{
var index = subquery.AddToProjection(orderingExpression);
var projectionExpression = subquery._projection[index];
outerColumn = new ColumnExpression(projectionExpression, subquery, IsNullableProjection(projectionExpression));
_orderings.Add(new OrderingExpression(outerColumn, ordering.Ascending));
}
_orderings.Add(new OrderingExpression(outerColumn, ordering.Ascending));
}

Offset = null;
Expand All @@ -590,18 +611,21 @@ private static bool IsNullableProjection(ProjectionExpression projection)
return projection.Expression is ColumnExpression column ? column.Nullable : true;
}

public CollectionShaperExpression AddCollectionProjection(ShapedQueryExpression shapedQueryExpression, INavigation navigation)
public CollectionShaperExpression AddCollectionProjection(
ShapedQueryExpression shapedQueryExpression, INavigation navigation, Type elementType)
{
var innerSelectExpression = (SelectExpression)shapedQueryExpression.QueryExpression;
_pendingCollections.Add(innerSelectExpression);

return new CollectionShaperExpression(
new ProjectionBindingExpression(this, _pendingCollections.Count - 1, typeof(object)),
shapedQueryExpression.ShaperExpression,
navigation);
navigation,
elementType);
}

public Expression ApplyCollectionJoin(int collectionIndex, int collectionId, Expression innerShaper, INavigation navigation)
public Expression ApplyCollectionJoin(
int collectionIndex, int collectionId, Expression innerShaper, INavigation navigation, Type elementType)
{
var innerSelectExpression = _pendingCollections[collectionIndex];
_pendingCollections[collectionIndex] = null;
Expand All @@ -627,21 +651,27 @@ public Expression ApplyCollectionJoin(int collectionIndex, int collectionId, Exp
|| innerSelectExpression.Predicate != null
|| innerSelectExpression.Tables.Count > 1)
{
joinPredicate = new SqlRemappingVisitor(innerSelectExpression.PushdownIntoSubquery())
.Remap(joinPredicate);
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);
var indexOffset = _projection.Count;
foreach (var projection in innerSelectExpression.Projection)
{
AddToProjection(projection.Expression is ColumnExpression column ? column.MakeNullable() : projection.Expression);
AddToProjection(MakeNullable(projection.Expression));
}

foreach (var column in innerSelectExpression._identifier.Concat(innerSelectExpression._childIdentifiers))
foreach (var identifier in innerSelectExpression._identifier.Concat(innerSelectExpression._childIdentifiers))
{
var updatedColumn = column is ColumnExpression column1 ? column1.MakeNullable() : column;
var updatedColumn = MakeNullable(identifier);
_childIdentifiers.Add(updatedColumn);
AppendOrdering(new OrderingExpression(updatedColumn, ascending: true));
}
Expand All @@ -651,12 +681,15 @@ public Expression ApplyCollectionJoin(int collectionIndex, int collectionId, Exp
selfIdentifier = shaperRemapper.Visit(selfIdentifier);

return new RelationalCollectionShaperExpression(
collectionId, parentIdentifier, outerIdentifier, selfIdentifier, innerShaper, navigation);
collectionId, parentIdentifier, outerIdentifier, selfIdentifier, innerShaper, navigation, elementType);
}

throw new NotImplementedException();
throw new InvalidOperationException("CollectionJoin: Unable to identify correlation predicate to convert to Left Join");
}

private SqlExpression MakeNullable(SqlExpression sqlExpression)
=> sqlExpression is ColumnExpression column ? column.MakeNullable() : sqlExpression;

private Expression GetIdentifierAccessor(IEnumerable<SqlExpression> identifyingProjection)
{
var updatedExpressions = new List<Expression>();
Expand Down
9 changes: 6 additions & 3 deletions src/EFCore/Query/Pipeline/CollectionShaperExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@ public class CollectionShaperExpression : Expression, IPrintable
public CollectionShaperExpression(
ProjectionBindingExpression projection,
Expression innerShaper,
INavigation navigation)
INavigation navigation,
Type elementType)
{
Projection = projection;
InnerShaper = innerShaper;
Navigation = navigation;
ElementType = elementType;
}

protected override Expression VisitChildren(ExpressionVisitor visitor)
Expand All @@ -33,17 +35,18 @@ protected override Expression VisitChildren(ExpressionVisitor visitor)
public CollectionShaperExpression Update(ProjectionBindingExpression projection, Expression innerShaper)
{
return projection != Projection || innerShaper != InnerShaper
? new CollectionShaperExpression(projection, innerShaper, Navigation)
? new CollectionShaperExpression(projection, innerShaper, Navigation, ElementType)
: this;
}

public override ExpressionType NodeType => ExpressionType.Extension;

public override Type Type => Navigation?.ClrType ?? typeof(List<>).MakeGenericType(InnerShaper.Type);
public override Type Type => Navigation?.ClrType ?? typeof(List<>).MakeGenericType(ElementType);

public ProjectionBindingExpression Projection { get; }
public Expression InnerShaper { get; }
public INavigation Navigation { get; }
public Type ElementType { get; }

public virtual void Print(ExpressionPrinter expressionPrinter)
{
Expand Down
7 changes: 7 additions & 0 deletions src/EFCore/Query/Pipeline/EnumerableToQueryableReMappingExpressionVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;
Expand Down Expand Up @@ -113,6 +114,12 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp

private static bool CanConvertEnumerableToQueryable(Type enumerableType, Type queryableType, Type argumentType)
{
if (enumerableType == typeof(IEnumerable)
&& queryableType == typeof(IQueryable))
{
return true;
}

if (!enumerableType.IsGenericType
|| !queryableType.IsGenericType
|| argumentType.TryGetElementType(typeof(IQueryable<>)) == null
Expand Down
3 changes: 1 addition & 2 deletions src/EFCore/Query/Pipeline/QueryableMethodTranslatingExpressionVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
: null);

case nameof(Queryable.AsQueryable):
// Don't know
break;
return source;

case nameof(Queryable.Average):
shapedQueryExpression.ResultType = ResultType.Single;
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Query/Pipeline/ShapedQueryExpressionVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ var discriminatorValue
{
var accessorExpression = Expression.Constant(new ClrCollectionAccessorFactory().Create(navigation));
navigationExpression = Expression.Call(accessorExpression, _accessorAddRangeMethodInfo,
convertedInstanceVariable, new CollectionShaperExpression(null, nestedShaper, navigation));
convertedInstanceVariable, new CollectionShaperExpression(null, nestedShaper, navigation, null));
}
else
{
Expand Down
18 changes: 18 additions & 0 deletions test/EFCore.Cosmos.FunctionalTests/Query/SimpleQueryCosmosTest.ResultOperators.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,24 @@ public override void Max_no_data()
base.Max_no_data();
}

[ConditionalFact(Skip = "Issue#16146")]
public override void Average_no_data_subquery()
{
base.Average_no_data_subquery();
}

[ConditionalFact(Skip = "Issue#16146")]
public override void Max_no_data_subquery()
{
base.Max_no_data_subquery();
}

[ConditionalFact(Skip = "Issue#16146")]
public override void Min_no_data_subquery()
{
base.Min_no_data_subquery();
}

[ConditionalTheory(Skip = "Issue#16146")]
public override async Task Average_with_no_arg(bool isAsync)
{
Expand Down
Loading