Skip to content

Commit

Permalink
Fix to #7003 - Wrong SQL generated for query with group join on a sub…
Browse files Browse the repository at this point in the history 
…query that is not present in the final projection

Problem was that for GroupJoin we need to mark both inputs for materialization, so we have all the elements necessary to produce joining calls.
However, if one side is a subquery, we just mark that outer query source for materialization, without digging deep into the subquery itself. This results in the actual table (which is inside) not being materialized, and that in turn produces SQL which doesn't have necessary elements to produce the correct joins.

Fix is to check if the groupjoin element in a subquery, and if so recursively mark it's elements for materialization.
  • Loading branch information
@maumar
maumar committed Nov 18, 2016
1 parent 1caba2c commit a089701
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 13 deletions.
47 changes: 39 additions & 8 deletions src/Microsoft.EntityFrameworkCore.Specification.Tests/ComplexNavigationsQueryTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3111,14 +3111,45 @@ public virtual void Contains_with_subquery_optional_navigation_and_constant_item
public virtual void Complex_query_with_optional_navigations_and_client_side_evaluation()
{
AssertQuery<Level1>(
l1s => l1s.Where(l1 => !l1.OneToMany_Optional.Select(l2 => l2.OneToOne_Optional_FK.OneToOne_Optional_FK.Id).All(l4 => ClientMethod(l4))),
l1s => l1s.Where(l1 => l1.OneToMany_Optional.Select(l2 => MaybeScalar(
l2.OneToOne_Optional_FK,
() => MaybeScalar<int>(
l2.OneToOne_Optional_FK.OneToOne_Optional_FK,
() => l2.OneToOne_Optional_FK.OneToOne_Optional_FK.Id))).All(a => true)),
e => e.Id,
(e, a) => Assert.Equal(e.Id, a.Id));
l1s => l1s.Where(l1 => !l1.OneToMany_Optional.Select(l2 => l2.OneToOne_Optional_FK.OneToOne_Optional_FK.Id).All(l4 => ClientMethod(l4))),
l1s => l1s.Where(l1 => l1.OneToMany_Optional.Select(l2 => MaybeScalar(
l2.OneToOne_Optional_FK,
() => MaybeScalar<int>(
l2.OneToOne_Optional_FK.OneToOne_Optional_FK,
() => l2.OneToOne_Optional_FK.OneToOne_Optional_FK.Id))).All(a => true)),
e => e.Id,
(e, a) => Assert.Equal(e.Id, a.Id));
}

[ConditionalFact]
public virtual void GroupJoin_on_left_side_being_a_subquery()
{
AssertQuery<Level1>(
l1s => l1s.OrderBy(l1 => l1.OneToOne_Optional_FK.Name)
.Take(2)
.Select(x => new { Id = x.Id, Brand = x.OneToOne_Optional_FK.Name }),
l1s => l1s.OrderBy(l1 => Maybe(l1.OneToOne_Optional_FK, () => l1.OneToOne_Optional_FK.Name))
.Take(2)
.Select(x => new { Id = x.Id, Brand = Maybe(x.OneToOne_Optional_FK, () => x.OneToOne_Optional_FK.Name) }),
e => e.Id);
}

[ConditionalFact]
public virtual void GroupJoin_on_right_side_being_a_subquery()
{
AssertQuery<Level1, Level2>(
(l1s, l2s) =>
from l2 in l2s
join l1 in l1s.OrderBy(x => x.OneToOne_Optional_FK.Name).Take(2) on l2.Level1_Optional_Id equals l1.Id into grouping
from l1 in grouping.DefaultIfEmpty()
select new { Id = l2.Id, Nane = l1 != null ? l1.Name : null },
(l1s, l2s) =>
from l2 in l2s
join l1 in l1s.OrderBy(x => Maybe(x.OneToOne_Optional_FK, () => x.OneToOne_Optional_FK.Name)).Take(2)
on l2.Level1_Optional_Id equals l1.Id into grouping
from l1 in grouping.DefaultIfEmpty()
select new { Id = l2.Id, Nane = l1 != null ? l1.Name : null },
e => e.Id);
}

private bool ClientMethod(int? id)
Expand Down
21 changes: 16 additions & 5 deletions src/Microsoft.EntityFrameworkCore/Query/QueryCompilationContext.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public class QueryCompilationContext

private IReadOnlyCollection<IQueryAnnotation> _queryAnnotations;
private IDictionary<IQuerySource, List<IReadOnlyList<INavigation>>> _trackableIncludes;
private ISet<IQuerySource> _querySourcesRequiringMaterialization;
private ISet<IQuerySource> _querySourcesRequiringMaterialization = new HashSet<IQuerySource>();

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
Expand Down Expand Up @@ -340,15 +340,26 @@ public virtual void FindQuerySourcesRequiringMaterialization(
Check.NotNull(queryModelVisitor, nameof(queryModelVisitor));
Check.NotNull(queryModel, nameof(queryModel));

_querySourcesRequiringMaterialization
= _requiresMaterializationExpressionVisitorFactory
.Create(queryModelVisitor)
.FindQuerySourcesRequiringMaterialization(queryModel);
var querySourcesRequiringMaterialization = _requiresMaterializationExpressionVisitorFactory
.Create(queryModelVisitor)
.FindQuerySourcesRequiringMaterialization(queryModel);

foreach (var querySourceRequiringMaterialization in querySourcesRequiringMaterialization)
{
_querySourcesRequiringMaterialization.Add(querySourceRequiringMaterialization);
}

var groupJoinClauses = queryModel.BodyClauses.OfType<GroupJoinClause>().ToList();
if (groupJoinClauses.Any())
{
_querySourcesRequiringMaterialization.Add(queryModel.MainFromClause);

var mainFromClauseSubquery = queryModel.MainFromClause.FromExpression as SubQueryExpression;
if (mainFromClauseSubquery != null)
{
FindQuerySourcesRequiringMaterialization(queryModelVisitor, mainFromClauseSubquery.QueryModel);
}

foreach (var groupJoinClause in groupJoinClauses)
{
_querySourcesRequiringMaterialization.Add(groupJoinClause.JoinClause);
Expand Down
42 changes: 42 additions & 0 deletions ...oft.EntityFrameworkCore.SqlServer.FunctionalTests/ComplexNavigationsQuerySqlServerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2414,6 +2414,48 @@ public override void Complex_query_with_optional_navigations_and_client_side_eva
Sql);
}

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

Assert.Equal(
@"SELECT [x.OneToOne_Optional_FK].[Id], [x.OneToOne_Optional_FK].[Date], [x.OneToOne_Optional_FK].[Level1_Optional_Id], [x.OneToOne_Optional_FK].[Level1_Required_Id], [x.OneToOne_Optional_FK].[Name], [x.OneToOne_Optional_FK].[OneToMany_Optional_InverseId], [x.OneToOne_Optional_FK].[OneToMany_Optional_Self_InverseId], [x.OneToOne_Optional_FK].[OneToMany_Required_InverseId], [x.OneToOne_Optional_FK].[OneToMany_Required_Self_InverseId], [x.OneToOne_Optional_FK].[OneToOne_Optional_PK_InverseId], [x.OneToOne_Optional_FK].[OneToOne_Optional_SelfId]
FROM [Level2] AS [x.OneToOne_Optional_FK]
@__p_0: 2
SELECT [t].[Id], [t].[Date], [t].[Name], [t].[OneToMany_Optional_Self_InverseId], [t].[OneToMany_Required_Self_InverseId], [t].[OneToOne_Optional_SelfId], [t].[c0], [t].[c1], [t].[Level1_Optional_Id], [t].[Level1_Required_Id], [t].[c2], [t].[OneToMany_Optional_InverseId], [t].[c3], [t].[OneToMany_Required_InverseId], [t].[c4], [t].[OneToOne_Optional_PK_InverseId], [t].[c5]
FROM (
SELECT TOP(@__p_0) [l10].[Id], [l10].[Date], [l10].[Name], [l10].[OneToMany_Optional_Self_InverseId], [l10].[OneToMany_Required_Self_InverseId], [l10].[OneToOne_Optional_SelfId], [l1.OneToOne_Optional_FK1].[Id] AS [c0], [l1.OneToOne_Optional_FK1].[Date] AS [c1], [l1.OneToOne_Optional_FK1].[Level1_Optional_Id], [l1.OneToOne_Optional_FK1].[Level1_Required_Id], [l1.OneToOne_Optional_FK1].[Name] AS [c2], [l1.OneToOne_Optional_FK1].[OneToMany_Optional_InverseId], [l1.OneToOne_Optional_FK1].[OneToMany_Optional_Self_InverseId] AS [c3], [l1.OneToOne_Optional_FK1].[OneToMany_Required_InverseId], [l1.OneToOne_Optional_FK1].[OneToMany_Required_Self_InverseId] AS [c4], [l1.OneToOne_Optional_FK1].[OneToOne_Optional_PK_InverseId], [l1.OneToOne_Optional_FK1].[OneToOne_Optional_SelfId] AS [c5]
FROM [Level1] AS [l10]
LEFT JOIN [Level2] AS [l1.OneToOne_Optional_FK1] ON [l10].[Id] = [l1.OneToOne_Optional_FK1].[Level1_Optional_Id]
ORDER BY [l1.OneToOne_Optional_FK1].[Name], [l10].[Id]
) AS [t]
ORDER BY [t].[Name], [t].[Id]",
Sql);
}

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

Assert.Equal(
@"@__p_0: 2
SELECT [t].[Id], [t].[Date], [t].[Name], [t].[OneToMany_Optional_Self_InverseId], [t].[OneToMany_Required_Self_InverseId], [t].[OneToOne_Optional_SelfId], [t].[c0], [t].[c1], [t].[Level1_Optional_Id], [t].[Level1_Required_Id], [t].[c2], [t].[OneToMany_Optional_InverseId], [t].[c3], [t].[OneToMany_Required_InverseId], [t].[c4], [t].[OneToOne_Optional_PK_InverseId], [t].[c5]
FROM (
SELECT TOP(@__p_0) [x0].[Id], [x0].[Date], [x0].[Name], [x0].[OneToMany_Optional_Self_InverseId], [x0].[OneToMany_Required_Self_InverseId], [x0].[OneToOne_Optional_SelfId], [x.OneToOne_Optional_FK1].[Id] AS [c0], [x.OneToOne_Optional_FK1].[Date] AS [c1], [x.OneToOne_Optional_FK1].[Level1_Optional_Id], [x.OneToOne_Optional_FK1].[Level1_Required_Id], [x.OneToOne_Optional_FK1].[Name] AS [c2], [x.OneToOne_Optional_FK1].[OneToMany_Optional_InverseId], [x.OneToOne_Optional_FK1].[OneToMany_Optional_Self_InverseId] AS [c3], [x.OneToOne_Optional_FK1].[OneToMany_Required_InverseId], [x.OneToOne_Optional_FK1].[OneToMany_Required_Self_InverseId] AS [c4], [x.OneToOne_Optional_FK1].[OneToOne_Optional_PK_InverseId], [x.OneToOne_Optional_FK1].[OneToOne_Optional_SelfId] AS [c5]
FROM [Level1] AS [x0]
LEFT JOIN [Level2] AS [x.OneToOne_Optional_FK1] ON [x0].[Id] = [x.OneToOne_Optional_FK1].[Level1_Optional_Id]
ORDER BY [x.OneToOne_Optional_FK1].[Name], [x0].[Id]
) AS [t]
ORDER BY [t].[Name], [t].[Id]
SELECT [l2].[Id], [l2].[Date], [l2].[Level1_Optional_Id], [l2].[Level1_Required_Id], [l2].[Name], [l2].[OneToMany_Optional_InverseId], [l2].[OneToMany_Optional_Self_InverseId], [l2].[OneToMany_Required_InverseId], [l2].[OneToMany_Required_Self_InverseId], [l2].[OneToOne_Optional_PK_InverseId], [l2].[OneToOne_Optional_SelfId]
FROM [Level2] AS [l2]",
Sql);
}

private const string FileLineEnding = @"
";

Expand Down

0 comments on commit a089701

Please sign in to comment.