Skip to content

Commit

Permalink
Query: Wrap up collection materialization code path for relational
Browse files Browse the repository at this point in the history
Bugs fixed
- Allow ToList on collection projection to be called with less derived type
- Project out necessary identifiers for collection join when pushing down. (Except distinct case)
- Lift ordering from inner collection to generated ordered result (This would be improved further by #16226)
- Map Cast from Enumerable to Queryable
- Translate AsQueryable as no-op

Resolves #12098
Resolves #15043
Resolves #15611
Part of #15711
  • Loading branch information
smitpatel committed Jun 28, 2019
1 parent 84860c8 commit fd4b5d6
Show file tree
Hide file tree
Showing 36 changed files with 984 additions and 1,385 deletions.
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
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
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
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
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
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
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
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
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
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

0 comments on commit fd4b5d6

Please sign in to comment.