Skip to content

Commit

Permalink
Query: Implement filtered include for AsSplitQuery operator (#21277)
Browse files Browse the repository at this point in the history 
Resolves #21276
  • Loading branch information
@smitpatel
smitpatel authored Jun 16, 2020
1 parent 4ace0e2 commit 1b95ee1
Show file tree
Hide file tree
Showing 17 changed files with 826 additions and 77 deletions.
10 changes: 9 additions & 1 deletion src/EFCore.Relational/Query/Internal/SplitIncludeRewritingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,12 +152,20 @@ protected override Expression VisitExtension(Expression extensionExpression)

private Expression RewriteSubqueryToSelectMany(Expression subquery)
{
var collectionElementType = subquery.Type.TryGetSequenceType();
if (subquery.Type.GetGenericTypeDefinition() == typeof(IOrderedQueryable<>))
{
subquery = Expression.Call(
QueryableMethods.Skip.MakeGenericMethod(collectionElementType),
subquery,
Expression.Constant(0));
}

var newParameter = Expression.Parameter(_parameterExpression.Type);
subquery = ReplacingExpressionVisitor.Replace(_parameterExpression, newParameter, subquery);

// Collection selector body is IQueryable, we need to adjust the type to IEnumerable, to match the SelectMany signature
// therefore the delegate type is specified explicitly
var collectionElementType = subquery.Type.TryGetSequenceType();
var collectionSelectorLambdaType = typeof(Func<,>).MakeGenericType(
_sourceElementType,
typeof(IEnumerable<>).MakeGenericType(collectionElementType));
Expand Down
78 changes: 75 additions & 3 deletions src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1124,13 +1124,85 @@ public Expression ApplyCollectionJoin(
{
var parentIdentifier = GetIdentifierAccessor(_identifier).Item1;
innerSelectExpression.ApplyProjection();
var (childIdentifier, childIdentifierValueComparers) = innerSelectExpression
.GetIdentifierAccessor(innerSelectExpression._identifier.Take(_identifier.Count));

for (var i = 0; i < _identifier.Count; i++)
{
AppendOrdering(new OrderingExpression(_identifier[i].Column, ascending: true));
innerSelectExpression.AppendOrdering(new OrderingExpression(innerSelectExpression._identifier[i].Column, ascending: true));
}

// Copy over ordering from previous collections
var innerOrderingExpressions = new List<OrderingExpression>();
foreach (var table in innerSelectExpression.Tables)
{
if (table is InnerJoinExpression collectionJoinExpression
&& collectionJoinExpression.Table is SelectExpression collectionSelectExpression
&& collectionSelectExpression.Predicate != null
&& collectionSelectExpression.Tables.Count == 1
&& collectionSelectExpression.Tables[0] is SelectExpression rowNumberSubquery
&& rowNumberSubquery.Projection.Select(pe => pe.Expression)
.OfType<RowNumberExpression>().SingleOrDefault() is RowNumberExpression rowNumberExpression)
{
foreach (var partition in rowNumberExpression.Partitions)
{
innerOrderingExpressions.Add(new OrderingExpression(
collectionSelectExpression.GenerateOuterColumn(rowNumberSubquery.GenerateOuterColumn(partition)),
ascending: true));
}

foreach (var ordering in rowNumberExpression.Orderings)
{
innerOrderingExpressions.Add(new OrderingExpression(
collectionSelectExpression.GenerateOuterColumn(rowNumberSubquery.GenerateOuterColumn(ordering.Expression)),
ordering.IsAscending));
}
}

if (table is CrossApplyExpression collectionApplyExpression
&& collectionApplyExpression.Table is SelectExpression collectionSelectExpression2
&& collectionSelectExpression2.Orderings.Count > 0)
{
foreach (var ordering in collectionSelectExpression2.Orderings)
{
if (innerSelectExpression._identifier.Any(e => e.Column.Equals(ordering.Expression)))
{
continue;
}

innerOrderingExpressions.Add(new OrderingExpression(
collectionSelectExpression2.GenerateOuterColumn(ordering.Expression),
ordering.IsAscending));
}
}
}

var (childIdentifier, childIdentifierValueComparers) = innerSelectExpression
.GetIdentifierAccessor(innerSelectExpression._identifier.Take(_identifier.Count));

var identifierIndex = 0;
var orderingIndex = 0;
for (var i = 0; i < Orderings.Count; i++)
{
var outerOrdering = Orderings[i];
if (outerOrdering.Expression.Equals(_identifier[identifierIndex].Column))
{
innerSelectExpression.AppendOrdering(new OrderingExpression(innerSelectExpression._identifier[identifierIndex].Column, ascending: true));
identifierIndex++;
}
else
{
if (i < innerSelectExpression.Orderings.Count)
{
continue;
}

innerSelectExpression.AppendOrdering(innerOrderingExpressions[orderingIndex]);
orderingIndex++;
}
}

foreach (var orderingExpression in innerOrderingExpressions.Skip(orderingIndex))
{
innerSelectExpression.AppendOrdering(orderingExpression);
}

return new RelationalSplitCollectionShaperExpression(
Expand Down
74 changes: 42 additions & 32 deletions .../EFCore.Relational.Specification.Tests/Query/ComplexNavigationsQueryRelationalTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public virtual Task Filtered_include_basic_Where_split(bool async)
includeFilter: x => x.Where(l2 => l2.Id > 5))));
}

[ConditionalTheory(Skip = "Issue#20892")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Filtered_include_OrderBy_split(bool async)
{
Expand All @@ -44,7 +44,7 @@ public virtual Task Filtered_include_OrderBy_split(bool async)
includeFilter: x => x.OrderBy(x => x.Name))));
}

[ConditionalTheory(Skip = "Issue#20892")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Filtered_ThenInclude_OrderBy_split(bool async)
{
Expand All @@ -62,7 +62,7 @@ public virtual Task Filtered_ThenInclude_OrderBy_split(bool async)
includeFilter: x => x.OrderBy(x => x.Name))));
}

[ConditionalTheory(Skip = "Issue#20892")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Filtered_include_ThenInclude_OrderBy_split(bool async)
{
Expand All @@ -82,7 +82,7 @@ public virtual Task Filtered_include_ThenInclude_OrderBy_split(bool async)
includeFilter: x => x.OrderByDescending(x => x.Name))));
}

[ConditionalTheory(Skip = "Issue#20892")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Filtered_include_basic_OrderBy_Take_split(bool async)
{
Expand All @@ -95,7 +95,7 @@ public virtual Task Filtered_include_basic_OrderBy_Take_split(bool async)
includeFilter: x => x.OrderBy(x => x.Name).Take(3))));
}

[ConditionalTheory(Skip = "Issue#20892")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Filtered_include_basic_OrderBy_Skip_split(bool async)
{
Expand All @@ -108,7 +108,7 @@ public virtual Task Filtered_include_basic_OrderBy_Skip_split(bool async)
includeFilter: x => x.OrderBy(x => x.Name).Skip(1))));
}

[ConditionalTheory(Skip = "Issue#20892")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Filtered_include_basic_OrderBy_Skip_Take_split(bool async)
{
Expand All @@ -121,23 +121,23 @@ public virtual Task Filtered_include_basic_OrderBy_Skip_Take_split(bool async)
includeFilter: x => x.OrderBy(x => x.Name).Skip(1).Take(3))));
}

[ConditionalFact(Skip = "Issue#20892")]
[ConditionalFact]
public virtual void Filtered_include_Skip_without_OrderBy_split()
{
using var ctx = CreateContext();
var query = ctx.LevelOne.Include(l1 => l1.OneToMany_Optional1.Skip(1)).AsSplitQuery();
var result = query.ToList();
}

[ConditionalFact(Skip = "Issue#20892")]
[ConditionalFact]
public virtual void Filtered_include_Take_without_OrderBy_split()
{
using var ctx = CreateContext();
var query = ctx.LevelOne.Include(l1 => l1.OneToMany_Optional1.Take(1)).AsSplitQuery();
var result = query.ToList();
}

[ConditionalTheory(Skip = "Issue#20892")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Filtered_include_on_ThenInclude_split(bool async)
{
Expand All @@ -155,7 +155,7 @@ public virtual Task Filtered_include_on_ThenInclude_split(bool async)
x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Name).Skip(1).Take(3))));
}

[ConditionalTheory(Skip = "Issue#20892")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Filtered_include_after_reference_navigation_split(bool async)
{
Expand All @@ -172,7 +172,7 @@ public virtual Task Filtered_include_after_reference_navigation_split(bool async
x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Name).Skip(1).Take(3))));
}

[ConditionalTheory(Skip = "Issue#20892")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Filtered_include_after_different_filtered_include_same_level_split(bool async)
{
Expand All @@ -191,7 +191,7 @@ public virtual Task Filtered_include_after_different_filtered_include_same_level
includeFilter: x => x.Where(x => x.Name != "Bar").OrderByDescending(x => x.Name).Skip(1))));
}

[ConditionalTheory(Skip = "Issue#20892")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Filtered_include_after_different_filtered_include_different_level_split(bool async)
{
Expand All @@ -211,7 +211,7 @@ public virtual Task Filtered_include_after_different_filtered_include_different_
includeFilter: x => x.Where(x => x.Name != "Bar").OrderByDescending(x => x.Name).Skip(1))));
}

[ConditionalTheory(Skip = "Issue#20892")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Filtered_include_different_filter_set_on_same_navigation_twice_split(bool async)
{
Expand All @@ -237,7 +237,7 @@ public virtual async Task Filtered_include_different_filter_set_on_same_navigati
.AsSplitQuery()))).Message;
}

[ConditionalTheory(Skip = "Issue#20892")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Filtered_include_same_filter_set_on_same_navigation_twice_split(bool async)
{
Expand All @@ -253,7 +253,7 @@ public virtual Task Filtered_include_same_filter_set_on_same_navigation_twice_sp
includeFilter: x => x.Where(x => x.Name != "Foo").OrderByDescending(x => x.Id).Take(2))));
}

[ConditionalTheory(Skip = "Issue#20892")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Filtered_include_same_filter_set_on_same_navigation_twice_followed_by_ThenIncludes_split(bool async)
{
Expand All @@ -271,15 +271,17 @@ public virtual Task Filtered_include_same_filter_set_on_same_navigation_twice_fo
new ExpectedInclude<Level2>(e => e.OneToOne_Required_FK2)));
}

[ConditionalTheory(Skip = "Issue#20892")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Filtered_include_multiple_multi_level_includes_with_first_level_using_filter_include_on_one_of_the_chains_only_split(bool async)
{
return AssertQuery(
async,
ss => ss.Set<Level1>()
.Include(l1 => l1.OneToMany_Optional1.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(2)).ThenInclude(l2 => l2.OneToMany_Optional2)
.Include(l1 => l1.OneToMany_Optional1).ThenInclude(l2 => l2.OneToOne_Required_FK2)
.Include(l1 => l1.OneToMany_Optional1.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(2))
.ThenInclude(l2 => l2.OneToMany_Optional2)
.Include(l1 => l1.OneToMany_Optional1)
.ThenInclude(l2 => l2.OneToOne_Required_FK2)
.AsSplitQuery(),
elementAsserter: (e, a) => AssertInclude(e, a,
new ExpectedFilteredInclude<Level1, Level2>(
Expand All @@ -289,7 +291,7 @@ public virtual Task Filtered_include_multiple_multi_level_includes_with_first_le
new ExpectedInclude<Level2>(e => e.OneToOne_Required_FK2, "OneToMany_Optional1")));
}

[ConditionalTheory(Skip = "Issue#20892")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Filtered_include_and_non_filtered_include_on_same_navigation1_split(bool async)
{
Expand All @@ -305,7 +307,7 @@ public virtual Task Filtered_include_and_non_filtered_include_on_same_navigation
includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(3))));
}

[ConditionalTheory(Skip = "Issue#20892")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Filtered_include_and_non_filtered_include_on_same_navigation2_split(bool async)
{
Expand All @@ -321,7 +323,7 @@ public virtual Task Filtered_include_and_non_filtered_include_on_same_navigation
includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(3))));
}

[ConditionalTheory(Skip = "Issue#20892")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Filtered_include_and_non_filtered_include_followed_by_then_include_on_same_navigation_split(bool async)
{
Expand All @@ -342,15 +344,19 @@ public virtual Task Filtered_include_and_non_filtered_include_followed_by_then_i
includeFilter: x => x.Where(x => x.Id > 1))));
}

[ConditionalTheory(Skip = "Issue#20892")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Filtered_include_complex_three_level_with_middle_having_filter1_split(bool async)
{
return AssertQuery(
async,
ss => ss.Set<Level1>()
.Include(l1 => l1.OneToMany_Optional1).ThenInclude(l2 => l2.OneToMany_Optional2.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(1)).ThenInclude(l3 => l3.OneToMany_Optional3)
.Include(l1 => l1.OneToMany_Optional1).ThenInclude(l2 => l2.OneToMany_Optional2.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(1)).ThenInclude(l3 => l3.OneToMany_Required3)
.Include(l1 => l1.OneToMany_Optional1)
.ThenInclude(l2 => l2.OneToMany_Optional2.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(1))
.ThenInclude(l3 => l3.OneToMany_Optional3)
.Include(l1 => l1.OneToMany_Optional1)
.ThenInclude(l2 => l2.OneToMany_Optional2.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(1))
.ThenInclude(l3 => l3.OneToMany_Required3)
.AsSplitQuery(),
elementAsserter: (e, a) => AssertInclude(e, a,
new ExpectedInclude<Level1>(e => e.OneToMany_Optional1),
Expand All @@ -362,15 +368,19 @@ public virtual Task Filtered_include_complex_three_level_with_middle_having_filt
new ExpectedInclude<Level3>(e => e.OneToMany_Required3, "OneToMany_Optional1.OneToMany_Optional2")));
}

[ConditionalTheory(Skip = "Issue#20892")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Filtered_include_complex_three_level_with_middle_having_filter2_split(bool async)
{
return AssertQuery(
async,
ss => ss.Set<Level1>()
.Include(l1 => l1.OneToMany_Optional1).ThenInclude(l2 => l2.OneToMany_Optional2.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(1)).ThenInclude(l3 => l3.OneToMany_Optional3)
.Include(l1 => l1.OneToMany_Optional1).ThenInclude(l2 => l2.OneToMany_Optional2).ThenInclude(l3 => l3.OneToMany_Required3)
.Include(l1 => l1.OneToMany_Optional1)
.ThenInclude(l2 => l2.OneToMany_Optional2.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(1)).
ThenInclude(l3 => l3.OneToMany_Optional3)
.Include(l1 => l1.OneToMany_Optional1)
.ThenInclude(l2 => l2.OneToMany_Optional2)
.ThenInclude(l3 => l3.OneToMany_Required3)
.AsSplitQuery(),
elementAsserter: (e, a) => AssertInclude(e, a,
new ExpectedInclude<Level1>(e => e.OneToMany_Optional1),
Expand All @@ -382,7 +392,7 @@ public virtual Task Filtered_include_complex_three_level_with_middle_having_filt
new ExpectedInclude<Level3>(e => e.OneToMany_Required3, "OneToMany_Optional1.OneToMany_Optional2")));
}

[ConditionalFact(Skip = "Issue#20892")]
[ConditionalFact]
public virtual void Filtered_include_variable_used_inside_filter_split()
{
using var ctx = CreateContext();
Expand All @@ -392,7 +402,7 @@ public virtual void Filtered_include_variable_used_inside_filter_split()
var result = query.ToList();
}

[ConditionalFact(Skip = "Issue#20892")]
[ConditionalFact]
public virtual void Filtered_include_context_accessed_inside_filter_split()
{
using var ctx = CreateContext();
Expand All @@ -401,7 +411,7 @@ public virtual void Filtered_include_context_accessed_inside_filter_split()
var result = query.ToList();
}

[ConditionalFact(Skip = "Issue#20892")]
[ConditionalFact]
public virtual void Filtered_include_context_accessed_inside_filter_correlated_split()
{
using var ctx = CreateContext();
Expand All @@ -422,7 +432,7 @@ await Assert.ThrowsAsync<InvalidOperationException>(
.Select(l1 => ss.Set<Level2>().Include(l2 => l2.OneToMany_Optional2.Where(x => x.Id != l2.Id))).AsSplitQuery()));
}

[ConditionalFact(Skip = "Issue#20892")]
[ConditionalFact(Skip = "Issue#21234")]
public virtual void Filtered_include_outer_parameter_used_inside_filter_split()
{
// TODO: needs #18191 for result verification
Expand All @@ -436,7 +446,7 @@ public virtual void Filtered_include_outer_parameter_used_inside_filter_split()
var result = query.ToList();
}

[ConditionalFact(Skip = "Issue#20892")]
[ConditionalFact]
public virtual void Filtered_include_is_considered_loaded_split()
{
using var ctx = CreateContext();
Expand Down
Loading

0 comments on commit 1b95ee1

Please sign in to comment.