Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Set operations: cleanup #16249

Merged
merged 1 commit into from
Jun 28, 2019
Merged

Set operations: cleanup #16249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions src/EFCore.Relational/Query/Pipeline/QuerySqlGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,13 +236,9 @@ protected override Expression VisitSqlFunction(SqlFunctionExpression sqlFunction

protected override Expression VisitColumn(ColumnExpression columnExpression)
{
if (columnExpression.Table.Alias != null)
{
_relationalCommandBuilder
.Append(_sqlGenerationHelper.DelimitIdentifier(columnExpression.Table.Alias))
.Append(".");
}
_relationalCommandBuilder
.Append(_sqlGenerationHelper.DelimitIdentifier(columnExpression.Table.Alias))
.Append(".")
.Append(_sqlGenerationHelper.DelimitIdentifier(columnExpression.Name));

return columnExpression;
Expand Down
8 changes: 6 additions & 2 deletions src/EFCore.Relational/Query/Pipeline/SqlExpressions/ColumnExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Query.Internal;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Utilities;

namespace Microsoft.EntityFrameworkCore.Relational.Query.Pipeline.SqlExpressions
{
Expand All @@ -27,6 +28,10 @@ internal ColumnExpression(ProjectionExpression subqueryProjection, TableExpressi
private ColumnExpression(string name, TableExpressionBase table, Type type, RelationalTypeMapping typeMapping, bool nullable)
: base(type, typeMapping)
{
Check.NotEmpty(name, nameof(name));
Check.NotNull(table, nameof(table));
Check.NotEmpty(table.Alias, $"{nameof(table)}.{nameof(table.Alias)}");

Name = name;
Table = table;
Nullable = nullable;
Expand Down Expand Up @@ -71,7 +76,6 @@ private bool Equals(ColumnExpression columnExpression)

public override int GetHashCode() => HashCode.Combine(base.GetHashCode(), Name, Table, Nullable);

private string DebuggerDisplay()
=> Table.Alias == null ? Name : $"{Table.Alias}.{Name}";
private string DebuggerDisplay() => $"{Table.Alias}.{Name}";
}
}
190 changes: 66 additions & 124 deletions src/EFCore.Relational/Query/Pipeline/SqlExpressions/SelectExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Reflection;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Microsoft.EntityFrameworkCore.Query.Internal;
Expand Down Expand Up @@ -258,9 +259,8 @@ public void ApplyPredicate(SqlExpression expression)

public void ApplyOrdering(OrderingExpression orderingExpression)
{
if (IsDistinct
|| Limit != null
|| Offset != null)
// TODO: We should not be pushing down set operations, see #16244
if (IsDistinct || Limit != null || Offset != null || IsSetOperation)
{
orderingExpression = orderingExpression.Update(
new SqlRemappingVisitor(PushdownIntoSubquery())
Expand All @@ -281,7 +281,8 @@ public void AppendOrdering(OrderingExpression orderingExpression)

public void ApplyLimit(SqlExpression sqlExpression)
{
if (Limit != null)
// TODO: We should not be pushing down set operations, see #16244
if (Limit != null || IsSetOperation)
{
PushdownIntoSubquery();
}
Expand All @@ -291,8 +292,8 @@ public void ApplyLimit(SqlExpression sqlExpression)

public void ApplyOffset(SqlExpression sqlExpression)
{
if (Limit != null
|| Offset != null)
// TODO: We should not be pushing down set operations, see #16244
if (Limit != null || Offset != null || IsSetOperation)
{
PushdownIntoSubquery();
}
Expand Down Expand Up @@ -367,11 +368,14 @@ public Expression ApplySetOperation(
select1._projectionMapping = new Dictionary<ProjectionMember, Expression>(_projectionMapping);
_projectionMapping.Clear();

select1._identifier.AddRange(_identifier);
_identifier.Clear();

var select2 = otherSelectExpression;

if (_projection.Any())
{
throw new NotImplementedException("Set operation on SelectExpression with populated _projection");
throw new InvalidOperationException("Can't process set operations after client evaluation, consider moving the operation before the last Select() call (see issue #16243)");
}
else
{
Expand All @@ -387,106 +391,24 @@ public Expression ApplySetOperation(
kv => kv.Key,
(kv1, kv2) => (kv1.Key, Value1: kv1.Value, Value2: kv2.Value)))
{

if (joinedMapping.Value1 is EntityProjectionExpression entityProjection1
&& joinedMapping.Value2 is EntityProjectionExpression entityProjection2)
{
var propertyExpressions = new Dictionary<IProperty, ColumnExpression>();

if (entityProjection1.EntityType == entityProjection2.EntityType)
{
foreach (var property in GetAllPropertiesInHierarchy(entityProjection1.EntityType))
{
propertyExpressions[property] = AddSetOperationColumnProjections(
property,
select1, entityProjection1.GetProperty(property),
select2, entityProjection2.GetProperty(property));
}

_projectionMapping[joinedMapping.Key] = new EntityProjectionExpression(entityProjection1.EntityType, propertyExpressions);
continue;
}

// We're doing a set operation over two different entity types (within the same hierarchy).
// Since both sides of the set operations must produce the same result shape, find the
// closest common ancestor and load all the columns for that, adding null projections where
// necessary. Note this means we add null projections for properties which neither sibling
// actually needs, since the shaper doesn't know that only those sibling types will be coming
// back.
var commonParentEntityType = entityProjection1.EntityType.GetClosestCommonParent(entityProjection2.EntityType);

if (commonParentEntityType == null)
{
throw new NotSupportedException(RelationalStrings.SetOperationNotWithinEntityTypeHierarchy);
}

var properties1 = GetAllPropertiesInHierarchy(entityProjection1.EntityType).ToArray();
var properties2 = GetAllPropertiesInHierarchy(entityProjection2.EntityType).ToArray();

foreach (var property in properties1.Intersect(properties2))
{
propertyExpressions[property] = AddSetOperationColumnProjections(
property,
select1, entityProjection1.GetProperty(property),
select2, entityProjection2.GetProperty(property));
}

foreach (var property in properties1.Except(properties2))
{
propertyExpressions[property] = AddSetOperationColumnProjections(
property,
select1, entityProjection1.GetProperty(property),
select2, null);
}

foreach (var property in properties2.Except(properties1))
{
propertyExpressions[property] = AddSetOperationColumnProjections(
property,
select1, null,
select2, entityProjection2.GetProperty(property));
}

foreach (var property in GetAllPropertiesInHierarchy(commonParentEntityType)
.Except(properties1).Except(properties2))
{
propertyExpressions[property] = AddSetOperationColumnProjections(
property,
select1, null,
select2, null);
}

_projectionMapping[joinedMapping.Key] = new EntityProjectionExpression(commonParentEntityType, propertyExpressions);

if (commonParentEntityType != entityProjection1.EntityType)
{
if (!(shaperExpression.RemoveConvert() is EntityShaperExpression entityShaperExpression))
{
throw new Exception("Non-entity shaper expression while handling set operation over siblings.");
}

shaperExpression = new EntityShaperExpression(
commonParentEntityType, entityShaperExpression.ValueBufferExpression, entityShaperExpression.Nullable);
}

HandleEntityMapping(joinedMapping.Key, select1, entityProjection1, select2, entityProjection2);
roji marked this conversation as resolved.
Show resolved Hide resolved
continue;
}

if (joinedMapping.Value1 is ColumnExpression innerColumn1
&& joinedMapping.Value2 is ColumnExpression innerColumn2)
if (joinedMapping.Value1 is ColumnExpression && joinedMapping.Value2 is ColumnExpression
|| joinedMapping.Value1 is SubSelectExpression && joinedMapping.Value2 is SubSelectExpression)
{
// The actual columns may actually be different, but we don't care as long as the type and alias
// coming out of the two operands are the same
var alias = joinedMapping.Key.LastMember?.Name;
var index = select1.AddToProjection(innerColumn1, alias);
var projectionExpression1 = select1._projection[index];
select2.AddToProjection(innerColumn2, alias);
var outerColumn = new ColumnExpression(projectionExpression1, select1, IsNullableProjection(projectionExpression1));
_projectionMapping[joinedMapping.Key] = outerColumn;
HandleColumnMapping(
joinedMapping.Key,
select1, (SqlExpression)joinedMapping.Value1,
select2, (SqlExpression)joinedMapping.Value2);
continue;
}

throw new NotSupportedException("Non-matching or unknown projection mapping type in set operation");
throw new InvalidOperationException("Non-matching or unknown projection mapping type in set operation");
}
}

Expand All @@ -502,34 +424,59 @@ public Expression ApplySetOperation(

return shaperExpression;

static ColumnExpression AddSetOperationColumnProjections(
void HandleEntityMapping(
ProjectionMember projectionMember,
SelectExpression select1, EntityProjectionExpression projection1,
SelectExpression select2, EntityProjectionExpression projection2)
{
var propertyExpressions = new Dictionary<IProperty, ColumnExpression>();

if (projection1.EntityType == projection2.EntityType)
{
foreach (var property in GetAllPropertiesInHierarchy(projection1.EntityType))
{
propertyExpressions[property] = AddSetOperationColumnProjections(
property,
select1, projection1.GetProperty(property),
select2, projection2.GetProperty(property));
}

_projectionMapping[projectionMember] = new EntityProjectionExpression(projection1.EntityType, propertyExpressions);
return;
}

throw new InvalidOperationException("Set operations over different entity types are currently unsupported (see #16298)");
}

ColumnExpression AddSetOperationColumnProjections(
IProperty property,
SelectExpression select1, ColumnExpression column1,
SelectExpression select2, ColumnExpression column2)
{
var columnName = column1?.Name ?? column2?.Name ?? property.Name;
var baseColumnName = columnName;
var counter = 0;
while (select1._projection.Any(pe => string.Equals(pe.Alias, columnName, StringComparison.OrdinalIgnoreCase)))
{
columnName = $"{baseColumnName}{counter++}";
}
var columnName = column1.Name;

var typeMapping = column1?.TypeMapping ?? column2?.TypeMapping ?? property.FindRelationalMapping();
select1._projection.Add(new ProjectionExpression(column1, columnName));
select2._projection.Add(new ProjectionExpression(column2, columnName));

select1._projection.Add(new ProjectionExpression(column1 != null
? (SqlExpression)column1
: new SqlConstantExpression(Constant(null), typeMapping),
columnName));
if (select1._identifier.Contains(column1))
{
_identifier.Add(column1);
}

select2._projection.Add(new ProjectionExpression(column2 != null
? (SqlExpression)column2
: new SqlConstantExpression(Constant(null), typeMapping),
columnName));
return column1;
}

var projectionExpression = select1._projection[select1._projection.Count - 1];
var outerColumn = new ColumnExpression(projectionExpression, select1, IsNullableProjection(projectionExpression));
return outerColumn;
void HandleColumnMapping(
ProjectionMember projectionMember,
SelectExpression select1, SqlExpression innerColumn1,
SelectExpression select2, SqlExpression innerColumn2)
{
// The actual columns may actually be different, but we don't care as long as the type and alias
// coming out of the two operands are the same
var alias = projectionMember.LastMember?.Name;
select1.AddToProjection(innerColumn1, alias);
select2.AddToProjection(innerColumn2, alias);
_projectionMapping[projectionMember] = innerColumn1;
}
}

Expand Down Expand Up @@ -901,9 +848,7 @@ public void AddInnerJoin(SelectExpression innerSelectExpression, SqlExpression j

public void AddLeftJoin(SelectExpression innerSelectExpression, SqlExpression joinPredicate, Type transparentIdentifierType)
{
if (Limit != null
|| Offset != null
|| IsDistinct)
if (Limit != null || Offset != null || IsDistinct || IsSetOperation)
{
joinPredicate = new SqlRemappingVisitor(PushdownIntoSubquery())
.Remap(joinPredicate);
Expand Down Expand Up @@ -951,10 +896,7 @@ public void AddLeftJoin(SelectExpression innerSelectExpression, SqlExpression jo

public void AddCrossJoin(SelectExpression innerSelectExpression, Type transparentIdentifierType)
{
if (Limit != null
|| Offset != null
|| IsDistinct
|| Predicate != null)
if (Limit != null || Offset != null || IsDistinct || Predicate != null || IsSetOperation)
{
PushdownIntoSubquery();
}
Expand Down
11 changes: 4 additions & 7 deletions src/EFCore/Query/NavigationExpansion/NavigationTreeNode.cs
View file
Open in desktop
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
10 changes: 0 additions & 10 deletions test/EFCore.Cosmos.FunctionalTests/Query/SimpleQueryCosmosTest.ResultOperators.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,6 @@ namespace Microsoft.EntityFrameworkCore.Cosmos.Query
{
public partial class SimpleQueryCosmosTest
{
public override async Task Union_with_custom_projection(bool isAsync)
{
await base.Union_with_custom_projection(isAsync);

AssertSql(
@"SELECT c
FROM root c
WHERE (c[""Discriminator""] = ""Customer"")");
}

[ConditionalTheory(Skip = "Issue#14935")]
public override void Select_All()
{
Expand Down
Loading