Skip to content

Commit

Permalink
Query: Navigations: Three bug fixes.
Browse files Browse the repository at this point in the history 
Fix: #3676 - Usage of the "let" keyword breaks grouping.
 - Fixed compiler bug in tracked, grouped queries.
Fix: #5427 - A column has been specified more than once in the order by list thrown for a complex query with orderby and navigation.
 - Fixed bug in SelectExpression.PrependToOrderBy
Fix: #4539 - Query :: Join flattening fails for some complex cases involving SelectMany
 - SelectMany after GroupJoin should not cause client-eval.

Also addressed most of the R# warnings in nav. rewriter.
  • Loading branch information
@anpete
anpete committed Jul 3, 2016
1 parent 6d94f68 commit 8f18be6
Show file tree
Hide file tree
Showing 12 changed files with 237 additions and 128 deletions.
12 changes: 10 additions & 2 deletions src/Microsoft.EntityFrameworkCore.Relational/Query/Expressions/SelectExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ public virtual TableExpressionBase GetTableForQuerySource([NotNull] IQuerySource
return _tables.FirstOrDefault(te
=> te.QuerySource == querySource
|| ((te as SelectExpression)?.HandlesQuerySource(querySource) ?? false))
?? _tables.Single();
?? _tables.Last();
}

/// <summary>
Expand Down Expand Up @@ -816,7 +816,15 @@ public virtual void PrependToOrderBy([NotNull] IEnumerable<Ordering> orderings)
{
Check.NotNull(orderings, nameof(orderings));

_orderBy.InsertRange(0, orderings);
var oldOrderBy = _orderBy.ToList();

_orderBy.Clear();
_orderBy.AddRange(orderings);

foreach (var ordering in oldOrderBy)
{
AddToOrderBy(ordering);
}
}

/// <summary>
Expand Down
28 changes: 25 additions & 3 deletions src/Microsoft.EntityFrameworkCore.Relational/Query/RelationalQueryModelVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,27 @@ public override void VisitAdditionalFromClause(
Check.NotNull(queryModel, nameof(queryModel));

base.VisitAdditionalFromClause(fromClause, queryModel, index);

var fromQuerySourceReferenceExpression
= fromClause.FromExpression as QuerySourceReferenceExpression;

if (fromQuerySourceReferenceExpression != null)
{
var previousQuerySource = FindPreviousQuerySource(queryModel, index - 1);

if (previousQuerySource != null
&& !RequiresClientJoin)
{
var previousSelectExpression = TryGetQuery(previousQuerySource);

if (previousSelectExpression != null)
{
AddQuery(fromClause, previousSelectExpression);
}
}

return;
}

RequiresClientSelectMany = true;

Expand Down Expand Up @@ -582,7 +603,7 @@ protected virtual void OptimizeJoinClause(
Check.NotNull(operatorToFlatten, nameof(operatorToFlatten));

RequiresClientJoin = true;

var previousQuerySource = FindPreviousQuerySource(queryModel, index);

var previousSelectExpression
Expand All @@ -594,8 +615,9 @@ var previousSelectProjectionCount
= previousSelectExpression?.Projection.Count ?? -1;

baseVisitAction();

if (previousSelectExpression != null)

if (!RequiresClientSelectMany
&& previousSelectExpression != null)
{
var selectExpression = TryGetQuery(joinClause);

Expand Down
9 changes: 3 additions & 6 deletions src/Microsoft.EntityFrameworkCore.Specification.Tests/GearsOfWarQueryTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -985,8 +985,7 @@ var cogTags
}
}

// issue 4539
////[ConditionalFact]
[ConditionalFact]
public virtual void Select_Where_Navigation_Scalar_Equals_Navigation_Scalar()
{
List<KeyValuePair<Guid, Guid>> expected;
Expand Down Expand Up @@ -1014,8 +1013,7 @@ from ct2 in context.Tags
}
}

// issue 4539
////[ConditionalFact]
[ConditionalFact]
public virtual void Select_Where_Navigation_Scalar_Equals_Navigation_Scalar_Projected()
{
List<KeyValuePair<Guid, Guid>> expected;
Expand Down Expand Up @@ -1112,8 +1110,7 @@ var cogTags
}
}

// issue 4539
////[ConditionalFact]
[ConditionalFact]
public virtual void Select_Where_Navigation_Equals_Navigation()
{
using (var context = CreateContext())
Expand Down
54 changes: 22 additions & 32 deletions src/Microsoft.EntityFrameworkCore.Specification.Tests/QueryNavigationsTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,13 @@
using Microsoft.EntityFrameworkCore.Specification.Tests.TestUtilities.Xunit;
using Xunit;

// ReSharper disable InconsistentNaming
// ReSharper disable PossibleMultipleEnumeration
// ReSharper disable ReplaceWithSingleCallToFirstOrDefault
// ReSharper disable ReplaceWithSingleCallToAny
// ReSharper disable ReplaceWithSingleCallToFirst
// ReSharper disable StringStartsWithIsCultureSpecific
// ReSharper disable UseCollectionCountProperty

// ReSharper disable AccessToDisposedClosure
// ReSharper disable PossibleUnintendedReferenceComparison

Expand Down Expand Up @@ -43,8 +48,7 @@ var orders
}
}

// issue 4539
////[ConditionalFact]
[ConditionalFact]
public virtual void Select_Where_Navigation_Scalar_Equals_Navigation_Scalar()
{
using (var context = CreateContext())
Expand All @@ -59,8 +63,7 @@ from o2 in context.Set<Order>().Where(o => o.OrderID < 10400)
}
}

// issue 4539
////[ConditionalFact]
[ConditionalFact]
public virtual void Select_Where_Navigation_Scalar_Equals_Navigation_Scalar_Projected()
{
using (var context = CreateContext())
Expand Down Expand Up @@ -186,8 +189,7 @@ public virtual void Select_Where_Navigation_Null_Deep()
entryCount: 6);
}

// issue 4539
////[ConditionalFact]
[ConditionalFact]
public virtual void Select_Where_Navigation_Equals_Navigation()
{
using (var context = CreateContext())
Expand Down Expand Up @@ -616,11 +618,11 @@ public virtual void Collection_select_nav_prop_first_or_default_then_nav_prop_ne
);
}

// #5427
////[ConditionalFact]
[ConditionalFact]
public virtual void Collection_select_nav_prop_first_or_default_then_nav_prop_nested_with_orderby()
{
AssertQuery<Customer, Order, string>(
// ReSharper disable once StringStartsWithIsCultureSpecific
(cs, os) => cs.Where(e => e.CustomerID.StartsWith("A"))
.Select(c => os.OrderBy(o => o.CustomerID).FirstOrDefault(o =>o.CustomerID == "ALFKI").Customer.City));
}
Expand Down Expand Up @@ -728,13 +730,10 @@ where c.Orders.Select(o => o.OrderID)
entryCount: 1);
}

private int ClientMethod(int argument)
{
return argument;
}
// ReSharper disable once MemberCanBeMadeStatic.Local
private int ClientMethod(int argument) => argument;

// issue #4547
////[ConditionalFact]
[ConditionalFact]
public virtual void Navigation_in_subquery_referencing_outer_query()
{
using (var context = CreateContext())
Expand Down Expand Up @@ -794,15 +793,13 @@ join efItem in efItems on l2oItem.Key equals efItem.Key
});
}

// issue #3676
//// [ConditionalFact]
[ConditionalFact]
public virtual void Let_group_by_nav_prop()
{
AssertQuery<OrderDetail, IGrouping<string, OrderDetail>>(
ods => from od in ods
let customer = od.Order.CustomerID
group od by customer
into odg
group od by customer into odg
select odg,
asserter: (l2oItems, efItems) =>
{
Expand Down Expand Up @@ -836,32 +833,25 @@ protected void AssertQuery<TItem>(
bool assertOrder = false,
int entryCount = 0,
Action<IList<object>, IList<object>> asserter = null)
where TItem : class
{
AssertQuery(query, query, assertOrder, entryCount, asserter);
}
where TItem : class
=> AssertQuery(query, query, assertOrder, entryCount, asserter);

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

where TItem2 : class
=> AssertQuery(query, query, assertOrder, entryCount, asserter);

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

protected void AssertQuery<TItem>(
Func<IQueryable<TItem>, IQueryable<object>> efQuery,
Expand Down
26 changes: 25 additions & 1 deletion src/Microsoft.EntityFrameworkCore.Specification.Tests/QueryTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3153,7 +3153,7 @@ from c in cs
select new { c, hasOrders });
}

// TODO: Need to figure out how to do this
// TODO: Need to figure out how to do this
// [ConditionalFact]
// public virtual void GroupBy_anonymous()
// {
Expand Down Expand Up @@ -4623,6 +4623,30 @@ from o in orders.DefaultIfEmpty()
select o);
}


[ConditionalFact]
public virtual void GroupJoin_Where()
{
AssertQuery<Customer, Order>((cs, os) =>
from c in cs
join o in os on c.CustomerID equals o.CustomerID into orders
from o in orders
where o.CustomerID == "ALFKI"
select o);
}

[ConditionalFact]
public virtual void GroupJoin_DefaultIfEmpty_Where_OrderBy()
{
AssertQuery<Customer, Order>((cs, os) =>
from c in cs
join o in os on c.CustomerID equals o.CustomerID into orders
from o in orders
where o.CustomerID == "ALFKI" || c.CustomerID == "ANATR"
orderby c.City
select o);
}

[ConditionalFact]
public virtual void SelectMany_Joined()
{
Expand Down
18 changes: 12 additions & 6 deletions src/Microsoft.EntityFrameworkCore/Query/EntityQueryModelVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -548,9 +548,15 @@ var lastTrackingModifier
return;
}

var groupResultOperator
= queryModel.ResultOperators.OfType<GroupResultOperator>().LastOrDefault();

var outputExpression
= groupResultOperator?.ElementSelector ?? queryModel.SelectClause.Selector;

var entityTrackingInfos
= _entityResultFindingExpressionVisitorFactory.Create(QueryCompilationContext)
.FindEntitiesInResult(queryModel.SelectClause.Selector);
.FindEntitiesInResult(outputExpression);

if (entityTrackingInfos.Any())
{
Expand All @@ -562,21 +568,21 @@ var entityTrackingInfos
if (resultItemTypeInfo.IsGenericType
&& (resultItemTypeInfo.GetGenericTypeDefinition() == typeof(IGrouping<,>)
|| resultItemTypeInfo.GetGenericTypeDefinition() == typeof(IAsyncGrouping<,>)))

{
trackingMethod
= LinqOperatorProvider.TrackGroupedEntities
.MakeGenericMethod(
resultItemType.GenericTypeArguments[0],
resultItemType.GenericTypeArguments[1],
queryModel.SelectClause.Selector.Type);
resultItemType.GenericTypeArguments[1]);
}
else
{
trackingMethod
= LinqOperatorProvider.TrackEntities
.MakeGenericMethod(
resultItemType,
queryModel.SelectClause.Selector.Type);
outputExpression.Type);
}

_expression
Expand All @@ -587,13 +593,13 @@ var entityTrackingInfos
Expression.Constant(entityTrackingInfos),
Expression.Constant(
_getEntityAccessors
.MakeGenericMethod(queryModel.SelectClause.Selector.Type)
.MakeGenericMethod(outputExpression.Type)
.Invoke(
null,
new object[]
{
entityTrackingInfos,
queryModel.SelectClause.Selector
outputExpression
})));
}
}
Expand Down
Loading

0 comments on commit 8f18be6

Please sign in to comment.