Skip to content

Commit

Permalink
Fix to #4749 - Query :: error during groupjoin flattening for complex…
Browse files Browse the repository at this point in the history 
… query with multiple joins subquery and LOJ

Problem was that we tried to lift queries that we shouldn't have - i.e. GroupJoin whose ShapedQuery elements were wrapped in Select. Fix is to check the shape of the expression before performing the lifting - we do similar thing for SelectMany already.

CR: Andrew
  • Loading branch information
@maumar
maumar committed Jul 21, 2016
1 parent 0b5de67 commit 9a34e1d
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 22 deletions.
57 changes: 35 additions & 22 deletions src/Microsoft.EntityFrameworkCore.Relational/Query/RelationalQueryModelVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -509,47 +509,45 @@ private bool CanFlattenSelectMany()
{
var selectManyExpression = Expression as MethodCallExpression;
if (selectManyExpression == null
|| !selectManyExpression.Method.MethodIsClosedFormOf(LinqOperatorProvider.SelectMany))
|| !selectManyExpression.Method.MethodIsClosedFormOf(LinqOperatorProvider.SelectMany)
|| !IsShapedQueryExpression(selectManyExpression.Arguments[0] as MethodCallExpression, innerShapedQuery: false)
|| !IsShapedQueryExpression((selectManyExpression.Arguments[1] as LambdaExpression)?.Body as MethodCallExpression, innerShapedQuery: true))
{
return false;
}

var outerShapedQuery = selectManyExpression.Arguments[0] as MethodCallExpression;
if (outerShapedQuery == null || outerShapedQuery.Arguments.Count != 3)
{
return false;
}

var outerShaper = outerShapedQuery.Arguments[2] as ConstantExpression;
if (!(outerShaper?.Value is Shaper))
{
return false;
}
return true;
}

var innerShapedQuery = (selectManyExpression.Arguments[1] as LambdaExpression)?.Body as MethodCallExpression;
if (innerShapedQuery == null)
private bool IsShapedQueryExpression(MethodCallExpression shapedQueryExpression, bool innerShapedQuery)
{
if (shapedQueryExpression == null)
{
return false;
}

if (innerShapedQuery.Method.MethodIsClosedFormOf(LinqOperatorProvider.DefaultIfEmpty)
|| innerShapedQuery.Method.MethodIsClosedFormOf(LinqOperatorProvider.DefaultIfEmptyArg))
if (innerShapedQuery && (shapedQueryExpression.Method.MethodIsClosedFormOf(LinqOperatorProvider.DefaultIfEmpty)
|| shapedQueryExpression.Method.MethodIsClosedFormOf(LinqOperatorProvider.DefaultIfEmptyArg)))
{
innerShapedQuery = innerShapedQuery.Arguments.Single() as MethodCallExpression;
if (innerShapedQuery == null)
shapedQueryExpression = shapedQueryExpression.Arguments.Single() as MethodCallExpression;
if (shapedQueryExpression == null)
{
return false;
}
}

if (innerShapedQuery.Arguments.Count != 3)
if (shapedQueryExpression == null || shapedQueryExpression.Arguments.Count != 3)
{
return false;
}

var innerShaper = innerShapedQuery.Arguments[2] as ConstantExpression;
var shaper = shapedQueryExpression.Arguments[2] as ConstantExpression;
if (shaper == null || !(shaper.Value is Shaper))
{
return false;
}

return innerShaper?.Value is Shaper;
return true;
}

/// <summary>
Expand Down Expand Up @@ -667,7 +665,8 @@ var previousSelectProjectionCount
baseVisitAction();

if (!RequiresClientSelectMany
&& previousSelectExpression != null)
&& previousSelectExpression != null
&& (!operatorToFlatten.MethodIsClosedFormOf(LinqOperatorProvider.GroupJoin) || CanFlattenGroupJoin()))
{
var selectExpression = TryGetQuery(joinClause);

Expand Down Expand Up @@ -737,6 +736,20 @@ var joinExpression
}
}

private bool CanFlattenGroupJoin()
{
var groupJoinExpression = Expression as MethodCallExpression;
if (groupJoinExpression == null
|| !groupJoinExpression.Method.MethodIsClosedFormOf(LinqOperatorProvider.GroupJoin)
|| !IsShapedQueryExpression(groupJoinExpression.Arguments[0] as MethodCallExpression, innerShapedQuery: false)
|| !IsShapedQueryExpression(groupJoinExpression.Arguments[1] as MethodCallExpression, innerShapedQuery: true))
{
return false;
}

return true;
}

private class OuterJoinOrderingExtractor : ExpressionVisitor
{
private readonly List<Expression> _expressions = new List<Expression>();
Expand Down
51 changes: 51 additions & 0 deletions src/Microsoft.EntityFrameworkCore.Specification.Tests/QueryNavigationsTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -871,6 +871,25 @@ orderby o.OrderID
}).Take(3));
}

[ConditionalFact]
public virtual void GroupJoin_with_complex_subquery_and_LOJ_does_not_get_flattened()
{
AssertQuery<Customer, Order, OrderDetail, Customer>(
(cs, os, ods) => (from c in cs
join subquery in
(
from od in ods
join o in os on od.OrderID equals 10260
join c2 in cs on o.CustomerID equals c2.CustomerID
select c2
)
on c.CustomerID equals subquery.CustomerID
into result
from subquery in result.DefaultIfEmpty()
select c),
entryCount: 91);
}

protected QueryNavigationsTestBase(TFixture fixture)
{
Fixture = fixture;
Expand Down Expand Up @@ -901,6 +920,16 @@ protected void AssertQuery<TItem1, TItem2, TResult>(
where TItem2 : class
=> AssertQuery(query, query, assertOrder, entryCount, asserter);

protected void AssertQuery<TItem1, TItem2, TItem3, TResult>(
Func<IQueryable<TItem1>, IQueryable<TItem2>, IQueryable<TItem3>, IQueryable<TResult>> query,
bool assertOrder = false,
int entryCount = 0,
Action<IList<TResult>, IList<TResult>> asserter = null)
where TItem1 : class
where TItem2 : class
where TItem3 : class
=> AssertQuery(query, query, assertOrder, entryCount, asserter);

protected void AssertQuery<TItem, TResult>(
Func<IQueryable<TItem>, IQueryable<TResult>> query,
bool assertOrder = false,
Expand Down Expand Up @@ -967,5 +996,27 @@ protected void AssertQuery<TItem1, TItem2, TResult>(
Assert.Equal(entryCount, context.ChangeTracker.Entries().Count());
}
}

protected void AssertQuery<TItem1, TItem2, TItem3, TResult>(
Func<IQueryable<TItem1>, IQueryable<TItem2>, IQueryable<TItem3>, IQueryable<TResult>> efQuery,
Func<IQueryable<TItem1>, IQueryable<TItem2>, IQueryable<TItem3>, IQueryable<TResult>> l2oQuery,
bool assertOrder = false,
int entryCount = 0,
Action<IList<TResult>, IList<TResult>> asserter = null)
where TItem1 : class
where TItem2 : class
where TItem3 : class
{
using (var context = CreateContext())
{
TestHelpers.AssertResults(
l2oQuery(NorthwindData.Set<TItem1>(), NorthwindData.Set<TItem2>(), NorthwindData.Set<TItem3>()).ToArray(),
efQuery(context.Set<TItem1>(), context.Set<TItem2>(), context.Set<TItem3>()).ToArray(),
assertOrder,
asserter);

Assert.Equal(entryCount, context.ChangeTracker.Entries().Count());
}
}
}
}
20 changes: 20 additions & 0 deletions .../Microsoft.EntityFrameworkCore.SqlServer.FunctionalTests/QueryNavigationsSqlServerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1145,6 +1145,26 @@ FROM [Order Details] AS [od1]
Sql);
}

public override void GroupJoin_with_complex_subquery_and_LOJ_does_not_get_flattened()
{
base.GroupJoin_with_complex_subquery_and_LOJ_does_not_get_flattened();

Assert.Contains(
@"SELECT [t].[CustomerID]
FROM (
SELECT [c20].*
FROM [Order Details] AS [od0]
INNER JOIN [Orders] AS [o0] ON [od0].[OrderID] = 10260
INNER JOIN [Customers] AS [c20] ON [o0].[CustomerID] = [c20].[CustomerID]
) AS [t]",
Sql);

Assert.Contains(
@"SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region]
FROM [Customers] AS [c]",
Sql);
}

protected override void ClearLog() => TestSqlLoggerFactory.Reset();

private const string FileLineEnding = @"
Expand Down

0 comments on commit 9a34e1d

Please sign in to comment.