Skip to content

Commit

Permalink
Addressed review comments
Browse files Browse the repository at this point in the history 
Also found some more skipped set operation tests, unskipped and
rearranged them.

Fixes #12568
  • Loading branch information
@roji
roji committed Jun 27, 2019
1 parent b2b7e5b commit 8b7a7fa
Show file tree
Hide file tree
Showing 9 changed files with 249 additions and 299 deletions.
36 changes: 18 additions & 18 deletions src/EFCore.Relational/Query/Pipeline/SqlExpressions/SelectExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -497,18 +497,8 @@ void HandleEntityMapping(
if (commonParentEntityType != projection1.EntityType)
{
// The first source has been up-cast by the set operation, so we also need to change the shaper expression.
var entityShaperExpression =
shaperExpression as EntityShaperExpression ?? (
shaperExpression is UnaryExpression unary
&& unary.NodeType == ExpressionType.Convert
&& unary.Type == commonParentEntityType.ClrType
? unary.Operand as EntityShaperExpression : null);

if (entityShaperExpression != null)
{
shaperExpression = new EntityShaperExpression(
commonParentEntityType, entityShaperExpression.ValueBufferExpression, entityShaperExpression.Nullable);
}
// The EntityShaperExpression may be buried under Convert nodes produced by Cast operators, preserve those.
shaperExpression = UpdateEntityShaperEntityType(shaperExpression, commonParentEntityType);
}
}

Expand Down Expand Up @@ -562,6 +552,21 @@ void HandleColumnMapping(
var outerColumn = new ColumnExpression(projectionExpression1, select1, IsNullableProjection(projectionExpression1));
_projectionMapping[projectionMember] = outerColumn;
}

static Expression UpdateEntityShaperEntityType(Expression shaperExpression, IEntityType newEntityType)
{
switch (shaperExpression)
{
case EntityShaperExpression entityShaperExpression:
return new EntityShaperExpression(newEntityType, entityShaperExpression.ValueBufferExpression, entityShaperExpression.Nullable);
case UnaryExpression unary when unary.NodeType == ExpressionType.Convert:
return Convert(UpdateEntityShaperEntityType(unary.Operand, newEntityType), unary.Type);
case UnaryExpression unary when unary.NodeType == ExpressionType.ConvertChecked:
return ConvertChecked(UpdateEntityShaperEntityType(unary.Operand, newEntityType), unary.Type);
default:
throw new Exception($"Unexpected expression type {shaperExpression.GetType().Name} encountered in {nameof(UpdateEntityShaperEntityType)}");
}
}
}

public IDictionary<SqlExpression, ColumnExpression> PushdownIntoSubquery()
Expand Down Expand Up @@ -1349,12 +1354,7 @@ public enum SetOperationType
/// <summary>
/// Represents an SQL EXCEPT set operation.
/// </summary>
Except = 4,

/// <summary>
/// Represents a custom, provider-specific set operation.
/// </summary>
Other = 9999
Except = 4
}
}

5 changes: 5 additions & 0 deletions src/EFCore/Properties/CoreStrings.Designer.cs
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion src/EFCore/Properties/CoreStrings.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1186,4 +1186,7 @@
<data name="UnableToDiscriminate" xml:space="preserve">
<value>Unable to materialize entity of type '{entityType}'. No discriminators matched '{discriminator}'.</value>
</data>
</root>
<data name="SetOperationWithDifferentIncludesInOperands" xml:space="preserve">
<value>When performing a set operation, both operands must have the same Include operations.</value>
</data>
</root>
11 changes: 4 additions & 7 deletions src/EFCore/Query/NavigationExpansion/NavigationTreeNode.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,17 +132,14 @@ public static NavigationTreeNode Create(
return result;
}

public List<NavigationTreeNode> Flatten()
public IEnumerable<NavigationTreeNode> Flatten()
{
var result = new List<NavigationTreeNode>();
result.Add(this);
yield return this;

foreach (var child in Children)
foreach (var child in Children.SelectMany(c => c.Flatten()))
{
result.AddRange(child.Flatten());
yield return child;
}

return result;
}

// TODO: just make property settable?
Expand Down
30 changes: 30 additions & 0 deletions src/EFCore/Query/NavigationExpansion/Visitors/NavigationExpandingVisitor_MethodCall.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Linq.Expressions;
using System.Reflection;
using System.Xml;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
Expand Down Expand Up @@ -873,6 +874,35 @@ private Expression ProcessSetOperation(MethodCallExpression methodCallExpression
var source2 = VisitSourceExpression(methodCallExpression.Arguments[1]);
var preProcessResult2 = PreProcessTerminatingOperation(source2);

// Compare the include chains from each side to make sure they're identical. We don't allow set operations over
// operands with different include chains.
var current1 = preProcessResult1.state.PendingIncludeChain?.NavigationTreeNode;
var current2 = preProcessResult2.state.PendingIncludeChain?.NavigationTreeNode;
while (true)
{
if (current1 == null)
{
if (current2 == null)
{
break;
}
throw new NotSupportedException(CoreStrings.SetOperationWithDifferentIncludesInOperands);
}

if (current2 == null)
{
throw new NotSupportedException(CoreStrings.SetOperationWithDifferentIncludesInOperands);
}

if (current1.FromMappings.Zip(current2.FromMappings, (m1, m2) => (m1, m2))
.Any(t => !t.m1.SequenceEqual(t.m2)))
{
throw new NotSupportedException(CoreStrings.SetOperationWithDifferentIncludesInOperands);
}

(current1, current2) = (current1.Parent, current2.Parent);
}

// If the siblings are different types, one is derived from the other the set operation returns the less derived type.
// Find that.
var clrType1 = preProcessResult1.state.CurrentParameter.Type;
Expand Down
236 changes: 0 additions & 236 deletions test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.ResultOperators.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,6 @@ namespace Microsoft.EntityFrameworkCore.Query
{
public abstract partial class SimpleQueryTestBase<TFixture>
{
[ConditionalTheory(Skip = "Issue #6812. Cannot eval 'Union({from Customer c in value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[Microsoft.EntityFrameworkCore.TestModels.Northwind.Customer]) where [c].CompanyName.StartsWith(\"B\") select [c]})'")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Union_with_custom_projection(bool isAsync)
{
return AssertQuery<Customer>(
isAsync,
cs => cs.Where(c => c.CompanyName.StartsWith("A"))
.Union(cs.Where(c => c.CompanyName.StartsWith("B")))
.Select(
c => new CustomerDeets
{
Id = c.CustomerID
}));
}

public class CustomerDeets
{
public string Id { get; set; }
Expand Down Expand Up @@ -1460,227 +1445,6 @@ public virtual void OfType_Select_OfType_Select()
}
}

[ConditionalFact(Skip = "Issue #6812. Cannot eval 'Concat({value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[Microsoft.EntityFrameworkCore.TestModels.Northwind.Customer])})'")]
public virtual void Concat_dbset()
{
using (var context = CreateContext())
{
var query = context.Set<Customer>()
.Where(c => c.City == "México D.F.")
.Concat(context.Set<Customer>())
.ToList();

Assert.Equal(96, query.Count);
}
}

[ConditionalFact(Skip = "Issue #6812. Cannot eval 'Concat({from Customer s in value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[Microsoft.EntityFrameworkCore.TestModels.Northwind.Customer]) where ([s].ContactTitle == \"Owner\") select [s]})'")]
public virtual void Concat_simple()
{
using (var context = CreateContext())
{
var query = context.Set<Customer>()
.Where(c => c.City == "México D.F.")
.Concat(
context.Set<Customer>()
.Where(s => s.ContactTitle == "Owner"))
.ToList();

Assert.Equal(22, query.Count);
}
}

[ConditionalTheory(Skip = "Issue #6812. Cannot eval 'Concat({from Customer s in value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[Microsoft.EntityFrameworkCore.TestModels.Northwind.Customer]) where ([s].City == \"Berlin\") select [s]})'")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Concat_nested(bool isAsync)
{
return AssertQuery<Customer>(
isAsync,
cs => cs.Where(c => c.City == "México D.F.")
.Concat(cs.Where(s => s.City == "Berlin"))
.Concat(cs.Where(e => e.City == "London")),
entryCount: 12);
}

[ConditionalFact(Skip = "Issue #6812. Cannot eval 'Concat({from Customer s in value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[Microsoft.EntityFrameworkCore.TestModels.Northwind.Customer]) where ([s].ContactTitle == \"Owner\") select [s].CustomerID})'")]
public virtual void Concat_non_entity()
{
using (var context = CreateContext())
{
var query = context.Set<Customer>()
.Where(c => c.City == "México D.F.")
.Select(c => c.CustomerID)
.Concat(
context.Set<Customer>()
.Where(s => s.ContactTitle == "Owner")
.Select(c => c.CustomerID))
.ToList();

Assert.Equal(22, query.Count);
}
}

[ConditionalTheory(Skip = "Issue #6812. Cannot eval 'Except({value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[Microsoft.EntityFrameworkCore.TestModels.Northwind.Customer])})'")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Except_dbset(bool isAsync)
{
return AssertQuery<Customer>(
isAsync,
cs => cs.Where(s => s.ContactTitle == "Owner").Except(cs));
}

[ConditionalTheory(Skip = "Issue #6812. Cannot eval 'Except({from Customer c in value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[Microsoft.EntityFrameworkCore.TestModels.Northwind.Customer]) where ([c].City == \"México D.F.\") select [c]})'")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Except_simple(bool isAsync)
{
return AssertQuery<Customer>(
isAsync,
cs => cs.Where(s => s.ContactTitle == "Owner")
.Except(cs.Where(c => c.City == "México D.F.")),
entryCount: 14);
}

[ConditionalTheory(Skip = "Issue#12568")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Except_simple_followed_by_projecting_constant(bool isAsync)
{
return AssertQueryScalar<Customer>(
isAsync,
cs => cs.Except(cs).Select(e => 1));
}

[ConditionalTheory(Skip = "Issue #6812. Cannot eval 'Except({from Customer s in value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[Microsoft.EntityFrameworkCore.TestModels.Northwind.Customer]) where ([s].City == \"México D.F.\") select [s]})'")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Except_nested(bool isAsync)
{
return AssertQuery<Customer>(
isAsync,
cs => cs.Where(s => s.ContactTitle == "Owner")
.Except(cs.Where(s => s.City == "México D.F."))
.Except(cs.Where(e => e.City == "Seattle")),
entryCount: 13);
}

[ConditionalFact(Skip = "Issue #6812. Cannot eval 'Except({from Customer c in value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[Microsoft.EntityFrameworkCore.TestModels.Northwind.Customer]) where ([c].City == \"México D.F.\") select [c].CustomerID})'")]
public virtual void Except_non_entity()
{
using (var context = CreateContext())
{
var query = context.Set<Customer>()
.Where(s => s.ContactTitle == "Owner")
.Select(c => c.CustomerID)
.Except(
context.Set<Customer>()
.Where(c => c.City == "México D.F.")
.Select(c => c.CustomerID))
.ToList();

Assert.Equal(14, query.Count);
}
}

[ConditionalTheory(Skip = "Issue #6812. Cannot eval 'Intersect({value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[Microsoft.EntityFrameworkCore.TestModels.Northwind.Customer])})'")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Intersect_dbset(bool isAsync)
{
return AssertQuery<Customer>(
isAsync,
cs => cs.Where(c => c.City == "México D.F.").Intersect(cs),
entryCount: 5);
}

[ConditionalTheory(Skip = "Issue #6812. Cannot eval 'Intersect({from Customer s in value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[Microsoft.EntityFrameworkCore.TestModels.Northwind.Customer]) where ([s].ContactTitle == \"Owner\") select [s]})'")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Intersect_simple(bool isAsync)
{
return AssertQuery<Customer>(
isAsync,
cs => cs.Where(c => c.City == "México D.F.")
.Intersect(cs.Where(s => s.ContactTitle == "Owner")),
entryCount: 3);
}

[ConditionalTheory(Skip = "Issue #6812. Cannot eval 'Intersect({from Customer s in value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[Microsoft.EntityFrameworkCore.TestModels.Northwind.Customer]) where ([s].ContactTitle == \"Owner\") select [s]})'")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Intersect_nested(bool isAsync)
{
return AssertQuery<Customer>(
isAsync,
cs => cs.Where(c => c.City == "México D.F.")
.Intersect(cs.Where(s => s.ContactTitle == "Owner"))
.Intersect(cs.Where(e => e.Fax != null)),
entryCount: 1);
}

[ConditionalFact(Skip = "Issue #6812. Cannot eval 'Intersect({from Customer s in value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[Microsoft.EntityFrameworkCore.TestModels.Northwind.Customer]) where ([s].ContactTitle == \"Owner\") select [s].CustomerID})'")]
public virtual void Intersect_non_entity()
{
using (var context = CreateContext())
{
var query = context.Set<Customer>()
.Where(c => c.City == "México D.F.")
.Select(c => c.CustomerID)
.Intersect(
context.Set<Customer>()
.Where(s => s.ContactTitle == "Owner")
.Select(c => c.CustomerID))
.ToList();

Assert.Equal(3, query.Count);
}
}

[ConditionalTheory(Skip = "Issue #6812. Cannot eval 'Union({value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[Microsoft.EntityFrameworkCore.TestModels.Northwind.Customer])})'")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Union_dbset(bool isAsync)
{
return AssertQuery<Customer>(
isAsync,
cs => cs.Where(s => s.ContactTitle == "Owner").Union(cs),
entryCount: 91);
}

[ConditionalTheory(Skip = "Issue #6812. Cannot eval 'Union({from Customer c in value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[Microsoft.EntityFrameworkCore.TestModels.Northwind.Customer]) where ([c].City == \"México D.F.\") select [c]})'")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Union_simple(bool isAsync)
{
return AssertQuery<Customer>(
isAsync,
cs => cs.Where(s => s.ContactTitle == "Owner")
.Union(cs.Where(c => c.City == "México D.F.")),
entryCount: 19);
}

[ConditionalTheory(Skip = "Issue #6812. Cannot eval 'Union({from Customer s in value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[Microsoft.EntityFrameworkCore.TestModels.Northwind.Customer]) where ([s].City == \"México D.F.\") select [s]})'")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Union_nested(bool isAsync)
{
return AssertQuery<Customer>(
isAsync,
cs => cs.Where(s => s.ContactTitle == "Owner")
.Union(cs.Where(s => s.City == "México D.F."))
.Union(cs.Where(e => e.City == "London")),
entryCount: 25);
}

[ConditionalFact(Skip = "Issue #6812. Cannot eval 'Union({from Customer c in value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[Microsoft.EntityFrameworkCore.TestModels.Northwind.Customer]) where ([c].City == \"México D.F.\") select [c].CustomerID})'")]
public virtual void Union_non_entity()
{
using (var context = CreateContext())
{
var query = context.Set<Customer>()
.Where(s => s.ContactTitle == "Owner")
.Select(c => c.CustomerID)
.Union(
context.Set<Customer>()
.Where(c => c.City == "México D.F.")
.Select(c => c.CustomerID))
.ToList();

Assert.Equal(19, query.Count);
}
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Average_with_non_matching_types_in_projection_doesnt_produce_second_explicit_cast(bool isAsync)
Expand Down
Loading

0 comments on commit 8b7a7fa

Please sign in to comment.