Skip to content

Commit

Permalink
Fix to #11904 - 2.1 regression: "variable referenced from scope '', b…
Browse files Browse the repository at this point in the history
…ut it is not defined" exception in GroupBy query translation

Problem was that we were not correctly handling scenarios where multiple groupbys are present:

- if combined with Include, we would try to create orderings based on grouping keys for each group by. This is incorrect because all group keys apart from the innermost are out of scope of the inner query model (where we attempted to add them)
- if combined with aggregate we would try to translate into group-aggregate pattern and fail.

Fix is to only add ordering for the first grouping in case of include (includes are ignored for multi-groupby scenarios, so there is no risk of collection misalignment) and don't try to convert queries into group-aggregate pattern if multiple groupbys are present in the query.
  • Loading branch information
maumar committed May 5, 2018
1 parent 84b75d5 commit c64c15d
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 10 deletions.
17 changes: 8 additions & 9 deletions src/EFCore.Relational/Query/RelationalQueryModelVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1293,16 +1293,15 @@ protected override void VisitResultOperators(ObservableCollection<ResultOperator
queryModel.BodyClauses.Add(orderByClause);
}

foreach (var groupResultOperator in groupResultOperators)
{
var groupKeys = groupResultOperator.KeySelector is NewExpression compositeGroupKey
? compositeGroupKey.Arguments.Reverse()
: new[] { groupResultOperator.KeySelector };
var firstGroupResultOperator = groupResultOperators.First();

foreach (var groupKey in groupKeys)
{
orderByClause.Orderings.Insert(0, new Ordering(groupKey, OrderingDirection.Asc));
}
var groupKeys = firstGroupResultOperator.KeySelector is NewExpression compositeGroupKey
? compositeGroupKey.Arguments.Reverse()
: new[] { firstGroupResultOperator.KeySelector };

foreach (var groupKey in groupKeys)
{
orderByClause.Orderings.Insert(0, new Ordering(groupKey, OrderingDirection.Asc));
}
}
}
Expand Down
67 changes: 67 additions & 0 deletions src/EFCore.Specification.Tests/Query/IncludeTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3996,6 +3996,73 @@ public virtual async Task Include_empty_reference_sets_IsLoaded(bool useString,
}
}

[Theory(Skip = "Issue #11916")]
[InlineData(false, false)]
[InlineData(true, false)]
// async blocked by issue #11917
//[InlineData(false, true)]
//[InlineData(true, true)]
public virtual async Task Include_with_double_group_by(bool useString, bool async)
{
using (var context = CreateContext())
{
var groups = (useString
? context.Orders.Include(nameof(Order.OrderDetails))
: context.Orders.Include(e => e.OrderDetails))
.GroupBy(o => new { o.OrderID, o.OrderDate })
.GroupBy(g => g.Key.OrderDate);

var employee = async
? await groups.ToListAsync()
: groups.ToList();
}
}

[Theory]
[InlineData(false, false)]
[InlineData(true, false)]
// async blocked by issue #11917
//[InlineData(false, true)]
//[InlineData(true, true)]
public virtual async Task Include_with_double_group_by_no_tracking(bool useString, bool async)
{
using (var context = CreateContext())
{
var groups = (useString
? context.Orders.Include(nameof(Order.OrderDetails)).AsNoTracking()
: context.Orders.Include(e => e.OrderDetails)).AsNoTracking()
.GroupBy(o => new { o.OrderID, o.OrderDate })
.GroupBy(g => g.Key.OrderDate);

var employee = async
? await groups.ToListAsync()
: groups.ToList();
}
}

[Theory]
[InlineData(false, false)]
[InlineData(true, false)]
// async blocked by issue #11917
//[InlineData(false, true)]
//[InlineData(true, true)]
public virtual async Task Include_with_double_group_by_and_aggregate(bool useString, bool async)
{
using (var context = CreateContext())
{
var groups = (useString
? context.Orders.Include(nameof(Order.OrderDetails))
: context.Orders.Include(e => e.OrderDetails))
.GroupBy(o => new { o.OrderID, o.OrderDate })
.GroupBy(g => g.Key.OrderDate)
.Select(g => new { g.Key, Lastest = g.OrderBy(e => e.Key.OrderID).FirstOrDefault() });

var query = async
? await groups.ToListAsync()
: groups.ToList();
}
}

private static void CheckIsLoaded(
NorthwindContext context,
Customer customer,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,9 @@ var referencedQuerySource
var isSubQuery = _queryModelStack.Count > 0;
var finalResultOperator = queryModel.ResultOperators.LastOrDefault();

if (isSubQuery && finalResultOperator is GroupResultOperator groupResultOperator)
if (isSubQuery
&& finalResultOperator is GroupResultOperator groupResultOperator
&& queryModel.ResultOperators.OfType<GroupResultOperator>().Count() == 1)
{
if (!(groupResultOperator.KeySelector is MemberInitExpression))
{
Expand Down

0 comments on commit c64c15d

Please sign in to comment.